Ensure suggested reviewers order is preserved
requested to merge 376024-suggestions-sort-order-is-alphabetical-but-should-be-based-on-ml-score into master
What does this MR do and why?
Ensure the suggested reviewer's order is preserved.
This commit fixes the order issue on suggested reviewers caused by sorting in the frontend and order skipping in the backend.
- Remove alphabetical sort by user's name in front-end
- Add ordinality on suggested usernames in SQL in model
The front-end feature is gated behind a feature flag suggested_reviewers_control
which is disabled by default.
Screenshots or screen recordings
Before | After |
---|---|
🐘 Database
Query
SELECT "users".*
FROM "users"
INNER JOIN "project_authorizations" ON "users"."id" = "project_authorizations"."user_id"
JOIN UNNEST(
ARRAY ['a_akgun','tle_gitlab','dstull','imphill']
) WITH ORDINALITY AS t(username, ord) USING(username)
WHERE "project_authorizations"."project_id" = 278964
AND ("users"."state" IN ('active'))
AND (
"users"."user_type" IS NULL
OR "users"."user_type" IN (6, 4)
)
ORDER BY t.ord
Plan
Time: 96.376 ms
- planning: 10.116 ms
- execution: 86.260 ms
- I/O read: 85.532 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 12 (~96.00 KiB) from the buffer pool
- reads: 23 (~184.00 KiB) from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/12494/commands/44139
How to set up and validate locally
- Ensure a SaaS (Gitlab.com) environment
- One way of doing this is to add a
env.runit
file to the root GDK folder with the following snippetexport GITLAB_SIMULATE_SAAS=1
- One way of doing this is to add a
- Set ultimate license on a group
http://gdk.test:3000/admin/groups
- Create a project in the ultimate group or use an existing one, e.g.
http://gdk.test:3000/gitlab-org/gitlab-test
- Set the feature flag on rails console
bundle exec rails c
project = Project.find(2) Feature.enable(:suggested_reviewers_control, project)
- Enable suggested_reviewers project settings
project.project_setting.update!(suggested_reviewers_enabled: true)
- Create a merge request on the project
- Use some existing project members, make sure to select the usernames in a reverse order
project.authorized_users.map(&:username) => ["root", "erinn_stoltenberg", "lorenza", "lea_mclaughlin", "curtis.tromp", "ken", "shayne.armstrong"] suggested_reviewers = ["shayne.armstrong", "curtis.tromp"]
- Add some suggestions on rails console
bundle exec rails console
mr = MergeRequest.find(7) mr.build_predictions mr.predictions.update!(suggested_reviewers: { reviewers: suggested_reviewers })
- Confirm via the console and UI that the suggested reviewers display in the correct order
mr.suggested_reviewer_users => [#<User id:17 @shayne.armstrong>, #<User id:10 @curtis.tromp>]
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 #376024 (closed)
Edited by Tan Le