Skip to content

Hide merge requests from banned users

Alex Buijs requested to merge hide-merge-requests-from-banned-users into master

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 Screenshot_2022-12-06_at_15.48.38 Screenshot_2022-12-06_at_15.48.45
Logged in as guest Screenshot_2022-12-06_at_15.48.09 Screenshot_2022-12-06_at_15.48.25

How to set up and validate locally

  1. Enable the feature flag in Rails console:
Feature.enable(:hide_merge_requests_from_banned_users)
  1. Create a public project.
  2. Impersonate a user and create a public merge request in the project.
  3. Stop impersonation.
  4. Ban the user (user's page > Settings > Ban user)
  5. 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.

Edited by Alex Buijs

Merge request reports