Hide merge requests from banned users
This feature was originally implemented here, but was reverted, because of an incident with the without_hidden
scope. That scope, which was shared with the hidden issues feature, as it turned out, was already under investigation because of it's slowness when finding Issues
.
Since the revert, the without_hidden
scope was changed. This MR applies the same changes to MergeRequest
and puts all hidden MR logic behind it's own feature flag, instead of re-using the same feature flag for hidden issues, which was the reason for the revert.
I think this resolves the issues with the slow query, as the original query caused the query plan to flip and the changed query proved to be efficient with issues.
What does this MR do and why?
When the hide_merge_requests_from_banned_users
feature flag is enabled and a user is banned, merge requests authored by that user are hidden, except for auditors and admins.
When logged in as auditor or admin, hidden merge requests (and issues) remain visible, with a spam icon to highlight their status.
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/375964
Epic: &5741
Database
This adds the MergeRequest.without_hidden
scope that is applied when listing merge requests and counting merge requests statuses.
Listing MR's
This is the query with the feature flag disabled (explained):
Click to expand
SELECT
"merge_requests".*
FROM
"merge_requests"
WHERE
"merge_requests"."target_project_id" = 278964
AND("merge_requests"."state_id" IN(1))
ORDER BY
"merge_requests"."created_at" DESC,
"merge_requests"."id" DESC
LIMIT 20 OFFSET 0
This is the query with the feature flag enabled (explained):
Click to expand
SELECT
"merge_requests".*
FROM
"merge_requests"
WHERE
(
NOT EXISTS (
SELECT
1
FROM
"banned_users"
WHERE
(banned_users.user_id = merge_requests.author_id)
)
)
AND "merge_requests"."target_project_id" = 278964
AND("merge_requests"."state_id" IN(1))
ORDER BY
"merge_requests"."created_at" DESC,
"merge_requests"."id" DESC
LIMIT
20 OFFSET 0
Counting MR's while filtering
This is the query with the feature flag disabled (explained):
Click to expand
WITH "merge_requests" AS MATERIALIZED (
SELECT
"merge_requests".*
FROM
"merge_requests"
WHERE
"merge_requests"."target_project_id" = 278964
)
SELECT
COUNT(*) AS count_all,
"merge_requests"."state_id" AS merge_requests_state_id
FROM
merge_requests
WHERE
(
"merge_requests"."title" ILIKE '%cache%'
OR "merge_requests"."description" ILIKE '%cache%'
)
GROUP BY
"merge_requests"."state_id"
This is the query with the feature flag enabled (explained):
Click to expand
WITH "merge_requests" AS MATERIALIZED (
SELECT
"merge_requests".*
FROM
"merge_requests"
WHERE
(
NOT EXISTS (
SELECT
1
FROM
"banned_users"
WHERE
(banned_users.user_id = merge_requests.author_id)
)
)
AND "merge_requests"."target_project_id" = 278964
)
SELECT
COUNT(*) AS count_all,
"merge_requests"."state_id" AS merge_requests_state_id
FROM
merge_requests
WHERE
(
"merge_requests"."title" ILIKE '%cache%'
OR "merge_requests"."description" ILIKE '%cache%'
)
GROUP BY
"merge_requests"."state_id"
Screenshots or screen recordings
Index | Show | |
---|---|---|
Logged in as admin | ||
Logged in as guest |
How to set up and validate locally
- Enable the feature flag in Rails console:
Feature.enable(:hide_merge_requests_from_banned_users)
- Create a public project.
- Impersonate a user and create a public merge request in the project.
- Stop impersonation.
- Ban the user (user's page > Settings > Ban user)
- View the project page as an admin, and as a guest.
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.