In &4315, we introduced a User Cap setting for self-managed admins to prevent accidental user overages, now we are doing the same for SaaS group owners.
This work will be broken down into the following backend steps:
Add a new column to store user cap number to the Namespaces table
Add ApplicationSetting support for new setting
Add ability to update the user cap setting for a root level group
Calculate and expose user cap status (cap reached etc) for front-end use (see: #330028 (closed))
Introduce a feature flag for controlling access to User Cap functionality, e.g. exposing the status in the point above
Edit: I've removed the second point above because I'm not sure it's actually needed - this is a group/namespace level setting, not an instance-wide one
To support #330028 (closed) I've added a new implementation step of calculating the user cap status for a group (i.e. whether or not they've reached the user cap or not by checking the group#billable_members_count against the user cap), depending on the implementation approach this may increase the complexity and/or weight, but the first iteration could be a simple calculation.
@vij (and cc @amandarueda, as Vij is off for some time) I'll rephrase what you wrote, just to make sure I understood. Can you please confirm the following?
there's one big difference in the way this new setting should take form, when comparing self-managed and saas:
in self-managed, this user cap setting takes the form of a number (an actual cap), as shown in this related MR.
for saas, it's only a matter of whether we turn on or off this setting, as the cap is calculated by the number of billable member for a given namespace (no need to manually set that cap)
for saas, it's only a matter of whether we turn on or off this setting, as the cap is calculated by the number of billable member for a given namespace (no need to manually set that cap)
@ebaque Actually in SaaS we also need the ability to dictate a user cap as a quantity, not a simple on/off. Please lmk if you would like to discuss or if you have any questions.
Thanks @amandarueda, that's the confirmation I needed to move forward with the review of this MR.
That said, I didn't see any UX mockup that made me think that a number-based user cap setting was needed @esybrant is there a set of designs where we can see an input text for this new user cap setting?
EDIT: Nevermind! I actually had not scrolled down on the first mockup of that mockup set
Add a new column to store user cap number to the Namespaces table.
@vij the namespaces table currently has 11,318,379 rows Did you consider the addition of a new table re. user cap, instead of adding a new column?
I'm not familiar with the full scope of the related epic for this issue: once we're done with this issue here, do you now if we could add more columns later in that new table, which would justify even more its creation now?
I don't think there's currently a plan to add any more columns for Namespaces - we add one for Members to track their state but I think that's it.
There's also the namespace_settings table, perhaps that'd make more sense to use for this? I'm not too familiar with it so we'd need to make sure it fits the purpose (setting the user cap value for a top level group), but I suspect it'll be fine, WDYT?
@vij I'll look into namespace_settings and see what I can do.
I don't think there's currently a plan to add any more columns for Namespaces
Sorry, I'm trying to ramp up on the related epic, I'm still lacking technical background. I was mentioning the namespaces because it was the first item in the implementation plan.
I meant there isn't a plan to add any more than the suggested column in this issue - I think you're right to avoid adding more to Namespaces table given the size of it
I tried to make it easier for someone working on this to get to grips with the entire context by linking all the related issues in the description in a similar fashion, hope it helps
The implementation plan on each issue is just a rough guide to help give a starting point, there maybe more or less to do than what's in them (and likely entirely better ways of going about it ) - for that reason I also tried to point out that the weights may change accordingly once an engineer begins work on them
@vij OK I finally just checked, namespace_settings also has 11.3 million rows. I'll keep wrapping the overall context around my head and will see what I can suggest.
For adding a column I'm not sure how much of a problem this is / should be. There are instances of this happening in the last few months for the namespace_settings table (example), but maybe it's worth pinging someone from the database team? The MR went through the DB review without issue.
Otherwise we could create a new table entirely e.g. namespace_user_caps, but future modifications to that will likely hit the same barrier
I guess a nice solution (which doesn't really help us here) would be if the namespaces table had a jsonb column for some settings like these, that way we could add to it without further migrations
@vij I think we should go ahead with a new table, such as namespace_user_caps (with, let's say for the sake of the example, a user_cap integer column), along with the following tweak which I think would help us to minimize numbers of rows/increase performance. Roughly something like:
# app/models/namespace.rbclassNamespace<ApplicationRecordhas_one:namespace_user_capdefnamespace_user_capsuper.presence||build_namespace_user_capend# app/models/namespace_user_cap.rbclassNamespaceUserCap<ApplicationRecordbelongs_to:namespaceDEFAULT_CAP_USER=1234567defuser_capsuper||DEFAULT_CAP_USER# or whatever logic with default values instead of DEFAULT_CAP_USERend
The same strategy is more or less done for Project and ProjectSetting classes.
@ebaque what is the risk with using the namespace_settings table? I think my preference would be to use that if we can, because the user cap is a namespace setting, but with that said, the only concern I have with the approach above is that it's a new table and a class just for one integer attribute/column, which seems overkill.
For what it's worth, Jason recently added a column to that table a few weeks ago too, here - and I don't think there were any concerns raised by the database team.
I see we're both OOO now, so I'll defer to you for when you return (I'll be OOO for a while longer), so if the risk of adding to namespaces/namespace_settings is too high and you decide to go with the new table, I think your suggestion above looks good overall
For what it's worth, Jason recently added a column to that table a few weeks ago too, here - and I don't think there were any concerns raised by the database team.
@vij oh that's good to know. So we should be ok. I'll ask confirmation from a database maintainer then I'll proceed with adding a column to namespace_settings.
Hello @a_akgun just to confirm: would it be ok to add a new user_cap column to namespace_settings which is already a very big table (~ 11.4 million rows)? The fact that this was done a few weeks ago (see Vij's comment above for more details) leads me to say that it's possible. Otherwise, I was suggesting an alternative in this comment.
What do you think? And more generally, how would you define the line between adding a column to an existing (big) table vs having it in another table?
@ebaque I think it's ok and the recommended way. There's usually a concern when adding new columns to wider tables with already a lot of columns such as namespaces - the recommended way is to add the columns to related tables such as namespace_settings. So <TABLE>_settings, _preferences, _details, _meta, _extra. So the not the size, but the width of the table matters in this consideration.
@amandarueda I'm going to do some work around providing calculation to front end (number of billable members vs user cap), for the purpose of showing this warning message.
However, I don't know where this warning will show up, on which page it'll be. Could you please let me know about this? That will help to see where to do the correct implementation of this calculation.
Thanks for the question @ebaque, the banner should display for all root group members which have a role of owner. We'd expect the banner to display on all pages within the namespace. I've updated the description of the epic to include these details. LMK if you have additional questions.
Thanks @amandarueda for these details In a new MR on Monday, I'll see how I can expose that user cap calculation check to all pages within the namespace.
@amandarueda The MR with the calculation logic (and the :saas_user_caps FF) has been merged: !66264 (merged)
I'm going to update the weight of this issue to 3, and I think we're all done with it, so I'm also going to close it. Feel free to reopen it if needed (or if you think I missed something).
Etienne Baquémarked the checklist item Calculate and expose user cap status (cap reached etc) for front-end use (see: #330028 (closed)) as completed
marked the checklist item Calculate and expose user cap status (cap reached etc) for front-end use (see: #330028 (closed)) as completed
Etienne Baquémarked the checklist item Introduce a feature flag for controlling access to User Cap functionality, e.g. exposing the status in the point above as completed
marked the checklist item Introduce a feature flag for controlling access to User Cap functionality, e.g. exposing the status in the point above as completed