From cb612573e5769b6eeb8a60137bbc4dd0cb780b20 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 30 May 2024 15:36:30 -0700 Subject: [PATCH 01/12] Add a migration to store archived users for an account --- .../migrations/0019_accountuserarchive.py | 59 +++++++++++++++++++ anvil_consortium_manager/models.py | 18 ++++++ 2 files changed, 77 insertions(+) create mode 100644 anvil_consortium_manager/migrations/0019_accountuserarchive.py diff --git a/anvil_consortium_manager/migrations/0019_accountuserarchive.py b/anvil_consortium_manager/migrations/0019_accountuserarchive.py new file mode 100644 index 00000000..7daa34ec --- /dev/null +++ b/anvil_consortium_manager/migrations/0019_accountuserarchive.py @@ -0,0 +1,59 @@ +# Generated by Django 5.0 on 2024-05-30 22:31 + +import django.db.models.deletion +import django_extensions.db.fields +import simple_history.models +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('anvil_consortium_manager', '0018_alter_historicalworkspace_is_requester_pays_and_more'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='AccountUserArchive', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='anvil_consortium_manager.account')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'get_latest_by': 'modified', + 'abstract': False, + }, + ), + migrations.AddField( + model_name='account', + name='archived_users', + field=models.ManyToManyField(help_text='Previous users that this account has been linked to.', null=True, related_name='archived_accounts', through='anvil_consortium_manager.AccountUserArchive', to=settings.AUTH_USER_MODEL), + ), + migrations.CreateModel( + name='HistoricalAccountUserArchive', + fields=[ + ('id', models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField(db_index=True)), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('account', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='anvil_consortium_manager.account')), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'historical account user archive', + 'verbose_name_plural': 'historical account user archives', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + ] diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index 24c12022..68ba7c1a 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -219,6 +219,13 @@ class Account(TimeStampedModel, ActivatorModel): help_text="""The UserEmailEntry object used to verify the email, if the account was created by a user linking their email.""", ) + archived_users = models.ManyToManyField( + settings.AUTH_USER_MODEL, + related_name="archived_accounts", + help_text="Previous users that this account has been linked to.", + null=True, + through="AccountUserArchive", + ) note = models.TextField(blank=True, help_text="Additional notes.") history = HistoricalRecords() @@ -346,6 +353,17 @@ def has_workspace_access(self, workspace): return workspace in accessible_workspaces +class AccountUserArchive(TimeStampedModel): + """A model to store information about the previous users of an Account.""" + + account = models.ForeignKey(Account, on_delete=models.CASCADE) + user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) + history = HistoricalRecords() + + def __str__(self): + return "{user} for {account}".format(user=self.user, account=self.account) + + class ManagedGroup(TimeStampedModel): """A model to store information about AnVIL Managed Groups.""" From fcc40a449a94953f283e35f23dca983bf5fb6f53 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 08:59:19 -0700 Subject: [PATCH 02/12] Rename unlinked_users field and change null to blank --- ...t_archived_users_account_unlinked_users.py | 24 +++++++++++++++++++ anvil_consortium_manager/models.py | 6 ++--- 2 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py diff --git a/anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py b/anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py new file mode 100644 index 00000000..12fb4e7c --- /dev/null +++ b/anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py @@ -0,0 +1,24 @@ +# Generated by Django 5.0 on 2024-05-31 15:59 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('anvil_consortium_manager', '0019_accountuserarchive'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.RemoveField( + model_name='account', + name='archived_users', + ), + migrations.AddField( + model_name='account', + name='unlinked_users', + field=models.ManyToManyField(blank=True, help_text='Previous users that this account has been linked to.', related_name='unlinked_accounts', through='anvil_consortium_manager.AccountUserArchive', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index 68ba7c1a..0e260b50 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -219,11 +219,11 @@ class Account(TimeStampedModel, ActivatorModel): help_text="""The UserEmailEntry object used to verify the email, if the account was created by a user linking their email.""", ) - archived_users = models.ManyToManyField( + unlinked_users = models.ManyToManyField( settings.AUTH_USER_MODEL, - related_name="archived_accounts", + related_name="unlinked_accounts", help_text="Previous users that this account has been linked to.", - null=True, + blank=True, through="AccountUserArchive", ) note = models.TextField(blank=True, help_text="Additional notes.") From 41718ed9d4ce063f786c1ce667c2ce95430c9954 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 09:52:36 -0700 Subject: [PATCH 03/12] Add a view to unlink a user from an account --- .../account_confirm_unlink_user.html | 33 ++++ anvil_consortium_manager/tests/test_views.py | 186 ++++++++++++++++++ anvil_consortium_manager/urls.py | 1 + anvil_consortium_manager/views.py | 39 ++++ 4 files changed, 259 insertions(+) create mode 100644 anvil_consortium_manager/templates/anvil_consortium_manager/account_confirm_unlink_user.html diff --git a/anvil_consortium_manager/templates/anvil_consortium_manager/account_confirm_unlink_user.html b/anvil_consortium_manager/templates/anvil_consortium_manager/account_confirm_unlink_user.html new file mode 100644 index 00000000..ed31553d --- /dev/null +++ b/anvil_consortium_manager/templates/anvil_consortium_manager/account_confirm_unlink_user.html @@ -0,0 +1,33 @@ +{% extends "anvil_consortium_manager/base.html" %} +{% load static %} + +{% load render_table from django_tables2 %} + +{% block title %}Unlink user{% endblock %} + +{% block content %} +
+ +
+
+ + +

Unlink user from account

+ +

+ Are you sure you want to unlink {{ object.user }} from {{ object }}? +

+ +
{% csrf_token %} + {{ form }} + + + No, cancel +
+ +
+
+ + +
+{% endblock content %} diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index 9cda1f09..f1faef37 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -3923,6 +3923,192 @@ def test_account_already_active_post(self): self.assertEqual(views.AccountReactivate.message_already_active, str(messages[0])) +class AccountUnlinkUserTest(TestCase): + def setUp(self): + """Set up test class.""" + super().setUp() + self.factory = RequestFactory() + # Create a user with both view and edit permissions. + self.user = User.objects.create_user(username="test", password="test") + self.user.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME) + ) + self.user.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_EDIT_PERMISSION_CODENAME) + ) + + def get_url(self, *args): + """Get the url for the view being tested.""" + return reverse("anvil_consortium_manager:accounts:unlink", args=args) + + def get_view(self): + """Return the view being tested.""" + return views.AccountUnlinkUser.as_view() + + def test_view_redirect_not_logged_in(self): + "View redirects to login view when user is not logged in." + # Need a client for redirects. + uuid = uuid4() + response = self.client.get(self.get_url(uuid)) + self.assertRedirects(response, resolve_url(settings.LOGIN_URL) + "?next=" + self.get_url(uuid)) + + def test_status_code_with_user_permission(self): + """Returns successful response code.""" + instance = factories.AccountFactory.create(verified=True) + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.uuid)) + self.assertEqual(response.status_code, 200) + + def test_access_with_view_permission(self): + """Raises permission denied if user has only view permission.""" + user_with_view_perm = User.objects.create_user(username="test-other", password="test-other") + user_with_view_perm.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME) + ) + uuid = uuid4() + request = self.factory.get(self.get_url(uuid)) + request.user = user_with_view_perm + with self.assertRaises(PermissionDenied): + self.get_view()(request, uuid=uuid) + + def test_access_with_limited_view_permission(self): + """Raises permission denied if user has limited view permission.""" + uuid = uuid4() + user = User.objects.create_user(username="test-limited", password="test-limited") + user.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.VIEW_PERMISSION_CODENAME) + ) + request = self.factory.get(self.get_url(uuid)) + request.user = user + with self.assertRaises(PermissionDenied): + self.get_view()(request, uuid=uuid) + + def test_access_without_user_permission(self): + """Raises permission denied if user has no permissions.""" + user_no_perms = User.objects.create_user(username="test-none", password="test-none") + uuid = uuid4() + request = self.factory.get(self.get_url(uuid)) + request.user = user_no_perms + with self.assertRaises(PermissionDenied): + self.get_view()(request, uuid=uuid) + + def test_get_context_data(self): + """Context data is correct.""" + instance = factories.AccountFactory.create(verified=True) + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.uuid)) + self.assertIn("object", response.context_data) + self.assertEqual(instance, response.context_data["object"]) + + def test_view_with_invalid_object(self): + """Returns a 404 when the object doesn't exist.""" + uuid = uuid4() + request = self.factory.get(self.get_url(uuid)) + request.user = self.user + with self.assertRaises(Http404): + self.get_view()(request, uuid=uuid) + + def test_unlinks_user(self): + """Successful post request unlinks the user from the account.""" + instance = factories.AccountFactory.create(verified=True) + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertEqual(response.status_code, 302) + instance.refresh_from_db() + self.assertEqual(instance.user, None) + + def test_adds_user_to_unlinked_users(self): + """A record is added to unlinked_users.""" + instance = factories.AccountFactory.create(verified=True) + user = instance.user + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertEqual(response.status_code, 302) + instance.refresh_from_db() + # User link was archived. + self.assertIn(user, instance.unlinked_users.all()) + self.assertEqual(models.AccountUserArchive.objects.count(), 1) + archive = models.AccountUserArchive.objects.first() + self.assertEqual(archive.account, instance) + self.assertEqual(archive.user, user) + self.assertIsNotNone(archive.created) + + def test_can_add_second_user_to_unlinked_users(self): + """A record is added to unlinked_users.""" + old_user = User.objects.create_user(username="old_user", password="test") + instance = factories.AccountFactory.create(verified=True) + instance.unlinked_users.add(old_user) + user = instance.user + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertEqual(response.status_code, 302) + instance.refresh_from_db() + # User link was archived. + self.assertIn(user, instance.unlinked_users.all()) + self.assertEqual(models.AccountUserArchive.objects.count(), 2) + models.AccountUserArchive.objects.get(account=instance, user=old_user) + models.AccountUserArchive.objects.get(account=instance, user=user) + + def test_get_account_has_no_user_redirect(self): + instance = factories.AccountFactory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.uuid), {"submit": ""}) + self.assertRedirects(response, instance.get_absolute_url()) + + def test_get_account_no_user_message(self): + # A message is included. + instance = factories.AccountFactory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.uuid), {"submit": ""}, follow=True) + self.assertEqual(response.status_code, 200) + messages = [m.message for m in get_messages(response.wsgi_request)] + self.assertEqual(len(messages), 1) + self.assertEqual(views.AccountUnlinkUser.message_no_user, str(messages[0])) + instance.refresh_from_db() + self.assertIsNone(instance.user) + + def test_post_account_has_no_user_redirect(self): + instance = factories.AccountFactory.create() + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertRedirects(response, instance.get_absolute_url()) + + def test_post_account_no_user_message(self): + # A message is included. + instance = factories.AccountFactory.create() + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}, follow=True) + self.assertEqual(response.status_code, 200) + messages = [m.message for m in get_messages(response.wsgi_request)] + self.assertEqual(len(messages), 1) + self.assertEqual(views.AccountUnlinkUser.message_no_user, str(messages[0])) + instance.refresh_from_db() + self.assertIsNone(instance.user) + + def test_success_message(self): + instance = factories.AccountFactory.create(verified=True) + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}, follow=True) + self.assertEqual(response.status_code, 200) + messages = [m.message for m in get_messages(response.wsgi_request)] + self.assertEqual(len(messages), 1) + self.assertEqual(views.AccountUnlinkUser.success_message, str(messages[0])) + + def test_only_unlinks_specified_instance(self): + """View only deletes the specified pk.""" + instance = factories.AccountFactory.create(verified=True) + other_instance = factories.AccountFactory.create(verified=True) + other_user = other_instance.user + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertEqual(response.status_code, 302) + self.assertEqual(models.Account.objects.count(), 2) + instance.refresh_from_db() + other_instance.refresh_from_db() + self.assertEqual(instance.user, None) + self.assertEqual(other_instance.user, other_user) + + class AccountAutocompleteTest(TestCase): def setUp(self): """Set up test class.""" diff --git a/anvil_consortium_manager/urls.py b/anvil_consortium_manager/urls.py index 0892c75f..999d5a5b 100644 --- a/anvil_consortium_manager/urls.py +++ b/anvil_consortium_manager/urls.py @@ -50,6 +50,7 @@ views.AccountLinkVerify.as_view(), name="verify", ), + path("/unlink/", views.AccountUnlinkUser.as_view(), name="unlink"), path( "/add_to_group/", views.GroupAccountMembershipCreateByAccount.as_view(), diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index b493391e..5279956a 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -646,6 +646,45 @@ class AccountAudit(auth.AnVILConsortiumManagerStaffViewRequired, viewmixins.AnVI audit_class = audit.AccountAudit +class AccountUnlinkUser( + auth.AnVILConsortiumManagerStaffEditRequired, viewmixins.SingleAccountMixin, SuccessMessageMixin, FormView +): + """Unlink an Account from a User.""" + + """Deactivate an account and remove it from all groups on AnVIL.""" + + # model = models.Account + form_class = Form + template_name = "anvil_consortium_manager/account_confirm_unlink_user.html" + success_message = "Successfully unlinked user from Account." + message_no_user = "This Account is not linked to a user." + + def get(self, request, *args, **kwargs): + self.object = self.get_object() + if not self.object.user: + messages.add_message(self.request, messages.ERROR, self.message_no_user) + return HttpResponseRedirect(self.object.get_absolute_url()) + context = self.get_context_data(object=self.object) + return self.render_to_response(context) + + def post(self, request, *args, **kwargs): + self.object = self.get_object() + if not self.object.user: + messages.add_message(self.request, messages.ERROR, self.message_no_user) + return HttpResponseRedirect(self.object.get_absolute_url()) + return super().post(request, *args, **kwargs) + + def form_valid(self, form): + """Unlink the user from the account.""" + self.object.unlinked_users.add(self.object.user) + self.object.user = None + self.object.save() + return super().form_valid(form) + + def get_success_url(self): + return self.object.get_absolute_url() + + class ManagedGroupDetail( auth.AnVILConsortiumManagerStaffViewRequired, viewmixins.ManagedGroupGraphMixin, From 27cb7402a0336a420e1b41f933044971be9a0294 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 11:24:30 -0700 Subject: [PATCH 04/12] Show an "unlink user" button on AccountDetail For Accounts that have a linked user, show an "Unlink user" button on the account detail page. --- .../account_detail.html | 3 + anvil_consortium_manager/tests/test_views.py | 66 +++++++++++++++++++ anvil_consortium_manager/views.py | 1 + 3 files changed, 70 insertions(+) diff --git a/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html b/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html index 4ad62e74..65d2b159 100644 --- a/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html +++ b/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html @@ -114,6 +114,9 @@

