Update redirects and commit lookup code for controllers
What does this MR do and why?
Update redirect and commit lookup code for controllers
A ref is ambiguous if a ref_type is not specified and it could be interpreted as a tag or a branch. When the ref_type parameter is missing and the ref is ambiguous we should redirect with a ref_type parameter. git uses the tag when it exists so we redirect to the tag when tag and branch both exist.
This change includes a new class RequestedRef which is meant to encapsulate the logic for determining when a ref is ambiguous.
How to set up and validate locally
- Create a project that has a branch and a tag with the same name
- Browse the repository and switch to the branch via the dropdown and see that the existing behaviour is preserved (get redirected to the same view except for the commit id)
- Enable the
redirect_with_ref_type
feature flag - Try selecting the branch again and see that there is a redirect to the same view with the ref_type in the query parameters of the url (the contents will be incorrect. displaying the correct contents will be handled in !122145 (merged))
- Try the same url without the ref_type and see that there is redirect to the same view with the ref_type but for tags
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 #20526 (closed)
Merge request reports
Activity
changed milestone to %16.1
assigned to @j.seto
- A deleted user
added feature flag label
4 Warnings This merge request is quite big (685 lines changed), please consider splitting it into multiple merge requests. dbd24e8f: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. e53cec92: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. Please add a merge request subtype to this merge request. 1 Message 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.
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 Ian Baum (
@ibaum
) (UTC-5, 1 hour behind@j.seto
)George Koltsov (
@georgekoltsov
) (UTC+1, 5 hours ahead of@j.seto
)UX Dan Mizzi-Harris (
@danmh
) (UTC+1, 5 hours ahead of@j.seto
)Maintainer review is optional for UX 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
Dangermentioned in commit gitlab-org-sandbox/gitlab-jh-validation@49c63207
- Resolved by Matthias Käppler
Allure report
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for 103d15b1expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Create | 8 | 0 | 1 | 6 | 9 | ❗ | | Manage | 1 | 0 | 0 | 0 | 1 | ✅ | | Framework sanity | 0 | 0 | 1 | 0 | 1 | ➖ | | Monitor | 4 | 0 | 0 | 4 | 4 | ❗ | | Data Stores | 2 | 0 | 0 | 0 | 2 | ✅ | | Govern | 2 | 0 | 0 | 0 | 2 | ✅ | | Plan | 4 | 0 | 0 | 3 | 4 | ❗ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 21 | 0 | 2 | 13 | 23 | ❗ | +------------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for 103d15b1expand test summary
+-------------------------------------------------------------+ | suites summary | +--------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +--------+--------+--------+---------+-------+-------+--------+ | Create | 268 | 0 | 34 | 4 | 302 | ❗ | +--------+--------+--------+---------+-------+-------+--------+ | Total | 268 | 0 | 34 | 4 | 302 | ❗ | +--------+--------+--------+---------+-------+-------+--------+
e2e-review-qa:
test report for 3123c72fexpand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Monitor | 4 | 0 | 0 | 0 | 4 | ✅ | | Create | 16 | 11 | 1 | 0 | 28 | ❌ | | Framework sanity | 0 | 0 | 1 | 0 | 1 | ➖ | | Govern | 2 | 0 | 0 | 0 | 2 | ✅ | | Data Stores | 2 | 0 | 0 | 1 | 2 | ❗ | | Manage | 1 | 0 | 0 | 0 | 1 | ✅ | | Plan | 3 | 0 | 1 | 0 | 4 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 28 | 11 | 3 | 1 | 42 | ❌ | +------------------+--------+--------+---------+-------+-------+--------+
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@3c2bf586
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@db0dfa92
mentioned in merge request !122144 (merged)
requested review from @jwoodwardgl
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@8d1b470e
added 4 commits
-
3123c72f...85e34e1f - 3 commits from branch
20526-impossible-to-browse-a-branch-if-a-tag-exists-with-the-same-name-be
- a2ba901c - Frontend changes to support fully qualified refs on blob and tree view
-
3123c72f...85e34e1f - 3 commits from branch
added 1 commit
- 6c8bf918 - Update redirect and commit lookup code for controllers
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@4625b33d
added 2208 commits
-
6c8bf918...ab066500 - 2207 commits from branch
20526-impossible-to-browse-a-branch-if-a-tag-exists-with-the-same-name-be
- e38ee9ee - Update redirect and commit lookup code for controllers
-
6c8bf918...ab066500 - 2207 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@88f64c16
- A deleted user
added documentation label
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@70d9159d
added 253 commits
-
e38ee9ee...efa4f790 - 252 commits from branch
master
- 807cd152 - Update redirect and commit lookup code for controllers
-
e38ee9ee...efa4f790 - 252 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@51846380
changed milestone to %16.2
requested review from @jpcyiza and removed review request for @jwoodwardgl
- Resolved by Jerry Seto
- Resolved by Jerry Seto
- Resolved by Jerry Seto
- Resolved by Patrick Cyiza
- Resolved by Jerry Seto
- Resolved by Jerry Seto
- Resolved by Jerry Seto
- Resolved by Patrick Cyiza
Hey @j.seto
, Great work on this issue! Lots of goodness here . I left a few comments for your consideration
added 2 commits
- Resolved by Jerry Seto
- Resolved by Jerry Seto
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@4d57b25c
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@cda2023b
@jpcyiza
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.
For more info, please refer to the following links:
added pipeline:mr-approved label
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@623cd67b
- Resolved by Matthias Käppler
Hey @mkaeppler
, could you take this review over as maintainer?
requested review from @mkaeppler
mentioned in merge request gitlab-com/www-gitlab-com!122232 (merged)
- Resolved by Jerry Seto
- Resolved by Matthias Käppler
added 1415 commits
-
c18eff9c...923cc8ac - 1408 commits from branch
master
- 94c80a71 - Update redirect and commit lookup code for controllers
- 641a29e6 - Refactor controller tests
- e53cec92 - Refactor RequestedRef
- 027763d1 - Refactor tests more
- 60beae12 - Update feature flag definition
- 895432a0 - Compare results with literal SHAs instead of result of code
- dbd24e8f - Refactor commit_without_ref_type
Toggle commit list-
c18eff9c...923cc8ac - 1408 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@2940cc6f
added 1 commit
- 103d15b1 - Refactor spec to the sha being tested more clear
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@034c8eaf
JiHu failures look unrelated; some problem with setting up ActiveRecord? https://gitlab.com/gitlab-org-sandbox/gitlab-jh-validation/-/jobs/4537367742
- Resolved by Matthias Käppler
@j.seto I think some of the QA failures could be related to these changes since they are in the
source_code
category. Could you have a look at https://gitlab-qa-allure-reports.s3.amazonaws.com/e2e-review-qa/20526-impossible-to-browse-a-branch-if-a-tag-exists-with-the-same-name-be-2/888169124/index.html#categories/2d9e7b3f55127a794c9b4ff9bc37206a/ec3b2e36454125b3/ and double-check?This, for example, looks suspiciously related to fetching refs:
enabled an automatic merge when the pipeline for 43df709c succeeds
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@43df709c
mentioned in commit e3021870
added workflowstaging-canary label and removed workflowin dev label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
added workflowpost-deploy-db-staging label and removed workflowproduction label
added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label
mentioned in issue gitlab-org/quality/triage-reports#15273 (closed)
mentioned in issue gitlab-org/quality/triage-reports#15701 (closed)