Skip to content

feat(insights): show update SDK banner cache module #72015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 4, 2024

Conversation

DominikB2014
Copy link
Contributor

  1. If the cache hit miss rate chart aren't showing data, this likely means the customer is on an outdated SDK or they aren't sending cache.hit, so we display a message accordingly
    image

  2. The message is not displayed if it is in an onboarding state

@DominikB2014 DominikB2014 requested a review from a team as a code owner June 4, 2024 16:16
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 4, 2024
Copy link

codecov bot commented Jun 4, 2024

Codecov Report

Attention: Patch coverage is 7.14286% with 13 lines in your changes missing coverage. Please review.

Project coverage is 77.91%. Comparing base (f0b4d91) to head (6615ed1).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #72015      +/-   ##
==========================================
- Coverage   77.94%   77.91%   -0.03%     
==========================================
  Files        6571     6572       +1     
  Lines      292615   292719     +104     
  Branches    50523    50532       +9     
==========================================
- Hits       228066   228065       -1     
- Misses      58304    58410     +106     
+ Partials     6245     6244       -1     
Files Coverage Δ
...tatic/app/utils/performance/contexts/pageAlert.tsx 60.00% <100.00%> (ø)
...c/app/views/performance/cache/cacheLandingPage.tsx 0.00% <0.00%> (ø)

... and 24 files with indirect coverage changes

@DominikB2014 DominikB2014 enabled auto-merge (squash) June 4, 2024 19:48
@0Calories
Copy link
Member

Should we also display an alert banner in the case when there's no data found?

image

Copy link
Member

@0Calories 0Calories left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DominikB2014 DominikB2014 merged commit 0abe65b into master Jun 4, 2024
42 of 43 checks passed
@DominikB2014 DominikB2014 deleted the DominikB2014/add-sdk-update-banner branch June 4, 2024 20:20
@0Calories
Copy link
Member

I'd recommend some test coverage for this too since there's a bit of branching logic and we do slightly finicky things like a conditional on an exact error message

@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants