feat: Banner for production #583

KirillDogadin-std · 2023-02-07T08:47:39Z

Closes #581

Checklist:

issue number linked above after pound (#)
- replace "Closes " with "Contributes to" or other if this PR does not close the issue
issue checkboxes are all addressed
manually checked my feature / not applicable
wrote tests / not applicable
attached screenshots / not applicable

LukSteib · 2023-02-07T08:49:15Z

Can we adjust the wording from "... electron packaged application" to "Desktop application".

LukSteib

Is there a way for me to test locally?!

LukSteib · 2023-02-07T08:57:16Z

frontend/README.md

@@ -40,6 +40,7 @@ Help on both things is given in the linked resources above.
 - `PRODUCTION_DOMAIN`: (optional) Required in order to enable [plausible.io statistics](https://github.com/moritzsternemann/vue-plausible#configuration). In addition to adding it here, the domain (e.g. `auctions.makerdao.network`) should also be registered within [plausible dashboard](https://plausible.io/).
 - `CONTACT_EMAIL`: (optional) Required in order to display contact link in the footer. This email should be able to accept and manage bug reports and other contact requests.
 - `STAGING_BANNER_URL`: (optional) When set a banner will be displayed, warning the user that they are using a staging version. The text will use `STAGING_BANNER_URL` as a link to production UI.
+- `PRODUCTION_BANNER_URL`: (optional) When set a banner will be displayed, notifying the user that they can also use an electron app. The text will use `STAGING_BANNER_URL` as a link the electron app.


2nd sentence probably needs to be adjusted to "Production_Banner_URL"?!

KirillDogadin-std · 2023-02-07T08:59:27Z

Is there a way for me to test locally?!

set the variable in the terminal according to the readme

export PRODUCTION_BANNER_URL='https://asdfalsdjf`
npm run dev

valiafetisov

I wouldn't merge this very noticeable banner on top of every page, until we have updation strategy in place. Otherwise, we will end up with users downloading the app and never updating it (and not even knowing that it's outdated)

LukSteib · 2023-02-07T09:32:09Z

Otherwise, we will end up with users downloading the app and never updating it

Good point. Didn't think about it tbh. So I agree here. Let's keep the PR open for now.

LukSteib · 2023-03-06T15:13:49Z

@valiafetisov can we merge this one as well as the updation strategy got merged?
If we do, is there anything else that needs to be done additionally except for adding the PRODUCTION_BANNER_URLto the chamber?

valiafetisov · 2023-03-06T18:32:10Z

Let's wait for the #589 as well 🙂

Although I wouldn't actually announce electron app via such annoying banner, which kind of suggests to go and install the electron instead of using the web app. I think instead we might want to just add electron somewhere to the main page, close to the regular links [1] or as a separate "block".

[1]

LukSteib · 2023-03-07T08:22:43Z

Although I wouldn't actually announce electron app via such annoying banner, which kind of suggests to go and install the electron instead of using the web app

See your point. I'd suggest to still show the banner but in a way more subtle way by giving it a more decent greyish color.

think instead we might want to just add electron somewhere to the main page, close to the regular links [1] or as a separate "block".

Agree. There is a PR for it already: #593

valiafetisov · 2023-03-07T09:16:19Z

I'd suggest to still show the banner but in a way more subtle way by giving it a more decent greyish color.

Ok, let's do it using warning/info antd Alert, also closable similar to the #589

aomafarag · 2023-03-07T15:49:21Z

Current appearance:

aomafarag · 2023-03-07T16:29:37Z

I had to revert the changes made in SuplusFlow.vue, DebtFlow.vue, etc. manually in order to refactor them by adding a new case for an Alert in ElectronUpdateBanner.vue.

valiafetisov · 2023-03-07T16:30:10Z

frontend/components/layout/ElectronUpdateBanner.vue

+    <Alert v-else-if="status === 'production'" type="info" show-icon banner closable class="text-center">
+        <template #message>
+            You can now run the Unified Auctions UI locally as a desktop application. Please see its release at
+            <a class="underline" :href="url" target="_blank">{{ url }}</a
+            >.
+        </template>
+    </Alert>


What it has to do with the ElectronUpdateBanner or its states? Please move it back to its dedicated component and just use closable Alert

My logic was that it is an electron-related topic, so instead of editing multiple components, just use an existing one that follows the same pattern.

valiafetisov

The code looks good.

Have you checked if the image is correctly built when PRODUCTION_BANNER_URL is provided (locally)? (ie: please check how docker file for STAGING_BANNER_URL)
I don't see the code which passes desired env var from the secrets to the build process in CI (ie: please check how it's done for STAGING_BANNER_URL)
Was the required url added to the production secrets?

valiafetisov · 2023-03-07T17:09:07Z

.drone.yml

+        - key: PRODUCTION_BANNER_URL
+          value_from_secret: auction-ui/main.auction-ui.k8s.sidestream.tech/frontend/production_banner_url


This will only add it to the main.auction-ui.k8s.sidestream.tech (which is staging). You also need to add it down to publish-frontend-makerdao-on-tag step

valiafetisov · 2023-03-07T17:09:23Z

Was the required url added to the production secrets?

I've added it via:

secret write auction-ui/production.auction-ui.k8s.sidestream.tech/frontend PRODUCTION_BANNER_URL https://github.com/sidestream-tech/unified-auctions-ui/releases/latest

