Skip to content

Updated User model variable names #477

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

del9ra
Copy link
Member

@del9ra del9ra commented Mar 9, 2025

Fixes #429

What changes did you make?

  • Updated names and added the new items in views.py, conftest.py, test_models.py, test_apis.py, admin.py, serializers.py and models.py
  • Added a new UserManager model that doesn't use email because of the issues with email field
  • Wrote a test for the new relationships this model has with UserStatusType and PracticeArea
  • Wrote a test to check that the old names will return errors
  • Added an action item to issue Create Table: referrer #68 to uncomment the code line we commented out when that issue's table is created.
  • Generated a schema table for User, post it in a comment below and added a link to it in the Update Schema dependency issue.

Why did you make the changes (we will use this info to test)?

  • to include new fields and have the correct names for fields

@github-project-automation github-project-automation bot moved this to PR Needs review (automated column, do not place items here manually) in P: PD: Project Board Mar 9, 2025
@del9ra del9ra requested a review from fyliu March 9, 2025 14:41
@fyliu
Copy link
Member

fyliu commented Mar 10, 2025

From slack after looking at error output from ./scripts/test.sh -x (the -x stops it after the first failure)

error output excerpt

create_user taking in an email parameter is a problem.

...
core/tests/conftest.py:116: in admin
    return django_user_model.objects.create_user(
/usr/local/lib/python3.10/site-packages/django/contrib/auth/models.py:161: in create_user
    return self._create_user(username, email, password, **extra_fields)
/usr/local/lib/python3.10/site-packages/django/contrib/auth/models.py:153: in _create_user
    user = self.model(username=username, email=email, **extra_fields)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <User 1a0a9b03-cb5b-4d1a-9e46-b683a87f9188>, args = ()
kwargs = {'email': ''}, cls = <class 'core.models.User'>
opts = <Options for User>, _setattr = <built-in function setattr>
_DEFERRED = <Deferred field>
fields_iter = <tuple_iterator object at 0x7f8040cb2800>
val = zoneinfo.ZoneInfo(key='America/Los_Angeles')
field = <timezone_field.fields.TimeZoneField: time_zone>
is_related_object = False
property_names = frozenset({'is_anonymous', 'is_authenticated', 'is_django_user', 'pk'})
...

1:55 AM
Fang
ah okay. It looks like Django's default user comes with username, email, password fields
2:02 AM
Fang
I found a tutorial that does something similar to what we want: https://heemayl.net/posts/django-custom-user-model-without-username-field-and-using-email-in-place-of-it/
2:06 AM
The key is to create a new UserManager class that doesn't use email
2:10 AM
Just copy the one from django and modify it. You can get a copy of it with this command: docker-compose exec web cat /usr/local/lib/python3.10/site-packages/django/contrib/auth/models.py
2:11 AM
I got the path from one of the error messages
2:12 AM
You should put a comment above the manager code saying that it's based on the UserManager from django 4.2 or whatever we're using right now
2:14 AM
yes, 4.2. I got the version number from /app/requirements.in
2:17 AM
that way, if it stops working when we upgrade to django 5.2, whoever's doing the upgrade will know they might need to copy some lines over from the same code in 5.2
2:37 AM
Fang
Anyway. the issue we have here is that we're doing something different from what django provides. Ideally, we should just use what django gives us so that upgrades can be done smoothly. But django lets us override it if we really need to. Maybe we can ask Nicole and Bonnie to provide descriptions for other email fields and when each one would be added and used. It would be good to put that in the documentation to explain what they all are for VRMS to know how to use each one.

@del9ra del9ra force-pushed the update-table-user-429 branch from 1eba559 to 850f20d Compare March 14, 2025 22:35
@del9ra del9ra requested review from fyliu and removed request for fyliu March 15, 2025 00:16
@shmonks shmonks marked this pull request as draft March 28, 2025 00:43
@shmonks
Copy link
Member

shmonks commented Mar 28, 2025

Just converting this to draft for now, @del9ra

@del9ra
Copy link
Member Author

del9ra commented Mar 28, 2025

Just converting this to draft for now, @del9ra

Understood, thanks!

@ExperimentsInHonesty
Copy link
Member

ExperimentsInHonesty commented May 9, 2025

Need to review with @Neecolaa the user table... this PR has not merged yet. See PD: Table and field explanations spreadsheet with filter user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PR Needs review (automated column, do not place items here manually)
Development

Successfully merging this pull request may close these issues.

Update Table: User
4 participants