feat: Include individual settings snapshots #33809
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rafaeelaudibert
commented
Jun 17, 2025
2c9951c to
56baa47
Compare
We consistently regenerate our settings snapshots and they're very hard to compare, let's instead create individual snapshots for each of our sections. There's a fair amount of boilerplate, but I've added an `_ALL_SECTIONS_CHECKER` constant to guarantee we're always including new sections. It's unfortunately very hard to make this work dynamically. I tried a couple approaches already but they all failed: `storiesOf`, `Indexer` API, etc. When upgrading to Storybook v8 we should check whether there's an easier way to do this
This used to be covered by snapshot tests which isnt ideal, let's instead test it for real using tests
56baa47 to
36a66ac
Compare
|
Size Change: 0 B Total Size: 2.57 MB ℹ️ View Unchanged
|
They'd be useful, but I dont know enough about kea, kea tests and this logic to get them to pass
There was a problem hiding this comment.
PR Summary
Refactors Storybook settings snapshots to use individual section stories instead of a monolithic snapshot, improving maintainability and diff readability.
- Added new
frontend/src/scenes/settings/SettingsScene.stories.tsxwith granular stories for each settings section and type-safe_ALL_SECTIONS_CHECKERconstant - Added comprehensive test suite in
frontend/src/lib/components/TimeSensitiveAuthentication/timeSensitiveAuthenticationLogic.test.ts - Cleaned up
frontend/src/scenes/settings/types.tsby removing deprecated sections like 'environment-role-based-access-control' - Extended mock handlers in
frontend/src/mocks/handlers.tsto support error tracking settings snapshots
4 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
...tend/src/lib/components/TimeSensitiveAuthentication/timeSensitiveAuthenticationLogic.test.ts
Show resolved
Hide resolved
rafaeelaudibert
commented
Jun 17, 2025
Comment on lines
+208
to
+210
| 'api/environments/:team_id/error_tracking/assignment_rules': EMPTY_PAGINATED_RESPONSE, | ||
| 'api/environments/:team_id/error_tracking/grouping_rules': EMPTY_PAGINATED_RESPONSE, | ||
| 'api/environments/:team_id/error_tracking/suppression_rules': EMPTY_PAGINATED_RESPONSE, |
There was a problem hiding this comment.
BTW, this is what was causing the tests to fail, but it was impossible to learn that was the case without the changes I did here ✨
Twixes
reviewed
Jun 18, 2025
| SettingsUserDangerZone.args = { sectionId: 'user-danger-zone' } | ||
|
|
||
| // NOTE: This is used to guarantee we're testing all sections | ||
| const _ALL_SECTIONS_CHECKER: Record<SettingSectionId, Story> = { |
Comment on lines
+213
to
+215
| // NOTE: This is here to avoid TS complaining | ||
| // about us not using the variable used above to check all sections | ||
| ;(() => _ALL_SECTIONS_CHECKER)() |
There was a problem hiding this comment.
Why not use a // @ts-expect-error comment instead?
There was a problem hiding this comment.
Because I wanna check with Typescript that we have a snapshot for every possible setting. If I disable Typescript to avoid the "umused variable" warning, it also defeats the purpose of using Typescript to check for full coverage.
I would love a way to disable a single TS rule rather than all or nothing
Twixes
approved these changes
Jun 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We consistently regenerate our settings snapshots and they're very hard to compare, let's instead create individual snapshots for each of our sections.
There's a fair amount of boilerplate, but I've added an
_ALL_SECTIONS_CHECKERconstant to guarantee we're always including new sections.It's unfortunately very hard to make this work dynamically. I tried a couple approaches already but they all failed:
storiesOf,IndexerAPI, etc. When upgrading to Storybook v8 we should check whether there's an easier way to do this