Skip to content

Comments

feat: add e2e test for insights access without insights.read permission#23948

Closed
eunjae-lee wants to merge 1 commit intomainfrom
devin/insights-pbac-e2e-test-1758289127
Closed

feat: add e2e test for insights access without insights.read permission#23948
eunjae-lee wants to merge 1 commit intomainfrom
devin/insights-pbac-e2e-test-1758289127

Conversation

@eunjae-lee
Copy link
Contributor

What does this PR do?

This PR adds an end-to-end test to validate the changes in PR #23945, which removed page-level permission checks for the /insights page. The test verifies that users with custom roles lacking the insights.read permission can still access the insights page without being redirected.

Key changes:

  • Adds a new e2e test that creates an organization with PBAC enabled, assigns a user a custom role without insights.read permission, and verifies they can access /insights
  • Includes helper functions for PBAC setup in e2e fixtures (though unused in the final implementation)
  • Fixes existing lint violations in the users fixture by replacing include: { AnyPropertyName: true } patterns with explicit select statements

How should this be tested?

Environment setup:

  • Ensure test database schema is up-to-date (previous testing showed missing showOptimizedSlots column issues)
  • No special environment variables required beyond standard test setup

Test execution:

TZ=UTC PLAYWRIGHT_HEADLESS=1 yarn e2e insights.e2e.ts

Expected behavior:

  • Test should create organization with PBAC enabled
  • User with custom role (lacking insights.read) should successfully access /insights page
  • Page should load insights filters without redirect to / or /event-types

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A - This only adds tests
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Critical Review Points

⚠️ Important items for reviewer attention:

  1. Test environment compatibility: Local testing failed due to database schema issues. Please verify the test runs successfully in CI.

  2. Lint fixes scope: This PR includes fixes to existing lint violations in users.ts that change Prisma queries from include: { field: true } to explicit select statements. Please verify these changes don't break existing functionality.

  3. PBAC setup validation: The test creates a complex PBAC setup with organizations, custom roles, and feature flags. Please verify the setup correctly represents the intended test scenario.

  4. Unused helper functions: The PR includes helper functions createCustomRoleWithoutInsights and enablePBACForTeam that aren't used in the final test. Consider if these should be removed or if the test should use them.


Requested by: @eunjae-lee
Devin session: https://app.devin.ai/sessions/a3a4948c7ffa4420b7dd312de455da2e

- Add helper functions for PBAC setup in e2e fixtures
- Add test verifying users with custom roles lacking insights.read can access /insights
- Test validates that PR #23945 correctly removes page-level permission checks
- Fix existing lint violations in users.ts fixture file

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 19, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/insights-pbac-e2e-test-1758289127

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants