Draft: Add membershipType argument
What does this MR do and why?
This MR adds membershipType
argument to the userAddOnAssignmentCreate
.
Following up on the discussion, this addition will help us reduce the check for valid billable membership of user to be assigned from at most 4 to 1.
There are 4 types of billable membership type that we support:
- group_member
- project_member
- group_invite
- project_invite
frontend will send the correct membership type of the user to be assigned based on the response fetched from billable_members endpoint.
Screenshots or screen recordings
Success
Failure
How to set up and validate locally
- Check out the branch
- Create a new root group namespace: "root-group"
- Setup some seed records
namespace = Namespace.last
add_on = GitlabSubscriptions::AddOn.find_or_create_by!(name: "code_suggestions") {|e| e.description = "Test"}
add_on_purchase = GitlabSubscriptions::AddOnPurchase.create!(
add_on: add_on, namespace: namespace, expires_on: 1.month.from_now, quantity: 5, purchase_xid: 'A-S0001'
)
add_on_purchase.to_global_id.to_s # "gid://gitlab/GitlabSubscriptions::AddOnPurchase/9"
# enable the feature flag
Feature.enable(:hamilton_seat_management)
- Create a subgroup for the namespace:
"sub-group"
- Go to Left Panel
Settings -> Manage -> Members -> Invite Members
- Add a user with developer role (or any role higher than guest)
- Go to graphql explorer (logged in as owner/admin): http://gdk.test:3000/-/graphql-explorer, and use following mutation with newly added user's id.
mutation {
userAddOnAssignmentCreate(
input: {
userId: "gid://gitlab/User/85",
addOnPurchaseId: "gid://gitlab/GitlabSubscriptions::AddOnPurchase/9",
membershipType: "project_member"
}) {
errors
}
}
- Check that response should be failure:
"errors": ["INVALID_USER_MEMBERSHIP"]
- Change the
membership_type
of mutation input to correct one:"group_member"
mutation {
userAddOnAssignmentCreate(
input: {
userId: "gid://gitlab/User/85",
addOnPurchaseId: "gid://gitlab/GitlabSubscriptions::AddOnPurchase/9",
membershipType: "group_member"
}) {
errors
}
}
- The response should be success:
errors: []
We can continue similar test for other scenarios too:
- Create a new project named "project-1" for the namespace, and add a new member with role greater than guest
- Run the GraphQL mutation for the new added user with
membershipType: "group_member"
first, and thenmembershipType: "project_member"
- The response should be failure first, and then success, respectively.
- Invite an existing group, say "invited-group-1" to the "sub-group" with role greater than guest
- Get the
user_id
of one of the member of "invited-group-1", and run the mutation withmembershipType: "group_invite"
- It should return success
- Invite an existing group, say "invited-group-2" to the "project-1" with role greater than guest
- Get the
user_id
of member of "invited-group-2", and run the mutation withmembershipType: "project_invite"
- It should return success
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 #415584
Merge request reports
Activity
changed milestone to %16.3
assigned to @bhrai
- A deleted user
added documentation label
1 Warning Please add a merge request subtype to this merge request. 1 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
*.md
files that you've changed. The first few lines of each*.md
file 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
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 Dzmitry Meshcharakou ( @dmeshcharakou
) (UTC+2, same timezone as@bhrai
)Ethan Urie ( @eurie
) (UTC-4, 6 hours behind@bhrai
)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
Danger -
@bhrai Some end-to-end (E2E) tests have been selected based on the stage label on this MR. Please start the
trigger-omnibus-and-follow-up-e2e
job in theqa
stage and ensure the tests infollow-up-e2e:package-and-test-ee
pipeline are passing before this MR is merged.If you would like to run all e2e tests, please apply the pipeline:run-all-e2e label and trigger a new pipeline. This will run all tests in
e2e:package-and-test
pipeline.For the list of known failures please refer to the latest pipeline triage issue.
Once done, please apply the
emoji on this comment. For any questions or help in reviewing the E2E test results, please reach out on the internal #quality Slack channel.
removed documentation label
12 12 argument :user_id, ::Types::GlobalIDType[::User], 13 13 required: true, description: 'Global ID of user to be assigned.' 14 14 15 argument :membership_type, GraphQL::Types::String, Info: The mutation is behind the feature flag hamilton_seat_management, and is also marked as alpha. So, adding this new argument will not require any deprecation process.
The issue was split in different MR to get faster review feedback.
Hi @ghinfey could you please do the initial review of the MR? Thank you
Thanks @bhrai Looks good to me, I pulled the branch and tested locally also with the steps above.
Hi @eurie,
Can you do the maintainer review please?
Thanks @ghinfey for the review.
Hi @eurie , I will unassign you from reviewer for now. We just found out that client may not have correct information on
membership_type
to send it to backend. We will need to investigate it further.I will change this MR to draft for now.
Sorry for the inconvenience :/
requested review from @ghinfey
Allure report
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for 22c0eea1 expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Plan | 47 | 0 | 0 | 40 | 47 | ❗ | | Create | 19 | 0 | 0 | 18 | 19 | ❗ | | Data Stores | 20 | 0 | 0 | 15 | 20 | ❗ | | Govern | 19 | 0 | 0 | 15 | 19 | ❗ | | Verify | 8 | 0 | 0 | 8 | 8 | ❗ | | Manage | 12 | 0 | 1 | 12 | 13 | ❗ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 125 | 0 | 1 | 108 | 126 | ❗ | +-------------+--------+--------+---------+-------+-------+--------+
@ghinfey
, 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
- A deleted user
added documentation label
removed review request for @eurie