Hide analytics dashboards from guest
What does this MR do and why?
This MR has 2 commits, each one explained below. I recommend we merge without squashing.
1. Simplify Analytics Dashboard tests
Analytics Dashboard
feature tests had different context
blocks based on several feature flag values and then calling shared product analytics dashboards
tests. However, those shared examples already testing the same scenarios. So I cleaned them up before making changes to the access requirement.
2. Hide analytics dashboards unless has permission
Analytics dashboards in public projects are currently accessible to any user (including guest). However, they should only be visible to users who have at least developer
role in the project. This is fixed with this MR.
Changes
- Return 404 if people without at least developer role visits analytics dashboards page
- Hide analytics dashboard menu item in the side from users without at least a developer role
Note: A changelog trailer is not added as the feature is behind a flag.
Screenshots or screen recordings
The following screenshots are taken as a not logged in user in a public project.
Before | After |
---|---|
![]() |
![]() |
![]() |
![]() |
How to set up and validate locally
- Create a public project
- Log out and visit http://gdk.test:3000/[namespace]/[project]/-/analytics/dashboards
- You should get 404.
- Visit project page, in the sidebar you should not see 'Application Analytics' or 'Dashboards' under 'Analyze'
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #413980 (closed)
Merge request reports
Activity
changed milestone to %16.2
assigned to @halilcoban
added 1 commit
- c0c547cb - Hide analytics dashboards unless has permission
2 Messages CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge. Documentation review
The following files require a review from a technical writer:
-
doc/user/analytics/analytics_dashboards.md
(Link to current live version)
The review does not need to block merging this merge request. See the:
-
Metadata for the
*.md
files that you've changed. The first few lines of each*.md
file identify the stage and group most closely associated with your docs change. - The Technical Writer assigned for that stage and group.
- Documentation workflows for information on when to assign a merge request for review.
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Gavin Hinfey (
@ghinfey
) (UTC+1, 1 hour behind@halilcoban
)Kassio Borges (
@kassio
) (UTC+1, 1 hour behind@halilcoban
)test for spec/features/*
Gavin Hinfey (
@ghinfey
) (UTC+1, 1 hour behind@halilcoban
)Maintainer review is optional for test for spec/features/*
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Danger-
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
Allure report
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for e116e82bexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Data Stores | 20 | 0 | 0 | 15 | 20 | ❗ | | Plan | 47 | 0 | 0 | 41 | 47 | ❗ | | Manage | 12 | 0 | 1 | 12 | 13 | ❗ | | Create | 19 | 0 | 0 | 18 | 19 | ❗ | | Govern | 19 | 0 | 0 | 18 | 19 | ❗ | | Verify | 8 | 0 | 0 | 8 | 8 | ❗ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 125 | 0 | 1 | 112 | 126 | ❗ | +-------------+--------+--------+---------+-------+-------+--------+
- A deleted user
added documentation label
added 1 commit
- 2b60acc4 - Hide analytics dashboards unless has permission
added 102 commits
-
2b60acc4...66977723 - 100 commits from branch
master
- c8970d9e - Simplify Analytics Dashboard tests
- 3c9e944e - Hide analytics dashboards unless has permission
-
2b60acc4...66977723 - 100 commits from branch
This merge request changes the behavior of the analytics dashboards so that they are no longer visible to guests. This is done by adding a check to the
dashboards_analytics_menu_item
method to ensure that the user has theread_combined_project_analytics_dashboards
andread_product_analytics
permissions.
(AI-generated summary for revision 3c9e944e)
- Resolved by Luke Duncalfe
@jwoodwardgl can you please make the initial review for backend and test?
requested review from @jwoodwardgl
added 1 commit
- 7224e9d6 - Hide analytics dashboards unless has permission