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

Auth user #241

Merged
merged 3 commits into from
Apr 18, 2018
Merged

Auth user #241

merged 3 commits into from
Apr 18, 2018

Conversation

anastasia
Copy link
Contributor

@anastasia anastasia commented Apr 17, 2018

  • APIUser is now CapUser
  • AUTH_USER_MODEL is now CapUser
  • DRF's Token model insists on using AUTH_USER_MODEL as its user field, so this means we can drop our implementation of it (APIToken is gone)
  • CapUserManager now has a create_superuser method
  • is_superuser and is_staff have access to admin. is_staff can see capapi models (currently just capuser). is_staff can authenticate capusers.

This pull request requires a long and arduous merging process, since Django is not friendly to changing one's mind about AUTH_USER_MODEL mid-project (https://docs.djangoproject.com/en/2.0/topics/auth/customizing/#changing-to-a-custom-user-model-mid-project).

This patch requires that we cleanly drop all users and have people re-register which is something we can get away with since we are early in the process with cap api (assuming we have ~3 users, all internal). This is a major change but if we don't start from a clean slate, making these seemingly mundane alterations to users will become an uphill battle. The below steps will outline this recommendations.

Here are the steps, as best as I can ascertain, since there were several false starts with this (see this for further information: https://code.djangoproject.com/ticket/25313):

  • pg_dump everything, everything, everything
    truncate django_migrations;
    drop table "capapi_apiuser" cascade;
    drop table "capapi_apitoken";
    drop table "django_admin_log";***
    drop table "authtoken_token";

*** locally, the only admin changes I had were of authenticating users. This step will not be right if the django_admin_log table is used for anything important.

make sure ./manage.py showmigrations shows that nothing is migrated

  • with the new migration included in this PR:
    ./manage.py migrate capapi
    ./manage.py migrate admin —fake-initial
    ./manage.py migrate

@jcushman
Copy link
Contributor

Nice work! Agreed it's a hassle, but I think it's a good plan.

One tweak -- the init_db function in fabfile.py is using Django's User model and needs to change to CapUser.objects.create_superuser('admin@example.com', 'admin').

For the merging process, I think we can just do this before deploying:

./manage.py migrate capapi zero
./manage.py migrate auth zero

And then deploy normally (check out new code, migrate). I tested this locally and it seems to work. It does essentially what you listed above -- drop the tables related to the capapi and auth app, and delete their migrations from django_migrations.

Copy link
Contributor

@bensteinberg bensteinberg left a comment

Choose a reason for hiding this comment

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

The deployment process looks pretty hectic. Thanks, @jcushman, for the simplification.

(Approved pending the change to fabfile.py)

@bensteinberg
Copy link
Contributor

A further comment: before merging this, I will do the zero migrations on dev. That is, please let me merge this one.

@anastasia
Copy link
Contributor Author

That's excellent, @jcushman, I didn't know about migrate zero. Such an improvement over my painful process

@bensteinberg bensteinberg merged commit 1ed22cf into harvard-lil:develop Apr 18, 2018
@anastasia anastasia deleted the auth-user branch March 19, 2019 16:56
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