Add Import Source Users to GraphQL
What does this MR do and why?
Add resolver to return all import source users associated with a namespace
Add mutations to manage import source users
- Cancel reassignment
- Reassign
- Keep as placeholder
Related to: #457066 (closed)
SQL queries
Raw SQL
SELECT
"import_source_users".*
FROM
"import_source_users"
WHERE
"import_source_users"."namespace_id" = 1
ORDER BY
"import_source_users"."id" DESC
LIMIT 101
Query plan
https://console.postgres.ai/gitlab/gitlab-production-main/sessions/29382/commands/91420
Limit (cost=0.01..0.02 rows=1 width=218) (actual time=0.043..0.043 rows=0 loops=1)
Buffers: shared hit=3
I/O Timings: read=0.000 write=0.000
-> Sort (cost=0.01..0.02 rows=1 width=218) (actual time=0.041..0.042 rows=0 loops=1)
Sort Key: import_source_users.id DESC
Sort Method: quicksort Memory: 25kB
Buffers: shared hit=3
I/O Timings: read=0.000 write=0.000
-> Seq Scan on public.import_source_users (cost=0.00..0.00 rows=1 width=218) (actual time=0.008..0.009 rows=0 loops=1)
Filter: (import_source_users.namespace_id = 9970)
Rows Removed by Filter: 0
I/O Timings: read=0.000 write=0.000
Time: 0.995 ms
- planning: 0.880 ms
- execution: 0.115 ms
- I/O read: 0.000 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 3 (~24.00 KiB) from the buffer pool
- reads: 0 from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
| Before | After |
|---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
Merge request reports
Activity
changed milestone to %17.2
assigned to @rodrigo.tomonari
added pipelinetier-1 label
- A deleted user
added database databasereview pending documentation labels
1 Warning 
This merge request is quite big (1378 lines changed), please consider splitting it into multiple merge requests. 2 Messages 
CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelogtrailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge. Documentation review
The following files require a review from a technical writer:
-
doc/api/graphql/reference/index.md(Link to current live version)
The review does not need to block merging this merge request. See the:
-
Metadata for the
*.mdfiles that you've changed. The first few lines of each*.mdfile identify the stage and group most closely associated with your docs change. - The Technical Writer assigned for that stage and group.
- Documentation workflows for information on when to assign a merge request for review.
Reviewer roulette
Category Reviewer Maintainer backend @ngala(UTC+5.5, 8.5 hours ahead of author)
@Andyschoenen(UTC+2, 5 hours ahead of author)
database @lma-git(UTC-7, 4 hours behind author)
@a_akgun(UTC+3, 6 hours ahead of author)
groupauthorization Reviewer review is optional for groupauthorization @jarka(UTC+2, 5 hours ahead of author)
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-reviewjob that generated this comment.Generated by
-
Resolved by Rodrigo Tomonari@.luke thanks for offering to review this change. I think this is good for a backend review. FYI: This is my first time adding resources to GraphQL. I think I managed to follow the pattern and guides
Edited by Rodrigo Tomonari- Last reply by Rodrigo Tomonari
requested review from @.luke
removed review request for @.luke
mentioned in merge request !156352 (merged)
requested review from @.luke
mentioned in issue #468506 (closed)
mentioned in issue #468437 (closed)
added pipeline:mr-approved label
added pipelinetier-2 label and removed pipelinetier-1 label
Resolved by Rodrigo TomonariBefore you set this MR to auto-merge
This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.
Before you resolve this discussion, please check the following:
- You are the last maintainer of this merge request
- The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
- This pipeline is recent enough (created in the last 8 hours)
If all the criteria above apply, please resolve this discussion and the set auto-merge for this merge request.
See pipeline tiers and merging a merge request for more details.
- Resolved by Evan Read
@eread Could you please give this GraphQL MR its Technical Writing review? Thank you!
- Last reply by Evan Read
requested review from @eread
added Technical Writing label
removed review request for @eread
gitlab_quality-test_tooling.
Click to expand
Job File Name Duration Expected duration #7167445443 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.57 s < 50.13 s #7173915548 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.53 s < 50.13 s #7175248768 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.48 s < 50.13 s #7176593683 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.79 s < 50.13 s #7182591752 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.79 s < 50.13 s #7183496661 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.43 s < 50.13 s #7194752893 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 68.19 s < 50.13 s #7194953840 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.08 s < 50.13 s #7196061203 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 65.98 s < 50.13 s #7197779249 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 65.16 s < 50.13 s #7202418613 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 65.75 s < 50.13 s #7203822796 spec/features/admin/users/users_spec.rb#L177Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.37 s < 50.13 s - A deleted user
added rspec:slow test detected label
E2E Test Result Summary
allure-report-publishergenerated test report!e2e-test-on-gdk:
expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Create | 126 | 0 | 11 | 0 | 137 | ✅ | | Verify | 36 | 0 | 2 | 0 | 38 | ✅ | | Plan | 60 | 0 | 2 | 0 | 62 | ✅ | | Data Stores | 27 | 0 | 4 | 0 | 31 | ✅ | | Govern | 65 | 0 | 0 | 0 | 65 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Package | 19 | 0 | 12 | 0 | 31 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | | Fulfillment | 1 | 0 | 0 | 0 | 1 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 349 | 0 | 32 | 0 | 381 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+e2e-test-on-cng:
expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Package | 23 | 0 | 15 | 0 | 38 | ✅ | | Create | 135 | 0 | 15 | 0 | 150 | ✅ | | Govern | 77 | 0 | 9 | 0 | 86 | ✅ | | Plan | 78 | 1 | 11 | 0 | 90 | ❌ | | Data Stores | 30 | 0 | 13 | 0 | 43 | ✅ | | Monitor | 8 | 0 | 12 | 0 | 20 | ✅ | | Verify | 53 | 0 | 13 | 0 | 66 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Fulfillment | 2 | 0 | 24 | 0 | 26 | ✅ | | Secure | 2 | 1 | 2 | 1 | 5 | ❌ | | ModelOps | 0 | 0 | 1 | 0 | 1 | ➖ | | Configure | 0 | 0 | 3 | 0 | 3 | ➖ | | Release | 5 | 0 | 1 | 0 | 6 | ✅ | | Ai-powered | 0 | 0 | 1 | 0 | 1 | ➖ | | Growth | 0 | 0 | 2 | 0 | 2 | ➖ | | Manage | 2 | 0 | 8 | 0 | 10 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 417 | 2 | 130 | 1 | 549 | ❌ | +-------------+--------+--------+---------+-------+-------+--------+e2e-package-and-test:
expand test summary
+-------------------------------------------------------------+ | suites summary | +--------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +--------+--------+--------+---------+-------+-------+--------+ | Manage | 29 | 1 | 15 | 1 | 45 | ❌ | | Create | 276 | 4 | 30 | 6 | 310 | ❌ | | Plan | 8 | 0 | 0 | 0 | 8 | ✅ | | Govern | 4 | 0 | 0 | 0 | 4 | ✅ | +--------+--------+--------+---------+-------+-------+--------+ | Total | 317 | 5 | 45 | 7 | 367 | ❌ | +--------+--------+--------+---------+-------+-------+--------+added 2 commits
reset approvals from @.luke by pushing to the branch
- Resolved by Rodrigo Tomonari
Thanks, @rodrigo.tomonari. The code looks good from the database perspective. I left you some comments to take a look
Can you also, update the description to include the database queries from the finders? https://docs.gitlab.com/ee/development/database_review.html#queries
- Last reply by Dylan Griffith
reset approvals from @jnutt by pushing to the branch
requested review from @eread
added databasereviewed label and removed databasereview pending label
requested review from @DylanGriffith
removed review request for @l.rosa
mentioned in issue #443555 (closed)
removed review request for @eread
added databaseapproved label and removed databasereviewed label
removed review request for @DylanGriffith
reset approvals from @DylanGriffith, @.luke, and @l.rosa by pushing to the branch
requested review from @eread
removed review request for @eread
reset approvals from @eread by pushing to the branch
- Resolved by Rodrigo Tomonari
@alexbuijs, may I ask you to do a groupauthorization review?
- Last reply by Alex Buijs
requested review from @alexbuijs
removed review request for @alexbuijs
added pipelinetier-3 label and removed pipelinetier-2 label
enabled automatic add to merge train when the pipeline for f9e8ed9f succeeds
-
Hello @rodrigo.tomonari
The database team is looking for ways to improve the database review process and we would love your help!
If you'd be open to someone on the database team reaching out to you for a chat, or if you'd like to leave some feedback asynchronously, just post a reply to this comment mentioning:
@gitlab-org/database-teamAnd someone will be by shortly!
Thanks for your help!
This message was generated automatically. You're welcome to improve it.
added workflowstaging-canary 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
mentioned in issue #477423 (closed)
added releasedpublished label and removed releasedcandidate label
15 loads: Types::UserType, 16 description: 'Global ID of the assignee user.' 17 18 field :import_source_user, 19 Types::Import::SourceUserType, 20 null: true, 21 description: "Mapping of a user on source instance to a user on destination instance after mutation." 22 23 authorize :admin_import_source_user 24 25 def resolve(args) 26 if Feature.disabled?(:bulk_import_user_mapping, current_user) 27 raise_resource_not_available_error! '`bulk_import_user_mapping` feature flag is disabled.' 28 end 29 30 import_source_user = authorized_find!(id: args[:id]) -
@rodrigo.tomonari I don't see any authorization to ensure that the namespace admin can only send reassignment emails to source users created in their own namespace. Am I missing something?
return error_invalid_permissions unless current_user.can?(:admin_import_source_user, import_source_user)is the only authorization code I could find inreassign_service.rbbelow, which only authorizes the admin, not the recipient./cc @wortschi
-
@ameyadarshan, the authorization is being performed twice, once at the GraphQL mutation level and once at the service level. In both cases the rule
admin_import_source_useris used.If you analyze the condition of the rule, you will see that the user needs to be able to administrate the
import_source_user.namespace(@subject.namespace)
added User Contribution Mapping label
