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

1104 add useraffiliation to the base user model #1126

Merged
merged 9 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions tom_common/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from django.contrib import admin
from django.contrib.auth.admin import UserAdmin as BaseUserAdmin
from django.contrib.auth.models import User

from tom_common.models import Profile


# Define an inline admin descriptor for the TomUser model
# which acts a bit like a singleton
class TomUserInline(admin.StackedInline):
model = Profile
can_delete = False
verbose_name_plural = "profiles"


# Define a new User admin
class UserAdmin(BaseUserAdmin):
inlines = [TomUserInline]


# Re-register UserAdmin
admin.site.unregister(User)
admin.site.register(User, UserAdmin)
82 changes: 72 additions & 10 deletions tom_common/forms.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
from django import forms
from django.contrib.auth.forms import UsernameField
from django.contrib.auth.models import User, Group
from django.db import transaction
from crispy_forms.helper import FormHelper

from tom_common.models import Profile

# UserCreationForm was changed for django 4.2 to not allow new users to have case-sensitive variations in
# existing usernames. This check breaks our username update process because we use the UserCreationForm rather
Expand Down Expand Up @@ -30,6 +34,22 @@ def save(self, *args, **kwargs):
return instance


class ProfileModelForm(forms.ModelForm):
class Meta:
model = Profile
fields = ('affiliation',)


UserProfileInlineFormSet = forms.inlineformset_factory(
User,
Profile,
form=ProfileModelForm,
extra=1,
can_delete=False,
can_order=False,
)


