RedHatInsights / insights-chrome

Chroming for Insights apps
MIT License
27 stars 130 forks source link

End preview #2835

Closed Hyperkid123 closed 2 months ago

Hyperkid123 commented 4 months ago

tickets:

Changes

@catastrophe-brandon This PR should introduce the new preview feature if the local storage is present. The current preview functionality should be unaffected.

To anyone who will be looking at the PR. This must be properly tested/verified. We don't want to block the Chrome UI deployment. The local storage gating must be 100% reliable.

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 55.91398% with 82 lines in your changes missing coverage. Please review.

Project coverage is 63.01%. Comparing base (24026e7) to head (678467e).

:exclamation: Current head 678467e differs from pull request most recent head e418376

Please upload reports for the commit e418376 to get more accurate results.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835/graphs/tree.svg?width=650&height=150&src=pr&token=GuRwyW1uUf&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights)](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights) ```diff @@ Coverage Diff @@ ## master #2835 +/- ## ========================================== - Coverage 63.66% 63.01% -0.66% ========================================== Files 207 210 +3 Lines 4503 4607 +104 Branches 845 856 +11 ========================================== + Hits 2867 2903 +36 - Misses 1625 1692 +67 - Partials 11 12 +1 ``` | [Files](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights) | Coverage Δ | | |---|---|---| | [src/auth/OIDCConnector/OIDCSecured.tsx](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fauth%2FOIDCConnector%2FOIDCSecured.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2F1dGgvT0lEQ0Nvbm5lY3Rvci9PSURDU2VjdXJlZC50c3g=) | `66.66% <ø> (+1.11%)` | :arrow_up: | | [src/auth/OIDCConnector/utils.ts](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fauth%2FOIDCConnector%2Futils.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2F1dGgvT0lEQ0Nvbm5lY3Rvci91dGlscy50cw==) | `60.97% <ø> (ø)` | | | [...c/components/FeatureFlags/FeatureFlagsProvider.tsx](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fcomponents%2FFeatureFlags%2FFeatureFlagsProvider.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2NvbXBvbmVudHMvRmVhdHVyZUZsYWdzL0ZlYXR1cmVGbGFnc1Byb3ZpZGVyLnRzeA==) | `100.00% <100.00%> (+16.00%)` | :arrow_up: | | [src/components/Header/ToolbarToggle.tsx](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fcomponents%2FHeader%2FToolbarToggle.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2NvbXBvbmVudHMvSGVhZGVyL1Rvb2xiYXJUb2dnbGUudHN4) | `50.00% <100.00%> (+2.63%)` | :arrow_up: | | [src/components/Navigation/ChromeNavItem.tsx](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fcomponents%2FNavigation%2FChromeNavItem.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2NvbXBvbmVudHMvTmF2aWdhdGlvbi9DaHJvbWVOYXZJdGVtLnRzeA==) | `89.47% <100.00%> (+0.58%)` | :arrow_up: | | [src/components/RootApp/ScalprumRoot.tsx](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fcomponents%2FRootApp%2FScalprumRoot.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2NvbXBvbmVudHMvUm9vdEFwcC9TY2FscHJ1bVJvb3QudHN4) | `75.64% <100.00%> (+0.31%)` | :arrow_up: | | [src/hooks/useAllLinks.ts](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fhooks%2FuseAllLinks.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2hvb2tzL3VzZUFsbExpbmtzLnRz) | `62.71% <100.00%> (-21.22%)` | :arrow_down: | | [src/hooks/useFavoritedServices.ts](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fhooks%2FuseFavoritedServices.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL2hvb2tzL3VzZUZhdm9yaXRlZFNlcnZpY2VzLnRz) | `85.41% <100.00%> (+0.97%)` | :arrow_up: | | [src/state/atoms/scalprumConfigAtom.ts](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fstate%2Fatoms%2FscalprumConfigAtom.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL3N0YXRlL2F0b21zL3NjYWxwcnVtQ29uZmlnQXRvbS50cw==) | `66.66% <100.00%> (+16.66%)` | :arrow_up: | | [src/state/atoms/utils.ts](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree&filepath=src%2Fstate%2Fatoms%2Futils.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#diff-c3JjL3N0YXRlL2F0b21zL3V0aWxzLnRz) | `100.00% <100.00%> (ø)` | | | ... and [24 more](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights) | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/RedHatInsights/insights-chrome/pull/2835/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights)
florkbr commented 3 months ago

@Hyperkid123 upon checking out the latest commit in this PR I'm seeing quite a few various errors trying to run locally:

I'm not sure if these are actually related to your PR... I will continue debugging locally.

EDIT: @Hyperkid123 nevermind - awhile back I was overriding network calls in my browsers and forgot to clear the overrides. Sorry for the noise. I'll keep testing now.

Hyperkid123 commented 3 months ago

Hmm, interesting. I have not experienced these. Were you just clicking around the UI? Any steps to reproduce on my side?

florkbr commented 3 months ago

@Hyperkid123 with your latest commit I see the preview notification sometimes flash with the old state before the page reloads (without the local storage flag enabled):

preview-dialog-flash

It doesn't happen all the time, but if I stay on the page for a moment and open/close the all-services menu it seems to happen. It can be confusing as when I click to enable preview is briefly shows "preview disabled" before reloading the page, or vice versa.

FWIW I'm unable to reproduce this on the UI running on stage today - just this PR.

florkbr commented 3 months ago

Otherwise, I have been trying things in the UI with localStorage.setItem('chrome:local-preview', true); and so far so good

Hyperkid123 commented 3 months ago

@Hyperkid123 with your latest commit I see the preview notification sometimes flash with the old state before the page reloads (without the local storage flag enabled):

@florkbr This is because of the mix of state and localstorage. I'll be honest. I don't care enough about the flash. It will be fixed by ending the old preview.

Hyperkid123 commented 2 months ago

/retest

Hyperkid123 commented 2 months ago

/retest

Hyperkid123 commented 2 months ago

What a perfect test run that fails on some OS random error:

[2024-06-19T07:53:54.845Z] [303:0619/075223.845745:ERROR:object_proxy.cc(590)] Failed to call method: org.freedesktop.portal.Settings.Read: object_path= /org/freedesktop/portal/desktop: unknown error type: 
Hyperkid123 commented 2 months ago

/retest