search

microbiomedata / nmdc-server

Data portal client and server for NMDC.
https://data.microbiomedata.org
Other
9 stars 0 forks source link

Remove Berkeley update banner from data and submission portals #1421

Closed naglepuff closed 2 weeks ago

naglepuff commented 1 month ago

Also consolidates all of the boolean checks into one place (AppBanner.vue).

naglepuff commented 1 month ago

I created this branch off of v1.0.0, which, at the time of writing, is at the same commit as main. Once this is approved, I can initiate a hotfix to get these changes into dev.

eecavanna commented 3 weeks ago

I have closed PR https://github.com/microbiomedata/nmdc-server/pull/1426, which only hid the banner and did not consolidate the recurring booleans. I want this PR (#1421) to be merged into main instead.

eecavanna commented 3 weeks ago

On second thought, I will merge that other PR into main first; since it's been reviewed/approved. I still want the consolidation that this one contains, to be applied to main eventually.

naglepuff commented 3 weeks ago

@eecavanna Good thought, I agree. I can either rebase this onto main once your PR is merged, or I could open up an issue to track consolidating the boolean. I'm also open to using this PR to add the desired functionality where the banner message is driven by an env var instead of being hard-coded and managed through hotfixes.

There's also the question of keeping the v-if statements as-is, in the case that a banner should be applied to some views but not all. I'm not sure of a use case or if the extra maintenance is worth it, I just though I'd mention that since it crossed my mind.

eecavanna commented 2 weeks ago

(I'm catching up after being OOO)

The diff still looks good to me and I'm comfortable with this being merged in whenever @naglepuff considers it done. I am OK with the "further" goals being left for a separate PR (e.g. introducing an environment variable or having the client fetch the banner data from the server at run time).

Regarding keeping the v-if statements in the individual banner instances, I'd prefer leaving them out at this point. We can add them in if we find we need them (I don't think the need has arisen in the 2-3 events where we have put up banners so far).