class CustomUserCreationForm(UserCreationForm):
"""
Form used for creation of new users and update of existing users.
Expand All @@ -43,14 +63,56 @@ class Meta:
fields = ('username', 'first_name', 'last_name', 'email', 'groups')
field_classes = {'username': UsernameField}

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

self.user_profile_formset = UserProfileInlineFormSet(
data=kwargs.get('data'), instance=self.instance
)

self.helper = FormHelper()
self.form_tag = False

def save(self, commit=True):
user = super(forms.ModelForm, self).save(commit=False)
# Because this form is used for both create and update user, and the user can be updated without modifying the
# password, we check if the password field has been populated in order to set a new one.
if self.cleaned_data['password1']:
user.set_password(self.cleaned_data["password1"])
if commit:
user.save()
self.save_m2m()

return user
# If any operations fail, we roll back
with transaction.atomic():
# Saving the MaterialRequisition first
user = super(forms.ModelForm, self).save(commit=False)

# Because this form is used for both create and update user, and the user can be updated without modifying
# the password, we check if the password field has been populated in order to set a new one.
if self.cleaned_data['password1']:
user.set_password(self.cleaned_data["password1"])
if commit:
# Saving the inline formsets
user.save()
self.user_profile_formset.instance = user
self.user_profile_formset.save()
self.save_m2m()

return user

# Also needs to be overridden in case any clean method are implemented
def clean(self):
self.user_profile_formset.clean()
super().clean()

return self.cleaned_data

# is_valid sets the cleaned_data attribute so we need to override that too
def is_valid(self):
is_valid = True
is_valid &= self.user_profile_formset.is_valid()
is_valid &= super().is_valid()

return is_valid

# In case you're using the form for updating, you need to do this too
# because nothing will be saved if you only update field in the inner formset
def has_changed(self):
has_changed = False

has_changed |= self.user_profile_formset.has_changed()
has_changed |= super().has_changed()

return has_changed
25 changes: 25 additions & 0 deletions tom_common/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django 4.2.16 on 2024-11-22 22:08

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

initial = True

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
]

operations = [
migrations.CreateModel(
name='Profile',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('affiliation', models.CharField(blank=True, max_length=100, null=True)),
('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
],
),
]
Empty file.
11 changes: 11 additions & 0 deletions tom_common/models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
from django.conf import settings
from django.db.models.signals import post_save
from django.db import models
from django.dispatch import receiver
from django.contrib.auth.models import User
from rest_framework.authtoken.models import Token


@receiver(post_save, sender=settings.AUTH_USER_MODEL)
def create_auth_token(sender, instance=None, created=False, **kwargs):
if created:
Token.objects.create(user=instance)


class Profile(models.Model):
"""Profile model for a TOMToolkit User"""
user = models.OneToOneField(User, on_delete=models.CASCADE)
affiliation = models.CharField(max_length=100, null=True, blank=True)

def __str__(self):
return f'{self.user.username} Profile'
22 changes: 22 additions & 0 deletions tom_common/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from django.db.models.signals import post_save
from django.contrib.auth.models import User
from django.dispatch import receiver
from tom_common.models import Profile


@receiver(post_save, sender=User)
def create_profile(sender, instance, created, **kwargs):
"""When a new user is created, create a profile for them."""
if created:
Profile.objects.create(user=instance)


@receiver(post_save, sender=User)
def save_profile(sender, instance, **kwargs):
"""When a user is saved, save their profile."""
# Take advantage of the fact that logging in updates a user's last_login field
# to create a profile for users that don't have one.
try:
instance.profile.save()
except User.profile.RelatedObjectDoesNotExist:
Profile.objects.create(user=instance)
1 change: 1 addition & 0 deletions tom_common/templates/tom_common/create_user.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{% endif %}
{% csrf_token %}
{% bootstrap_form form %}
{% bootstrap_formset form.user_profile_formset %}
{% buttons %}
<button type="submit" class="btn btn-primary">
{% if object %}
Expand Down
8 changes: 7 additions & 1 deletion tom_common/templates/tom_common/partials/user_data.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,18 @@ <h4 class="card-title">User Info</h4>
</div>
<div class="card-body">
<dl class="row">
{% for key, value in profile_data.items %}
{% for key, value in user_data.items %}
{% if value %}
<dt class="col-sm-6" >{% verbose_name user key %}</dt>
<dd class="col-sm-6">{{ value }}</dd>
{% endif %}
{% endfor %}
{% for key, value in profile_data.items %}
{% if value %}
<dt class="col-sm-6" >{% verbose_name profile key %}</dt>
<dd class="col-sm-6">{{ value }}</dd>
{% endif %}
{% endfor %}
</dl>
</div>
</div>
Expand Down
6 changes: 5 additions & 1 deletion tom_common/templatetags/tom_common_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.conf import settings
from django_comments.models import Comment
from django.apps import apps
from django.core.exceptions import FieldDoesNotExist
from guardian.shortcuts import get_objects_for_user

from tom_targets.models import Target
Expand Down Expand Up @@ -56,7 +57,10 @@ def verbose_name(instance, field_name):
"""
Displays the more descriptive field name from a Django model field
"""
return instance._meta.get_field(field_name).verbose_name.title()
try:
return instance._meta.get_field(field_name).verbose_name.title()
except FieldDoesNotExist:
return field_name.title()


@register.simple_tag
Expand Down
10 changes: 7 additions & 3 deletions tom_common/templatetags/user_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@ def user_data(user):
"""
Returns the user information as a dictionary.
"""
exlcude_fields = ['password', 'last_login', 'id', 'is_active']
user_fields = [field.name for field in user._meta.fields if field.name not in exlcude_fields]

exclude_fields = ['password', 'last_login', 'id', 'is_active', 'user']
user_dict = model_to_dict(user, exclude=exclude_fields)
profile_dict = model_to_dict(user.profile, exclude=exclude_fields)
return {
'user': user,
'profile_data': model_to_dict(user, fields=user_fields),
'profile': user.profile,
'user_data': user_dict,
'profile_data': profile_dict,
}
49 changes: 49 additions & 0 deletions tom_common/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django_comments.models import Comment

from tom_targets.tests.factories import SiderealTargetFactory
from tom_common.templatetags.tom_common_extras import verbose_name


class TestCommonViews(TestCase):
Expand All @@ -26,6 +27,17 @@ def test_index(self):
self.assertEqual(response.status_code, 200)


class TestCommonExtras(TestCase):
def setUp(self):
pass

def test_verbose_name(self):
# Check that the verbose name for a model field is returned correctly
self.assertEqual(verbose_name(User, 'email'), 'Email Address')
# Check that the verbose name for a non-existent field is returned correctly
self.assertEqual(verbose_name(User, 'definitely_not_a_field'), 'Definitely_Not_A_Field')


class TestUserManagement(TestCase):
def setUp(self):
self.admin = User.objects.create_superuser(username='admin', password='admin', email='[email protected]')
Expand All @@ -38,6 +50,8 @@ def test_user_list(self):

def test_user_create(self):
user_data = {
'profile-TOTAL_FORMS': '1',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We changed the user creation form to a formset, so we need to include the profile ManagementForm data here or it will get angry.

'profile-INITIAL_FORMS': '0',
'username': 'testuser',
'first_name': 'first',
'last_name': 'last',
Expand Down Expand Up @@ -83,6 +97,8 @@ def test_user_can_update_self(self):
user = User.objects.create(username='luke', password='forc3')
self.client.force_login(user)
user_data = {
'profile-TOTAL_FORMS': '1',
'profile-INITIAL_FORMS': '0',
'username': 'luke',
'first_name': 'Luke',
'last_name': 'Skywalker',
Expand All @@ -99,6 +115,8 @@ def test_user_cannot_update_other(self):
user = User.objects.create(username='luke', password='forc3')
self.client.force_login(user)
user_data = {
'profile-TOTAL_FORMS': '1',
'profile-INITIAL_FORMS': '0',
'username': 'luke',
'first_name': 'Luke',
'last_name': 'Skywalker',
Expand All @@ -111,6 +129,37 @@ def test_user_cannot_update_other(self):
self.assertRedirects(response, reverse('user-update', kwargs={'pk': user.id}))
self.assertNotEqual(self.admin.username, user_data['username'])

def test_user_can_delete_self(self):
user = User.objects.create(username='luke', password='forc3')
self.client.force_login(user)
self.assertTrue(User.objects.filter(username='luke').exists())
response = self.client.post(reverse('user-delete', kwargs={'pk': user.id}), follow=True)
self.assertEqual(response.status_code, 200)
self.assertFalse(User.objects.filter(username='luke').exists())


class TestUserProfile(TestCase):
def setUp(self):
self.admin = User.objects.create_superuser(username='admin', password='admin', email='[email protected]')
self.client.force_login(self.admin)

def test_user_profile(self):
user_data = {
'profile-TOTAL_FORMS': '1',
'profile-INITIAL_FORMS': '0',
'username': 'testuser',
'first_name': 'first',
'last_name': 'last',
'email': '[email protected]',
'password1': 'suchsecure543',
'password2': 'suchsecure543',
'profile-0-affiliation': 'Test University',
}
response = self.client.post(reverse('user-create'), data=user_data, follow=True)
self.assertEqual(response.status_code, 200)
user = User.objects.get(username='testuser')
self.assertEqual(user.profile.affiliation, 'Test University')


class TestAuthScheme(TestCase):
@override_settings(AUTH_STRATEGY='LOCKED')
Expand Down
Loading