Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tiny tweaks to form processing #3673

Merged
merged 13 commits into from
Dec 6, 2024

Conversation

rebeccacremona
Copy link
Contributor

This PR makes just a few tweaks to the how the new form for adding many users to an organization at a time saves data.

  • it adjusts when if commit is checked, so that all the logic except for the actual write operations is run if commit is False

  • it creates new OrganizationAffiliation objects all together in one batch, instead of two batches (one for brand new users and one for existing users)

  • when retrieving the collection of existing affiliation objects, it uses select_related('user'), so that later, when running set(affiliation.user for affiliation in preexisting_affiliations), each call to affiliation.user does not trigger a separate call to the database

  • it adjusts how we handle potentially mixed/uppercase and cast-to-lowercase email addresses so that a) duplicate emails are detected in the CSV even if they differ in casing, and b) we can retrieve the raw email address from self.user_data

I took the liberty of renaming a few variables, adding a few comments, and slightly changing the order of operations, so that existing users are handled first, and then we move on to new users.

@rebeccacremona rebeccacremona requested a review from a team as a code owner December 4, 2024 23:50
@rebeccacremona rebeccacremona requested review from cmsetzer and bensteinberg and removed request for a team and bensteinberg December 4, 2024 23:50
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 69.67%. Comparing base (344f09b) to head (5478649).
Report is 68 commits behind head on develop.

Files with missing lines Patch % Lines
perma_web/perma/forms.py 91.66% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3673      +/-   ##
===========================================
+ Coverage    69.05%   69.67%   +0.62%     
===========================================
  Files           54       54              
  Lines         7493     7661     +168     
===========================================
+ Hits          5174     5338     +164     
- Misses        2319     2323       +4     

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

@rebeccacremona rebeccacremona requested review from teovin and removed request for cmsetzer December 5, 2024 00:02
Copy link
Contributor

@cmsetzer cmsetzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I left a few comments about minor points of coding style; feel free to adopt or ignore as you prefer.

perma_web/perma/forms.py Outdated Show resolved Hide resolved
perma_web/perma/forms.py Outdated Show resolved Hide resolved
perma_web/perma/forms.py Outdated Show resolved Hide resolved
@rebeccacremona rebeccacremona merged commit 3e09be3 into harvard-lil:develop Dec 6, 2024
2 checks passed
@rebeccacremona rebeccacremona deleted the quick-tweaks branch January 6, 2025 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants