-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix(charts list): do not trigger ListViewError exception for anonymous user #20171
fix(charts list): do not trigger ListViewError exception for anonymous user #20171
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20171 +/- ##
==========================================
- Coverage 66.46% 66.43% -0.03%
==========================================
Files 1721 1723 +2
Lines 64521 64627 +106
Branches 6811 6829 +18
==========================================
+ Hits 42882 42935 +53
- Misses 19906 19960 +54
+ Partials 1733 1732 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and leave one nit.
userId, | ||
refreshData, | ||
saveFavoriteStatus, | ||
// userKey, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this unused code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I tested the chart list successfully (connected and anonymously); initial commit was amended minus the above chunk: 0bb05c4
8ffa4eb
to
0bb05c4
Compare
SUMMARY
This PR is a follow up for issue #18210 which fixes charts list, while the first PR #19409 fixes the dashboards list.
Note well: this PR is a copy/past of @dudasaron PR #19409 for dashboards list, for which, I must admit, I don't master each changes...
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
After
TESTING INSTRUCTIONS
Was tested on current master (b746e6f): visit the charts list as anonymous user...
ADDITIONAL INFORMATION