{% if show_reactivate_button %} Reactivate account {% endif %} + {% if show_unlink_button %} + Unlink user + {% endif %} Delete from app

{% endif %} diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index f1faef37..77a15871 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -1269,6 +1269,72 @@ def test_context_inactive_account(self): self.assertIn("show_reactivate_button", context) self.assertTrue(context["show_reactivate_button"]) + def test_context_show_unlink_button_linked_account(self): + """An is_inactive flag is included in the context.""" + account = factories.AccountFactory.create(verified=True) + edit_user = User.objects.create_user(username="edit", password="test") + edit_user.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME), + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_EDIT_PERMISSION_CODENAME), + ) + self.client.force_login(edit_user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("show_unlink_button", context) + self.assertTrue(context["show_unlink_button"]) + self.assertContains( + response, reverse("anvil_consortium_manager:accounts:unlink", kwargs={"uuid": account.uuid}) + ) + + def test_context_show_unlink_button_linked_account_view_permission(self): + """An is_inactive flag is included in the context.""" + account = factories.AccountFactory.create(verified=True) + self.client.force_login(self.user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("show_unlink_button", context) + self.assertTrue(context["show_unlink_button"]) + self.assertNotContains( + response, reverse("anvil_consortium_manager:accounts:unlink", kwargs={"uuid": account.uuid}) + ) + + def test_context_show_unlink_button_unlinked_account(self): + """An is_inactive flag is included in the context.""" + account = factories.AccountFactory.create(verified=False) + edit_user = User.objects.create_user(username="edit", password="test") + edit_user.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME), + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_EDIT_PERMISSION_CODENAME), + ) + self.client.force_login(edit_user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("show_unlink_button", context) + self.assertFalse(context["show_unlink_button"]) + self.assertNotContains( + response, reverse("anvil_consortium_manager:accounts:unlink", kwargs={"uuid": account.uuid}) + ) + + def test_context_show_unlink_button_previously_linked(self): + """An is_inactive flag is included in the context.""" + account = factories.AccountFactory.create(verified=True) + account.unlinked_users.add(self.user) + account.user = None + account.save() + edit_user = User.objects.create_user(username="edit", password="test") + edit_user.user_permissions.add( + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME), + Permission.objects.get(codename=models.AnVILProjectManagerAccess.STAFF_EDIT_PERMISSION_CODENAME), + ) + self.client.force_login(edit_user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("show_unlink_button", context) + self.assertFalse(context["show_unlink_button"]) + self.assertNotContains( + response, reverse("anvil_consortium_manager:accounts:unlink", kwargs={"uuid": account.uuid}) + ) + def test_template_verified_account(self): """The template renders with a verified account.""" obj = factories.AccountFactory.create(verified=True) diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index 5279956a..053db114 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -209,6 +209,7 @@ def get_context_data(self, **kwargs): context["show_edit_links"] = self.request.user.has_perm("anvil_consortium_manager." + edit_permission_codename) context["show_deactivate_button"] = not context["is_inactive"] context["show_reactivate_button"] = context["is_inactive"] + context["show_unlink_button"] = self.object.user is not None context["group_table"] = tables.GroupAccountMembershipStaffTable( self.object.groupaccountmembership_set.all(), From 30547d1e98a119acf884d55bd2dbf00f272d0f9a Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 11:41:25 -0700 Subject: [PATCH 05/12] Show unlinked users on the AccountDetail page --- .../account_detail.html | 23 ++++++++++-- anvil_consortium_manager/tests/test_views.py | 35 +++++++++++++++++++ anvil_consortium_manager/views.py | 1 + 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html b/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html index 65d2b159..164a81ca 100644 --- a/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html +++ b/anvil_consortium_manager/templates/anvil_consortium_manager/account_detail.html @@ -14,7 +14,7 @@ Service account {% endif %} - {% if object.verified_email_entry %} + {% if object.user and object.verified_email_entry %} Verified by user @@ -33,7 +33,7 @@
Email
{{ object.email }}
Status
{{ object.get_status_display }}
User
- {% if object.verified_email_entry %} + {% if object.user and object.verified_email_entry %} {% if object.verified_email_entry.user.get_absolute_url %} {{ object.verified_email_entry.user }} {% else %} @@ -43,14 +43,31 @@ — {% endif %}
-
Date verified
{% if object.verified_email_entry %}{{ object.verified_email_entry.date_verified }}{% else %} — {% endif %}
+
Date verified
{% if object.user and object.verified_email_entry %}{{ object.verified_email_entry.date_verified }}{% else %} — {% endif %}
Date created
{{ object.created }}
Date modified
{{ object.modified }}
+ {% endblock panel %} {% block after_panel %} +{% if object.unlinked_users.count %} +
+
+
Unlinked users
+

Ths account was previously linked to the following user(s):

+
+
    + {% for unlinked_user in object.unlinked_users.all %} +
  • + {{ unlinked_user }} +
    • + {% endfor %} +
+
+{% endif %} +
diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index 77a15871..9443a5ef 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -1269,6 +1269,41 @@ def test_context_inactive_account(self): self.assertIn("show_reactivate_button", context) self.assertTrue(context["show_reactivate_button"]) + def test_context_unlinked_users_no_unlinked_user(self): + account = factories.AccountFactory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("unlinked_users", context) + self.assertEqual(len(context["unlinked_users"]), 0) + + def test_context_unlinked_users_one_unlinked_user(self): + account = factories.AccountFactory.create() + user = User.objects.create_user(username="test_unlinked", password="test") + account.unlinked_users.add(user) + self.client.force_login(self.user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("unlinked_users", context) + self.assertEqual(len(context["unlinked_users"]), 1) + self.assertIn(user, context["unlinked_users"]) + self.assertIn(str(user), response.content.decode()) + + def test_context_unlinked_users_two_unlinked_users(self): + account = factories.AccountFactory.create() + user_1 = User.objects.create_user(username="test_unlinked_1", password="test") + user_2 = User.objects.create_user(username="test_unlinked_2", password="test") + account.unlinked_users.add(user_1, user_2) + self.client.force_login(self.user) + response = self.client.get(self.get_url(account.uuid)) + context = response.context_data + self.assertIn("unlinked_users", context) + self.assertEqual(len(context["unlinked_users"]), 2) + self.assertIn(user_1, context["unlinked_users"]) + self.assertIn(user_2, context["unlinked_users"]) + self.assertIn(str(user_1), response.content.decode()) + self.assertIn(str(user_2), response.content.decode()) + def test_context_show_unlink_button_linked_account(self): """An is_inactive flag is included in the context.""" account = factories.AccountFactory.create(verified=True) diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index 053db114..98fe4a5f 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -210,6 +210,7 @@ def get_context_data(self, **kwargs): context["show_deactivate_button"] = not context["is_inactive"] context["show_reactivate_button"] = context["is_inactive"] context["show_unlink_button"] = self.object.user is not None + context["unlinked_users"] = self.object.unlinked_users.all() context["group_table"] = tables.GroupAccountMembershipStaffTable( self.object.groupaccountmembership_set.all(), From 9ea4eefa776d6e40335b3762f051775b8f04e995 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 11:45:31 -0700 Subject: [PATCH 06/12] Also set verified_email_entry to None when unlinking a user --- anvil_consortium_manager/tests/test_views.py | 1 + anvil_consortium_manager/views.py | 1 + 2 files changed, 2 insertions(+) diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index 9443a5ef..42fc1498 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -4117,6 +4117,7 @@ def test_unlinks_user(self): self.assertEqual(response.status_code, 302) instance.refresh_from_db() self.assertEqual(instance.user, None) + self.assertEqual(instance.verified_email_entry, None) def test_adds_user_to_unlinked_users(self): """A record is added to unlinked_users.""" diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index 98fe4a5f..71501f05 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -680,6 +680,7 @@ def form_valid(self, form): """Unlink the user from the account.""" self.object.unlinked_users.add(self.object.user) self.object.user = None + self.object.verified_email_entry = None self.object.save() return super().form_valid(form) From f9a9b22d2789fa7d24d25911d876b43ffe6bec62 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 11:53:52 -0700 Subject: [PATCH 07/12] Add verified_user_entry to the linked user archive In addition to storing the unlinked user in the archive, also store a link to the verified email entry, if it exists. --- ...erarchive_verified_email_entry_and_more.py | 24 +++++++++++++ anvil_consortium_manager/models.py | 1 + anvil_consortium_manager/tests/test_views.py | 34 +++++++++++++++++++ anvil_consortium_manager/views.py | 4 ++- 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py diff --git a/anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py b/anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py new file mode 100644 index 00000000..079528ed --- /dev/null +++ b/anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py @@ -0,0 +1,24 @@ +# Generated by Django 5.0 on 2024-05-31 18:48 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('anvil_consortium_manager', '0020_remove_account_archived_users_account_unlinked_users'), + ] + + operations = [ + migrations.AddField( + model_name='accountuserarchive', + name='verified_email_entry', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='anvil_consortium_manager.useremailentry'), + ), + migrations.AddField( + model_name='historicalaccountuserarchive', + name='verified_email_entry', + field=models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='anvil_consortium_manager.useremailentry'), + ), + ] diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index 0e260b50..9f11705f 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -358,6 +358,7 @@ class AccountUserArchive(TimeStampedModel): account = models.ForeignKey(Account, on_delete=models.CASCADE) user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) + verified_email_entry = models.ForeignKey(UserEmailEntry, on_delete=models.CASCADE, null=True, blank=True) history = HistoricalRecords() def __str__(self): diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index 42fc1498..f0f6cb83 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -4119,9 +4119,23 @@ def test_unlinks_user(self): self.assertEqual(instance.user, None) self.assertEqual(instance.verified_email_entry, None) + def test_unlinks_user_not_verified(self): + """Successful post request unlinks the user from the account.""" + instance = factories.AccountFactory.create() + user = factories.UserFactory.create() + instance.user = user + instance.save() + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertEqual(response.status_code, 302) + instance.refresh_from_db() + self.assertEqual(instance.user, None) + self.assertEqual(instance.verified_email_entry, None) + def test_adds_user_to_unlinked_users(self): """A record is added to unlinked_users.""" instance = factories.AccountFactory.create(verified=True) + verified_email_entry = instance.verified_email_entry user = instance.user self.client.force_login(self.user) response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) @@ -4134,6 +4148,26 @@ def test_adds_user_to_unlinked_users(self): self.assertEqual(archive.account, instance) self.assertEqual(archive.user, user) self.assertIsNotNone(archive.created) + self.assertEqual(archive.verified_email_entry, verified_email_entry) + + def test_adds_user_to_unlinked_users_not_verified(self): + """A record is added to unlinked_users.""" + instance = factories.AccountFactory.create() + user = factories.UserFactory.create() + instance.user = user + instance.save() + self.client.force_login(self.user) + response = self.client.post(self.get_url(instance.uuid), {"submit": ""}) + self.assertEqual(response.status_code, 302) + instance.refresh_from_db() + # User link was archived. + self.assertIn(user, instance.unlinked_users.all()) + self.assertEqual(models.AccountUserArchive.objects.count(), 1) + archive = models.AccountUserArchive.objects.first() + self.assertEqual(archive.account, instance) + self.assertEqual(archive.user, user) + self.assertIsNotNone(archive.created) + self.assertIsNone(archive.verified_email_entry) def test_can_add_second_user_to_unlinked_users(self): """A record is added to unlinked_users.""" diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index 71501f05..54f1a3a5 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -678,7 +678,9 @@ def post(self, request, *args, **kwargs): def form_valid(self, form): """Unlink the user from the account.""" - self.object.unlinked_users.add(self.object.user) + self.object.unlinked_users.add( + self.object.user, through_defaults={"verified_email_entry": self.object.verified_email_entry} + ) self.object.user = None self.object.verified_email_entry = None self.object.save() From b124c84009dd66c5e2afdee6c399641f8ff63ca0 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 13:33:48 -0700 Subject: [PATCH 08/12] Add tests for the AccoutUserArchive model --- anvil_consortium_manager/tests/test_models.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/anvil_consortium_manager/tests/test_models.py b/anvil_consortium_manager/tests/test_models.py index fe9f080f..7906aa14 100644 --- a/anvil_consortium_manager/tests/test_models.py +++ b/anvil_consortium_manager/tests/test_models.py @@ -15,6 +15,7 @@ from ..adapters.default import DefaultWorkspaceAdapter from ..models import ( Account, + AccountUserArchive, BillingProject, DefaultWorkspaceData, GroupAccountMembership, @@ -950,6 +951,54 @@ def test_has_workspace_access_is_not_shared_two_auth_domain_in_both(self): self.assertFalse(account.has_workspace_access(workspace)) +class AccountUserArchiveTestCase(TestCase): + """Tests for the AccountUserArchive model.""" + + def test_model_saving(self): + """Creation using the model constructor and .save() works.""" + user = factories.UserFactory.create() + account = factories.AccountFactory.create() + instance = AccountUserArchive(user=user, account=account) + instance.save() + self.assertIsInstance(instance, AccountUserArchive) + + def test_created_timestamp(self): + """created timestamp is set.""" + user = factories.UserFactory.create() + account = factories.AccountFactory.create() + instance = AccountUserArchive(user=user, account=account) + instance.save() + self.assertIsNotNone(instance.created) + + def test_verified_email_entry(self): + """Creation using the model constructor and .save() works.""" + account = factories.AccountFactory.create(verified=True) + instance = AccountUserArchive( + user=account.user, account=account, verified_email_entry=account.verified_email_entry + ) + instance.save() + self.assertIsInstance(instance, AccountUserArchive) + + def test_one_account_two_users(self): + """Multiple AccountUserArchive records for one user.""" + user = factories.UserFactory.create() + account_1 = factories.AccountFactory.create() + account_2 = factories.AccountFactory.create() + instance = AccountUserArchive(user=user, account=account_1) + instance.save() + instance_2 = AccountUserArchive(user=user, account=account_2) + instance_2.save() + self.assertEqual(AccountUserArchive.objects.count(), 2) + + def test_str_method(self): + """The custom __str__ method returns a string.""" + user = factories.UserFactory.create() + account = factories.AccountFactory.create() + instance = AccountUserArchive(user=user, account=account) + instance.save() + self.assertIsInstance(instance.__str__(), str) + + class ManagedGroupTest(TestCase): def test_model_saving(self): """Creation using the model constructor and .save() works.""" From 1a4c00e228b4efb7cd3bfc22ac8f1ace043baa57 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 13:47:15 -0700 Subject: [PATCH 09/12] Combine migrations into one --- .../migrations/0019_accountuserarchive.py | 8 ++++--- ...t_archived_users_account_unlinked_users.py | 24 ------------------- ...erarchive_verified_email_entry_and_more.py | 24 ------------------- 3 files changed, 5 insertions(+), 51 deletions(-) delete mode 100644 anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py delete mode 100644 anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py diff --git a/anvil_consortium_manager/migrations/0019_accountuserarchive.py b/anvil_consortium_manager/migrations/0019_accountuserarchive.py index 7daa34ec..032587bd 100644 --- a/anvil_consortium_manager/migrations/0019_accountuserarchive.py +++ b/anvil_consortium_manager/migrations/0019_accountuserarchive.py @@ -1,4 +1,4 @@ -# Generated by Django 5.0 on 2024-05-30 22:31 +# Generated by Django 5.0 on 2024-05-31 20:45 import django.db.models.deletion import django_extensions.db.fields @@ -23,6 +23,7 @@ class Migration(migrations.Migration): ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='anvil_consortium_manager.account')), ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ('verified_email_entry', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='anvil_consortium_manager.useremailentry')), ], options={ 'get_latest_by': 'modified', @@ -31,8 +32,8 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='account', - name='archived_users', - field=models.ManyToManyField(help_text='Previous users that this account has been linked to.', null=True, related_name='archived_accounts', through='anvil_consortium_manager.AccountUserArchive', to=settings.AUTH_USER_MODEL), + name='unlinked_users', + field=models.ManyToManyField(blank=True, help_text='Previous users that this account has been linked to.', related_name='unlinked_accounts', through='anvil_consortium_manager.AccountUserArchive', to=settings.AUTH_USER_MODEL), ), migrations.CreateModel( name='HistoricalAccountUserArchive', @@ -47,6 +48,7 @@ class Migration(migrations.Migration): ('account', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='anvil_consortium_manager.account')), ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), + ('verified_email_entry', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='anvil_consortium_manager.useremailentry')), ], options={ 'verbose_name': 'historical account user archive', diff --git a/anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py b/anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py deleted file mode 100644 index 12fb4e7c..00000000 --- a/anvil_consortium_manager/migrations/0020_remove_account_archived_users_account_unlinked_users.py +++ /dev/null @@ -1,24 +0,0 @@ -# Generated by Django 5.0 on 2024-05-31 15:59 - -from django.conf import settings -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('anvil_consortium_manager', '0019_accountuserarchive'), - migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ] - - operations = [ - migrations.RemoveField( - model_name='account', - name='archived_users', - ), - migrations.AddField( - model_name='account', - name='unlinked_users', - field=models.ManyToManyField(blank=True, help_text='Previous users that this account has been linked to.', related_name='unlinked_accounts', through='anvil_consortium_manager.AccountUserArchive', to=settings.AUTH_USER_MODEL), - ), - ] diff --git a/anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py b/anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py deleted file mode 100644 index 079528ed..00000000 --- a/anvil_consortium_manager/migrations/0021_accountuserarchive_verified_email_entry_and_more.py +++ /dev/null @@ -1,24 +0,0 @@ -# Generated by Django 5.0 on 2024-05-31 18:48 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('anvil_consortium_manager', '0020_remove_account_archived_users_account_unlinked_users'), - ] - - operations = [ - migrations.AddField( - model_name='accountuserarchive', - name='verified_email_entry', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='anvil_consortium_manager.useremailentry'), - ), - migrations.AddField( - model_name='historicalaccountuserarchive', - name='verified_email_entry', - field=models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='anvil_consortium_manager.useremailentry'), - ), - ] From b741d007220cb8de1f07971d96f67e22f96775fe Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 14:21:10 -0700 Subject: [PATCH 10/12] Move the unlinking logic to the Account model Add a method to the Account model that unlinks the user from an instance of the model. Call that method in the view, instead of encoding the logic in the view. Add tests for the method. --- anvil_consortium_manager/models.py | 9 ++++ anvil_consortium_manager/tests/test_models.py | 49 +++++++++++++++++++ anvil_consortium_manager/views.py | 7 +-- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index 9f11705f..39bbcc0a 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -352,6 +352,15 @@ def has_workspace_access(self, workspace): accessible_workspaces = self.get_accessible_workspaces() return workspace in accessible_workspaces + def unlink_user(self): + """Unlink the user from this account.""" + if not self.user: + raise ValueError("No user is linked to this account.") + self.unlinked_users.add(self.user, through_defaults={"verified_email_entry": self.verified_email_entry}) + self.user = None + self.verified_email_entry = None + self.save() + class AccountUserArchive(TimeStampedModel): """A model to store information about the previous users of an Account.""" diff --git a/anvil_consortium_manager/tests/test_models.py b/anvil_consortium_manager/tests/test_models.py index 7906aa14..517df634 100644 --- a/anvil_consortium_manager/tests/test_models.py +++ b/anvil_consortium_manager/tests/test_models.py @@ -950,6 +950,55 @@ def test_has_workspace_access_is_not_shared_two_auth_domain_in_both(self): # Sharing membership. self.assertFalse(account.has_workspace_access(workspace)) + def test_unlink_user(self): + """The unlink_user method removes the user and verified_email_entry.""" + account = factories.AccountFactory.create(verified=True) + account.unlink_user() + self.assertIsNone(account.user) + self.assertIsNone(account.verified_email_entry) + + def test_unlink_user_adds_to_archive(self): + """The unlink_user method adds the user to the AccountUserArchive.""" + account = factories.AccountFactory.create(verified=True) + user = account.user + verified_email_entry = account.verified_email_entry + account.unlink_user() + self.assertEqual(AccountUserArchive.objects.count(), 1) + archive = AccountUserArchive.objects.first() + self.assertEqual(archive.user, user) + self.assertEqual(archive.account, account) + self.assertEqual(archive.verified_email_entry, verified_email_entry) + + def test_unlink_user_no_verified_email(self): + """The unlink_user method removes the user and verified_email_entry.""" + user = factories.UserFactory.create() + account = factories.AccountFactory.create(user=user) + account.unlink_user() + self.assertIsNone(account.user) + self.assertIsNone(account.verified_email_entry) + self.assertEqual(AccountUserArchive.objects.count(), 1) + archive = AccountUserArchive.objects.first() + self.assertEqual(archive.user, user) + self.assertEqual(archive.account, account) + self.assertIsNone(archive.verified_email_entry) + + def test_can_archive_more_than_one_account_for_one_user(self): + """The unlink_user method adds the user to the AccountUserArchive.""" + user = factories.UserFactory.create() + account_1 = factories.AccountFactory.create(user=user) + account_1.unlink_user() + account_2 = factories.AccountFactory.create(user=user) + account_2.unlink_user() + self.assertEqual(AccountUserArchive.objects.count(), 2) + AccountUserArchive.objects.get(account=account_1, user=user) + AccountUserArchive.objects.get(account=account_2, user=user) + + def test_raises_value_error_no_user(self): + """Raises a ValueError if the account has no user.""" + account = factories.AccountFactory.create() + with self.assertRaises(ValueError): + account.unlink_user() + class AccountUserArchiveTestCase(TestCase): """Tests for the AccountUserArchive model.""" diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index 54f1a3a5..fb435115 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -678,12 +678,7 @@ def post(self, request, *args, **kwargs): def form_valid(self, form): """Unlink the user from the account.""" - self.object.unlinked_users.add( - self.object.user, through_defaults={"verified_email_entry": self.object.verified_email_entry} - ) - self.object.user = None - self.object.verified_email_entry = None - self.object.save() + self.object.unlink_user() return super().form_valid(form) def get_success_url(self): From 01b986adf0fe30cd52fa3fcfeab7a725686bba57 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 14:22:44 -0700 Subject: [PATCH 11/12] Clean up docstrings --- anvil_consortium_manager/models.py | 9 ++++++++- anvil_consortium_manager/views.py | 2 -- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index 39bbcc0a..19be2c9b 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -353,7 +353,14 @@ def has_workspace_access(self, workspace): return workspace in accessible_workspaces def unlink_user(self): - """Unlink the user from this account.""" + """Unlink the user from this account. + + This will remove the user from the account and add the user (and verified email entry, if applicable) to the + unlinked_users field. + + Raises: + ValueError: If there is no user linked to the account. + """ if not self.user: raise ValueError("No user is linked to this account.") self.unlinked_users.add(self.user, through_defaults={"verified_email_entry": self.verified_email_entry}) diff --git a/anvil_consortium_manager/views.py b/anvil_consortium_manager/views.py index fb435115..b89190fe 100644 --- a/anvil_consortium_manager/views.py +++ b/anvil_consortium_manager/views.py @@ -653,8 +653,6 @@ class AccountUnlinkUser( ): """Unlink an Account from a User.""" - """Deactivate an account and remove it from all groups on AnVIL.""" - # model = models.Account form_class = Form template_name = "anvil_consortium_manager/account_confirm_unlink_user.html" From 49ad21c9d275e7a618495e49bc6a016270c57d7c Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 31 May 2024 14:23:15 -0700 Subject: [PATCH 12/12] Bump version number and update changelog --- CHANGELOG.md | 1 + anvil_consortium_manager/__init__.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc0d79f7..9623ea5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Do not track previous groups for inactive accounts. * Add an audit error if any group membership records exist for inactive accounts. +* Add the ability to unlink a user from an account. ## 0.22.1 (2024-05-22) diff --git a/anvil_consortium_manager/__init__.py b/anvil_consortium_manager/__init__.py index 03b05414..036c5359 100644 --- a/anvil_consortium_manager/__init__.py +++ b/anvil_consortium_manager/__init__.py @@ -1 +1 @@ -__version__ = "0.23.0.dev0" +__version__ = "0.23.0.dev1"