Is it a correct path @aomafarag?

aomafarag · 2023-03-07T17:10:03Z

Have you checked if the image is correctly built when PRODUCTION_BANNER_URL is provided (locally)? (ie: please check how docker file for STAGING_BANNER_URL)

Yes, the above screenshot is from local testing.

* [x]  I don't see the code which passes desired env var from the secrets to the build process in CI (ie: please check how it's done for `STAGING_BANNER_URL`)

Added PRODUCTION_BANNER_URL to Dockerfile and .drone.yml with the latest commit.

Was the required url added to the production secrets?

If by this you mean adding it to our secrets storage on AWS, I don't believe I have the privileges to do that.

aomafarag · 2023-03-07T17:11:17Z

Is it a correct path @aomafarag?

The path is correct. However, the variable name should be all lowercase to match our secrets pattern.

feat: Banner for production

a4c290c

wording

1f0fe65

LukSteib requested changes Feb 7, 2023

View reviewed changes

adjust readme

98272f1

LukSteib previously approved these changes Feb 7, 2023

View reviewed changes

valiafetisov reviewed Feb 7, 2023

View reviewed changes

adjust story

3079caa

KirillDogadin-std dismissed LukSteib’s stale review via 3079caa February 7, 2023 09:41

aomafarag and others added 4 commits March 7, 2023 10:21

Merge branch 'main' into banner-electron

ee72532

Revert some changes in preparation for refactoring

14d4ecc

Add new case for production

0a594d4

Remove unnecessary component

4fda81b

valiafetisov reviewed Mar 7, 2023

View reviewed changes

aomafarag added 2 commits March 7, 2023 17:35

Revert commits after ee72532

865292f

Use Alert banner

c61ec4b

valiafetisov reviewed Mar 7, 2023

View reviewed changes

Add env var to Dockerfile and .drone

d491b1c

valiafetisov reviewed Mar 7, 2023

View reviewed changes

add PRODUCTION_BANNER_URL to the publish-frontend-makerdao-on-tag step

93a103c

LukSteib approved these changes Mar 7, 2023

View reviewed changes

valiafetisov approved these changes Mar 7, 2023

View reviewed changes

valiafetisov merged commit 6887469 into main Mar 7, 2023

valiafetisov deleted the banner-electron branch March 7, 2023 17:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Banner for production #583

feat: Banner for production #583

KirillDogadin-std commented Feb 7, 2023 •

edited by valiafetisov

Loading

LukSteib commented Feb 7, 2023 •

edited by KirillDogadin-std

Loading

LukSteib left a comment

LukSteib Feb 7, 2023 •

edited by KirillDogadin-std

Loading

KirillDogadin-std commented Feb 7, 2023

valiafetisov left a comment •

edited

Loading

LukSteib commented Feb 7, 2023

LukSteib commented Mar 6, 2023

valiafetisov commented Mar 6, 2023 •

edited

Loading

LukSteib commented Mar 7, 2023

valiafetisov commented Mar 7, 2023

aomafarag commented Mar 7, 2023

aomafarag commented Mar 7, 2023

valiafetisov Mar 7, 2023

aomafarag Mar 7, 2023

valiafetisov left a comment •

edited by aomafarag

Loading

valiafetisov Mar 7, 2023

valiafetisov commented Mar 7, 2023

aomafarag commented Mar 7, 2023

aomafarag commented Mar 7, 2023

		- key: PRODUCTION_BANNER_URL
		value_from_secret: auction-ui/main.auction-ui.k8s.sidestream.tech/frontend/production_banner_url

feat: Banner for production #583

feat: Banner for production #583

Conversation

KirillDogadin-std commented Feb 7, 2023 • edited by valiafetisov Loading

LukSteib commented Feb 7, 2023 • edited by KirillDogadin-std Loading

LukSteib left a comment

Choose a reason for hiding this comment

LukSteib Feb 7, 2023 • edited by KirillDogadin-std Loading

Choose a reason for hiding this comment

KirillDogadin-std commented Feb 7, 2023

valiafetisov left a comment • edited Loading

Choose a reason for hiding this comment

LukSteib commented Feb 7, 2023

LukSteib commented Mar 6, 2023

valiafetisov commented Mar 6, 2023 • edited Loading

LukSteib commented Mar 7, 2023

valiafetisov commented Mar 7, 2023

aomafarag commented Mar 7, 2023

Current appearance:

aomafarag commented Mar 7, 2023

valiafetisov Mar 7, 2023

Choose a reason for hiding this comment

aomafarag Mar 7, 2023

Choose a reason for hiding this comment

valiafetisov left a comment • edited by aomafarag Loading

Choose a reason for hiding this comment

valiafetisov Mar 7, 2023

Choose a reason for hiding this comment

valiafetisov commented Mar 7, 2023

aomafarag commented Mar 7, 2023

aomafarag commented Mar 7, 2023

KirillDogadin-std commented Feb 7, 2023 •

edited by valiafetisov

Loading

LukSteib commented Feb 7, 2023 •

edited by KirillDogadin-std

Loading

LukSteib Feb 7, 2023 •

edited by KirillDogadin-std

Loading

valiafetisov left a comment •

edited

Loading

valiafetisov commented Mar 6, 2023 •

edited

Loading

valiafetisov left a comment •

edited by aomafarag

Loading