add action to link users to accounts #696

nora-codecov · 2024-07-18T23:47:33Z

Purpose/Motivation

Gives us a method to do the initial setup when an Account is created. This action will create the link between the Account object and the User object for any Owner who is a plan_activated_user on any of the Organizations linked to the Account.

Can be run multiple times safely (in case it errors or times out)
Won't do the link if the Account doesn't have enough seats

For "auditing" its behavior - it gives messages about how many Users and AccountUsers were created. You can see the Users linked to the Account on the Account edit page (when you click into an Account on the admin). You can remove a AccountUsers connection on the User admin page.

Links to relevant tickets

Uses codecov/shared#295
part of codecov/engineering-team#2058

codecov-notifications · 2024-07-19T01:24:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

codecov-qa · 2024-07-19T01:24:56Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.65%. Comparing base (4fde36b) to head (b9f1a9a).

✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main     #696      +/-   ##
==========================================
+ Coverage   91.63%   91.65%   +0.02%     
==========================================
  Files         632      632              
  Lines       16872    16917      +45     
==========================================
+ Hits        15461    15506      +45     
  Misses       1411     1411

Flag	Coverage Δ
unit	`91.65% <100.00%> (+0.02%)`	⬆️
unit-latest-uploader	`91.65% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
codecov_auth/admin.py	`100.00% <100.00%> (ø)`

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

codecov-public-qa · 2024-07-19T01:25:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.65%. Comparing base (4fde36b) to head (b9f1a9a).

✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main     #696      +/-   ##
==========================================
+ Coverage   91.63%   91.65%   +0.02%     
==========================================
  Files         632      632              
  Lines       16872    16917      +45     
==========================================
+ Hits        15461    15506      +45     
  Misses       1411     1411

Flag	Coverage Δ
unit	`91.65% <100.00%> (+0.02%)`	⬆️
unit-latest-uploader	`91.65% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
codecov_auth/admin.py	`100.00% <100.00%> (ø)`

codecov · 2024-07-19T01:26:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.98%. Comparing base (4fde36b) to head (b9f1a9a).

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff             @@
##               main       #696   +/-   ##
===========================================
  Coverage   95.98000   95.98000           
===========================================
  Files           812        812           
  Lines         18209      18254   +45     
===========================================
+ Hits          17477      17522   +45     
  Misses          732        732

