Discussion: Should domain removal/expiration or change in group plan unmark related enterprise users of the group?
According to planned work for &9675
There is no planned work for this issue. The goal of this issue is to figure out whether we should plan any additional work for &9675 or future iterations of Enterprise Users, that is related to the topic of this issue.
We create a service class Groups::EnterpriseUsers::AssociateService
that is responsible for marking a user as an Enterprise User of the top-level group if they meet the condition.
From the implementation perspective marking a user as an enterprise user of the group means setting the user's enterprise_group_id
attribute to the group.id
value. See details #406266 (closed).
Domain verification or re-verification should create a background job to apply this service class to all users whose primary email domain matches the verified domain (and for performance, exclude users that are already marked as enterprise users of the related top-level group, based on enterprise_group_id
column), see #388415 (closed).
We plan to reimplement the existing enterprise users features based on enterprise_group_id
instead of provisioned_by_group_id
. This is to make the Enterprise Users definition and related functionality in the GitLab system aligned with the legal definition. See https://gitlab.com/gitlab-org/gitlab/-/issues/396384.
enterprise_group_id
attribute instead of an SQL queries that reflect the legal Enterprise User definition to check whether user is an enterprise?
As a solution, why do we want to go with the new
enterprise_group_id
- Pros
- It is a safer and more flexible approach
- We need to have complete control over the process that makes user enterprise
- To send an email notification to a user to inform them that they are enterprise users of the group
- In case of enterprise user of one group becomes an enterprise user of another group, they should get the same email notification too
- We need to know the time since when the user is an enterprise user of the group. You might suggest that that could be
pages_domains.verified_at
, but that is inaccurate. There is domain re-verification that happens every 7 days - this attribute gets updated.
- Log for better visibility.
- To send an email notification to a user to inform them that they are enterprise users of the group
- This is the same as the current approach (
provisioned_by_group_id
). It will ease the replacing all related enterprise user conditions we have in the GitLab system. Related to https://gitlab.com/gitlab-org/gitlab/-/issues/396384 - It will allow us to get the users info like: "is the user an enterprise", "is the user an enterprise of this group", "which group is this user an enterprise of", "count of all enterprise users", "all enterprise users of the group", etc., easily and in a performant way.
- This is less incident prone. Imagine, if there is some lagging/issues with the domain verification or the group's subscription, users will remain enterprise.
- Cons
- We have to maintain the state of this attribute as per
- #388415 (closed)
- And this issue if we decide that domain removal/expiration or change in group plan should unmark related enterprise users of the group
- We have to maintain the state of this attribute as per
the legal Enterprise User definition
SQL queries that reflect- Pros
- This would eliminate the need in #388415 (closed) and this issue
- Cons
- It is not a safer or flexible approach
- It doesn't give us control over the process that makes user enterprise to
- know the time since when the user is an enterprise user of the group
- send an email notification to a user to inform them that they are enterprise users of the group
- know the time since when the user is an enterprise user of the group
- log for visibility
- This is different from the current approach (
provisioned_by_group_id
). It will be harder to replace all related enterprise user conditions we have in the GitLab system. Related to https://gitlab.com/gitlab-org/gitlab/-/issues/396384 - To get the users info like: "is the user an enterprise", "is the user an enterprise of this group", "which group is this user an enterprise of", "count of all enterprise users", "all enterprise users of the group", etc., would be too difficult. Each of those cases would require a unique SQL query based on the legal Enterprise User definition
- Performance of those queries
- This is more incident prone. Imagine, if there is some lagging/issues with the domain verification or the group's subscription, this will impact those SQL queries - users wouldn't remain enterprise.