Skip to content

Status checks widget shouldn't appear if no status checks match the branch

Robert Hunt requested to merge 334242-status-checks-widget-shouldn-t-appear-if-no-status-checks-match-the-branch into master

What does this MR do?

This MR stops the status checks widget appearing in an MR if there are no status checks applicable to the branch the MR is targeting. This means that if a status check has been selected to be applied to all branches (Any branch option) or has the same branch as the MRs target branch; then it will be shown in the MR under the MRs status checks widget. Otherwise, the widget will now appear at all - as originally intended.

Screenshots (strongly suggested)

(Compliance using the MR's target branch/Any branch and Requirements check using an unmatchable branch)

Status checks Before After
No matching branch image image
Any branch applied image image
Matching branch image image

Database

Original query

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/4746/commands/16903

SELECT
    1 AS ONE
FROM
    external_status_checks
WHERE
    external_status_checks.project_id = 1
LIMIT 1;
Query plan

https://explain.depesz.com/s/dqV1#html

Limit  (cost=0.15..2.17 rows=1 width=4) (actual time=0.069..0.069 rows=0 loops=1)
   Buffers: shared hit=5
   I/O Timings: read=0.000 write=0.000
   ->  Index Only Scan using idx_on_external_status_checks_project_id_external_url on public.external_status_checks  (cost=0.15..6.20 rows=3 width=4) (actual time=0.067..0.067 rows=0 loops=1)
         Index Cond: (external_status_checks.project_id = 1)
         Heap Fetches: 0
         Buffers: shared hit=5
         I/O Timings: read=0.000 write=0.000

New query

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/4746/commands/16909

SELECT
    1 AS ONE
FROM
    external_status_checks
LEFT OUTER JOIN
    external_status_checks_protected_branches ON external_status_checks_protected_branches.external_status_check_id = external_status_checks.id
LEFT OUTER JOIN 
    protected_branches ON protected_branches.id = external_status_checks_protected_branches.protected_branch_id
WHERE
    external_status_checks.project_id = 1
AND (
    PROTECTED_BRANCHES.ID IS NULL
OR 
    PROTECTED_BRANCHES.NAME = 'master'
)
LIMIT 1;
Query plan

https://explain.depesz.com/s/b0cK

 Limit  (cost=0.74..8.61 rows=1 width=4) (actual time=0.032..0.033 rows=0 loops=1)
   Buffers: shared hit=5
   I/O Timings: read=0.000 write=0.000
   ->  Nested Loop Left Join  (cost=0.74..47.93 rows=6 width=4) (actual time=0.030..0.031 rows=0 loops=1)
         Filter: ((protected_branches.id IS NULL) OR ((protected_branches.name)::text = 'master'::text))
         Rows Removed by Filter: 0
         Buffers: shared hit=5
         I/O Timings: read=0.000 write=0.000
         ->  Nested Loop Left Join  (cost=0.30..23.82 rows=7 width=8) (actual time=0.029..0.030 rows=0 loops=1)
               Buffers: shared hit=5
               I/O Timings: read=0.000 write=0.000
               ->  Index Scan using idx_on_external_status_checks_project_id_external_url on public.external_status_checks  (cost=0.15..6.20 rows=3 width=8) (actual time=0.028..0.029 rows=0 loops=1)
                     Index Cond: (external_status_checks.project_id = 1)
                     Buffers: shared hit=5
                     I/O Timings: read=0.000 write=0.000
               ->  Index Scan using index_esc_protected_branches_on_external_status_check_id on public.external_status_checks_protected_branches  (cost=0.15..5.79 rows=8 width=16) (actual time=0.000..0.000 rows=0 loops=0)
                     Index Cond: (external_status_checks_protected_branches.external_status_check_id = external_status_checks.id)
                     I/O Timings: read=0.000 write=0.000
         ->  Index Scan using protected_branches_pkey on public.protected_branches  (cost=0.44..3.43 rows=1 width=11) (actual time=0.000..0.000 rows=0 loops=0)
               Index Cond: (protected_branches.id = external_status_checks_protected_branches.protected_branch_id)
               I/O Timings: read=0.000 write=0.000

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Setup & Testing

  1. You must be logged in to view the status checks reports.
  2. You need a GitLab Ultimate license.
  3. Enable the development feature flag: echo "Feature.enable(:ff_external_status_checks)" | rails c
  4. Add a status check to Project > General Settings > Merge Requests > Status checks.
  5. Create an MR for the project using a different branch from one selected in the status check.
  6. View the MR and see that the status checks widget does not appear.
  7. Edit the status check to use Any branch.
  8. View the MR and see that the status checks widget appears with the status check.
  9. Edit the status check or MR to use the same branch as each other.
  10. View the MR and see that the status checks widget appears with the status check.

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Related to #334242 (closed)

Edited by Robert Hunt

Merge request reports