Flag	Coverage Δ
unit	`91.65% <100.00%> (+0.02%)`	⬆️
unit-latest-uploader	`91.65% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adrian-codecov · 2024-07-19T20:00:27Z

codecov_auth/admin.py

+    @admin.action(description="Link Users to Account")
+    def link_users_to_account(self, request, queryset):
+        for account in queryset:
+            all_plan_activated_user_ids = set()


Nit/Optional: I find the "all" a bit redundant since the variable is plural. I also got tripped up a bit reading this like "(all plan) (activated user ids)" instead of "(all) (plan activated user ids)" 😅, but that might be me. I think calling this "activated_user_ids_in_orgs" represents this a bit better?

Similar idea for the rest of the "alls" as well

i see what you mean. I'm naming it after the field on the Owner model, plan_activated_users, which is actually a list of ownerids (😭)
what I want this to represent is that they have been collected and deduped, so they represent the unique owner objects that need to be connected to the Account (through their User)
account_plan_activated_user_ownerids?

haha yeah naming is hard :/. That sounds good to me, I'll leave it up to you for final naming, you're the most familiar w/ the code so as long as it makes sense to you 👍

adrian-codecov · 2024-07-19T20:02:37Z

codecov_auth/admin.py

+                ownerid__in=all_plan_activated_user_ids
+            ).prefetch_related("user")
+            non_student_count = all_plan_activated_users.exclude(student=True).count()
+            total_seats_for_account = account.plan_seat_count + account.free_seat_count


What's a free seat count? I see it's defaulted to 0, is this for like free plans?

it's a thing that exists on Owners currently (named free), but it's a way for us to give Orgs additional seats not in the count they pay for (plan_seat_count) 🤷

damn, was not aware of this

adrian-codecov · 2024-07-19T20:03:42Z

codecov_auth/admin.py

+            all_plan_activated_users = Owner.objects.filter(
+                ownerid__in=all_plan_activated_user_ids
+            ).prefetch_related("user")
+            non_student_count = all_plan_activated_users.exclude(student=True).count()


Optional: maybe adding a comment on why we exclude students. Thanks for being so attentive and catching this this early, this is a edge case for suuuure

hmmm, not really an edge case - check out activated_user_count or can_activate_user methods on Owner and Account - it's our built in policy.
I'm following the methods defined on the models to calculate things like can_activate_user - but having to write it out instead of using the pre-built methods so that I can do bulk operations 👍

Right, what I meant was that this is an easy thing to slip since the majority of our customers aren't students, so I was praising you for being diligent, looking at previous code and learning about our policy

adrian-codecov · 2024-07-19T20:05:33Z

codecov_auth/admin.py

+            )
+            userless_owners = []
+            for userless_owner in owners_without_user_objects:
+                new_user = User.objects.create()


Is this new User object meant to be empty?

no! fixed!
ty ty ty

adrian-codecov · 2024-07-19T20:06:19Z

codecov_auth/admin.py

+            total = Owner.objects.bulk_update(userless_owners, ["user"])
+            self.message_user(
+                request,
+                f"Created a User for {total} Owners",


Would probably be handy to log the owner ids we did this for 👀, up to you

adrian-codecov

Left some comments, overall lgtm! Lmk and I can approve

michelletran-codecov

Generally LGTM! This is only for adding orgs for now, correct? We would need some different mechanism if we were to remove orgs from accounts...

nora-codecov · 2024-07-22T22:02:43Z

updated and added based on @adrian-codecov and @michelletran-codecov feedback! ty for the comments ❤️

added logs 🤖
added logic to remove AccountUsers so now the action is a "true" sync 🔄
improved names 💪

nora-codecov requested review from michelletran-codecov and a team July 18, 2024 23:47

nora-codecov force-pushed the nora/2058 branch 2 times, most recently from f5fc9d2 to daf2712 Compare July 19, 2024 19:39

adrian-codecov reviewed Jul 19, 2024

View reviewed changes

nora-codecov force-pushed the nora/2058 branch 2 times, most recently from 268e8f7 to 0bda337 Compare July 20, 2024 00:00

michelletran-codecov approved these changes Jul 22, 2024

View reviewed changes

nora-codecov force-pushed the nora/2058 branch from 6a06d6a to 8e14052 Compare July 22, 2024 21:54

adrian-codecov approved these changes Jul 22, 2024

View reviewed changes

nora-codecov added 5 commits July 23, 2024 10:34

add action to link users to accounts

c1d4a57

add the 2 new count properties to AccountAdmin list view

b36805b

add separate admin action for seat check dry run

10fb723

use name and email from owner on new user object

4bc80d3

change naming, add logs, add removal logic

b9f1a9a

nora-codecov force-pushed the nora/2058 branch from 8e14052 to b9f1a9a Compare July 23, 2024 17:38

nora-codecov added this pull request to the merge queue Jul 23, 2024

Merged via the queue into main with commit b3c6343 Jul 23, 2024
21 of 22 checks passed

nora-codecov deleted the nora/2058 branch July 23, 2024 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add action to link users to accounts #696

add action to link users to accounts #696

nora-codecov commented Jul 18, 2024

codecov-notifications bot commented Jul 19, 2024

codecov-qa bot commented Jul 19, 2024 •

edited

Loading

codecov-public-qa bot commented Jul 19, 2024 •

edited

Loading

codecov bot commented Jul 19, 2024 •

edited

Loading

adrian-codecov Jul 19, 2024

nora-codecov Jul 22, 2024

adrian-codecov Jul 22, 2024

adrian-codecov Jul 19, 2024

nora-codecov Jul 22, 2024

adrian-codecov Jul 22, 2024

adrian-codecov Jul 19, 2024

nora-codecov Jul 22, 2024

adrian-codecov Jul 22, 2024

adrian-codecov Jul 19, 2024

nora-codecov Jul 22, 2024

adrian-codecov Jul 19, 2024

adrian-codecov left a comment

michelletran-codecov left a comment

nora-codecov commented Jul 22, 2024

add action to link users to accounts #696

add action to link users to accounts #696

Conversation

nora-codecov commented Jul 18, 2024

Purpose/Motivation

Links to relevant tickets

codecov-notifications bot commented Jul 19, 2024

Codecov Report

codecov-qa bot commented Jul 19, 2024 • edited Loading

Codecov Report

codecov-public-qa bot commented Jul 19, 2024 • edited Loading

Codecov Report

codecov bot commented Jul 19, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrian-codecov left a comment

Choose a reason for hiding this comment

michelletran-codecov left a comment

Choose a reason for hiding this comment

nora-codecov commented Jul 22, 2024

codecov-qa bot commented Jul 19, 2024 •

edited

Loading

codecov-public-qa bot commented Jul 19, 2024 •

edited

Loading

codecov bot commented Jul 19, 2024 •

edited

Loading