Skip to content

fix(grouping): Make derived rules visible to users#109836

Merged
thetruecpaul merged 1 commit intomasterfrom
cpaul/030326/oncall-derived-group-su
Mar 3, 2026
Merged

fix(grouping): Make derived rules visible to users#109836
thetruecpaul merged 1 commit intomasterfrom
cpaul/030326/oncall-derived-group-su

Conversation

@thetruecpaul
Copy link
Contributor

confusing for users to hide this

confusing for users to hide this
@thetruecpaul thetruecpaul requested review from a team, armenzg and lobsterkatie March 3, 2026 21:31
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 3, 2026
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Misleading test name and dead mock after ungating
    • I renamed the test to reflect ungated behavior and removed the unused superuser mock/re-render path so the test now only asserts the intended always-visible section.

Create PR

Or push these changes by commenting:

@cursor push f3e4991d70
Preview (f3e4991d70)
diff --git a/static/app/views/settings/projectIssueGrouping/index.spec.tsx b/static/app/views/settings/projectIssueGrouping/index.spec.tsx
--- a/static/app/views/settings/projectIssueGrouping/index.spec.tsx
+++ b/static/app/views/settings/projectIssueGrouping/index.spec.tsx
@@ -1,13 +1,8 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
-import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';
 import ProjectIssueGrouping from 'sentry/views/settings/projectIssueGrouping';
 
-jest.mock('sentry/utils/isActiveSuperuser', () => ({
-  isActiveSuperuser: jest.fn(),
-}));
-
 describe('projectIssueGrouping', () => {
   const {organization, projects} = initializeOrg();
   const project = projects[0]!;
@@ -47,34 +42,21 @@
     ).toBeInTheDocument();
   });
 
-  it('shows derived grouping enhancements only for superusers', async () => {
-    // Mock the API response
+  it('shows derived grouping enhancements', async () => {
     MockApiClient.addMockResponse({
       url: `/projects/${organization.slug}/${project.slug}/grouping-configs/`,
       body: [],
     });
 
-    // First render with a non-superuser
-    const {rerender} = render(<ProjectIssueGrouping />, {
+    render(<ProjectIssueGrouping />, {
       organization,
       outletContext: {project},
     });
 
-    // Verify the section is visible for non-superuser
     expect(await screen.findByText('Issue Grouping')).toBeInTheDocument();
     expect(screen.getByText(/Derived Grouping Enhancements/)).toBeInTheDocument();
     expect(
       screen.getByRole('textbox', {name: /Derived Grouping Enhancements/})
     ).toBeDisabled();
-
-    // Re-render for superuser
-    jest.mocked(isActiveSuperuser).mockReturnValue(true);
-    rerender(<ProjectIssueGrouping />);
-
-    // Verify the section is visible for superuser
-    expect(screen.getByText(/Derived Grouping Enhancements/)).toBeInTheDocument();
-    expect(
-      screen.getByRole('textbox', {name: /Derived Grouping Enhancements/})
-    ).toBeDisabled();
   });
 });
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

expect(screen.getByText(/Derived Grouping Enhancements/)).toBeInTheDocument();
expect(
screen.getByRole('textbox', {name: /Derived Grouping Enhancements/})
).toBeDisabled();
Copy link
Contributor

Choose a reason for hiding this comment

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

Misleading test name and dead mock after ungating

Low Severity

The test is still named 'shows derived grouping enhancements only for superusers', which directly contradicts the PR's intent to make the section visible to all users. Additionally, jest.mocked(isActiveSuperuser).mockReturnValue(true) at line 71 is now a no-op — the component no longer imports or uses isActiveSuperuser, so the mock has zero effect on rendering. This leaves the test asserting the same thing twice under a misleading name, giving false confidence that superuser-gating is being tested when it isn't.

Fix in Cursor Fix in Web

@thetruecpaul thetruecpaul merged commit b7c6f55 into master Mar 3, 2026
62 checks passed
@thetruecpaul thetruecpaul deleted the cpaul/030326/oncall-derived-group-su branch March 3, 2026 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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