feat(notifications): Add Size Analysis to notification settings UI#106940
feat(notifications): Add Size Analysis to notification settings UI#106940
Conversation
cb142ff to
3d8a691
Compare
static/app/views/settings/account/notifications/notificationSettingsByType.tsx
Outdated
Show resolved
Hide resolved
332f050 to
a5c10b0
Compare
25fe0ab to
bfd96a1
Compare
- Set isBilledCategory: true for SIZE_ANALYSIS in DATA_CATEGORY_INFO - Add expose-category-size-analysis feature flag filtering in filterCategoryFields() - Gate quotaSizeAnalyses notification setting behind feature flag Closes BIL-2007
…tions Add the missing reservedSizeAnalyses property to all Reservations objects to fix TypeScript compilation errors in CI. This property was added to the Reservations type but was missing in several test files and implementation. Fixes: - productSelectionAvailability.spec.tsx - productUnavailableCTA.spec.tsx - usePreviewData.spec.tsx - useUpgradeNowParams.tsx
…ctations Update test expectations to include the reservedSizeAnalyses property that was added to the Reservations type in the implementation.
…frontend The backend notification setting used singular form `quotaSizeAnalysis` but the frontend DataCategory convention generates plural form `quotaSizeAnalyses`. This mismatch caused the Size Analysis notification setting to be non-functional. Changed backend from `quotaSizeAnalysis` to `quotaSizeAnalyses` to match the frontend convention and be consistent with similar settings like `quotaSeerUsers`.
…xture The am2Plans fixture returns sizeAnalyses with events: 100, so the test expectations should be 100, not undefined.
… RPC API The change from `quotaSizeAnalysis` to `quotaSizeAnalyses` was flagged as a breaking change by the openapi-diff CI check since the production RPC schema already contains the original value. Reverted to maintain backwards compatibility.
bfd96a1 to
6fed872
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| } | ||
| if (field.name.startsWith('quotaSizeAnalysis') && !includeSizeAnalysis) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Feature flag filter uses wrong field name spelling
High Severity
The filter checks for field.name.startsWith('quotaSizeAnalysis') (singular spelling with 'i'), but the actual field name generated in fields2.tsx is quotaSizeAnalyses (plural spelling with 'e'). Since "analyses" doesn't start with "analysis" (they differ at the 'e' vs 'i'), this filter will never match. The Size Analysis notification setting will appear for all users regardless of the expose-category-size-analysis feature flag being enabled.
## Summary Fixes a bug introduced in PR #106940 where the Size Analysis notification setting was appearing for all users regardless of the `expose-category-size-analysis` feature flag. The issue was a typo in the feature flag filter: - Filter checked for `quotaSizeAnalysis` (singular, with 'i') - Actual field name is `quotaSizeAnalyses` (plural, with 'e') Since `'quotaSizeAnalyses'.startsWith('quotaSizeAnalysis')` returns `false`, the filter never matched. Simplified the filter to `quotaSize` to prevent similar typos in the future. ## Test plan - [x] Existing notification settings tests pass (13 tests) - [x] Pre-commit hooks pass
| if (field.name.startsWith('quotaSizeAnalysis') && !includeSizeAnalysis) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Bug: The frontend generates the field name 'quotaSizeAnalyses' (plural) for the Size Analysis notification, but the backend expects 'quotaSizeAnalysis' (singular), causing save attempts to fail.
Severity: MEDIUM
Suggested Fix
Align the frontend and backend field names. Either update the backend's NotificationSettingEnum in types.py to accept the plural form 'quotaSizeAnalyses', or change the frontend's field name generation in fields2.tsx to produce the singular form 'quotaSizeAnalysis'. Given the history, ensure the chosen fix does not break backward compatibility with the RPC API.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
static/app/views/settings/account/notifications/notificationSettingsByType.tsx#L253-L255
Potential issue: There is a mismatch between the frontend and backend for the Size
Analysis notification setting's field name. The frontend generates the field name as
`'quotaSizeAnalyses'` based on the `plural` property of the data category. However, the
backend's `NotificationSettingEnum` only recognizes the singular form,
`'quotaSizeAnalysis'`. When a user attempts to save this setting, the frontend sends the
pluralized name, which the backend's `validate_type()` function rejects, resulting in a
validation error. This makes the Size Analysis notification setting visible but
unsaveable in the UI.
…106940) closes https://linear.app/getsentry/issue/BIL-2007/add-size-analysis-to-notification-settings-ui-frontend ## Summary Enable the Size Analysis quota notification setting to appear in the user notification settings UI, gated behind the `expose-category-size-analysis` feature flag. - Set `isBilledCategory: true` for SIZE_ANALYSIS in `DATA_CATEGORY_INFO` - Add `expose-category-size-analysis` feature flag filtering in `filterCategoryFields()` - Gate `quotaSizeAnalyses` notification setting behind feature flag ## Dependencies - Requires [PR #106939](#106939) (BIL-1918) to be merged first ## Test plan - [x] Existing notification settings tests pass (18 tests) - [x] Pre-commit checks pass - [ ] Manual verification that Size Analysis appears in notification settings only when feature flag is enabled
## Summary Fixes a bug introduced in PR #106940 where the Size Analysis notification setting was appearing for all users regardless of the `expose-category-size-analysis` feature flag. The issue was a typo in the feature flag filter: - Filter checked for `quotaSizeAnalysis` (singular, with 'i') - Actual field name is `quotaSizeAnalyses` (plural, with 'e') Since `'quotaSizeAnalyses'.startsWith('quotaSizeAnalysis')` returns `false`, the filter never matched. Simplified the filter to `quotaSize` to prevent similar typos in the future. ## Test plan - [x] Existing notification settings tests pass (13 tests) - [x] Pre-commit hooks pass


closes https://linear.app/getsentry/issue/BIL-2007/add-size-analysis-to-notification-settings-ui-frontend
Summary
Enable the Size Analysis quota notification setting to appear in the user notification settings UI, gated behind the
expose-category-size-analysisfeature flag.isBilledCategory: truefor SIZE_ANALYSIS inDATA_CATEGORY_INFOexpose-category-size-analysisfeature flag filtering infilterCategoryFields()quotaSizeAnalysesnotification setting behind feature flagDependencies
Test plan