From afa8a63a77b143e5617ff597603da1630dc927aa Mon Sep 17 00:00:00 2001 From: Johanna England Date: Mon, 4 Sep 2023 15:00:57 +0200 Subject: [PATCH 1/3] Handle multiple old defaults when setting new one For some reason we ended up with multiple default dashboards This fix will make setting a new one possible --- python/nav/web/webfront/views.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/python/nav/web/webfront/views.py b/python/nav/web/webfront/views.py index 8dfb2e19e6..87ba364e56 100644 --- a/python/nav/web/webfront/views.py +++ b/python/nav/web/webfront/views.py @@ -403,19 +403,19 @@ def set_account_preference(request): def set_default_dashboard(request, did): """Set the default dashboard for the user""" dash = get_object_or_404(AccountDashboard, pk=did, account=request.account) - try: - old_default = AccountDashboard.objects.get( - account=request.account, is_default=True - ) - except AccountDashboard.DoesNotExist: - # No previous default - old_default = None - dash.is_default = True - dash.save() - if old_default: + old_defaults = list( + AccountDashboard.objects.filter(account=request.account, is_default=True) + ) + for old_default in old_defaults: old_default.is_default = False - old_default.save() + + dash.is_default = True + + AccountDashboard.objects.bulk_update( + objs=old_defaults + [dash], fields=["is_default"] + ) + return HttpResponse(u'Default dashboard set to «{}»'.format(dash.name)) From 396dd3ae915b1242a6e8256cb1bae0e672f30ed7 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Tue, 19 Sep 2023 14:39:14 +0200 Subject: [PATCH 2/3] Factor out admin account as general fixture --- tests/integration/api_test.py | 5 ++--- tests/integration/conftest.py | 7 +++++++ tests/integration/models/alert_test.py | 13 ++++--------- tests/integration/seeddb_test.py | 4 ++-- tests/integration/web/info_test.py | 4 ++-- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/tests/integration/api_test.py b/tests/integration/api_test.py index 216c7795b9..44faf5b483 100644 --- a/tests/integration/api_test.py +++ b/tests/integration/api_test.py @@ -380,7 +380,7 @@ def test_api_urls_should_resolve(urlname, arg): @pytest.fixture() -def serializer_models(localhost): +def serializer_models(localhost, admin_account): """Fixture for testing API serializers - unrecognized_neighbor @@ -433,7 +433,6 @@ def serializer_models(localhost): alert_type_id=boxdown_id, end_time=INFINITY, ).save() - admin = profiles.Account.objects.get(login='admin') - auditlog.LogEntry.add_log_entry(admin, verb='verb', template='asd') + auditlog.LogEntry.add_log_entry(admin_account, verb='verb', template='asd') manage.Usage(id='ans', description='Ansatte').save() manage.Usage(id='student', description='Studenter').save() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5f74f1d6a0..aadc768365 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -366,3 +366,10 @@ def _lookfor(string, filename): """Very simple grep-like function""" data = io.open(filename, 'r', encoding='utf-8').read() return string in data + + +@pytest.fixture +def admin_account(db): + from nav.models.profiles import Account + + yield Account.objects.get(id=Account.ADMIN_ACCOUNT) diff --git a/tests/integration/models/alert_test.py b/tests/integration/models/alert_test.py index c3cf957104..a0f227c2c8 100644 --- a/tests/integration/models/alert_test.py +++ b/tests/integration/models/alert_test.py @@ -46,14 +46,9 @@ def test_sending_alert_to_alert_address_with_invalid_address_will_delete_alert_a @pytest.fixture -def account(db): - return Account.objects.get(pk=Account.ADMIN_ACCOUNT) - - -@pytest.fixture -def alert_address(db, account): +def alert_address(db, admin_account): addr = AlertAddress( - account=account, + account=admin_account, type=AlertSender.objects.get(name=AlertSender.SMS), ) addr.save() @@ -63,8 +58,8 @@ def alert_address(db, account): @pytest.fixture -def alert_profile(db, account): - profile = AlertProfile(account=account) +def alert_profile(db, admin_account): + profile = AlertProfile(account=admin_account) profile.save() yield profile if profile.pk: diff --git a/tests/integration/seeddb_test.py b/tests/integration/seeddb_test.py index 7b0f195e87..efa7edfe60 100644 --- a/tests/integration/seeddb_test.py +++ b/tests/integration/seeddb_test.py @@ -17,12 +17,12 @@ def test_usage_edit_url_should_allow_slashes(): assert reverse('seeddb-usage-edit', args=('TEST/SLASH',)) -def test_editing_deleted_netboxes_should_raise_404(): +def test_editing_deleted_netboxes_should_raise_404(admin_account): netboxid = 666 # Assuming no such netbox exists in test data set! factory = RequestFactory() url = reverse('seeddb-netbox-edit', args=(netboxid,)) request = factory.get(url) - request.account = Account.objects.get(pk=Account.ADMIN_ACCOUNT) + request.account = admin_account request.session = MagicMock() with pytest.raises(Http404): diff --git a/tests/integration/web/info_test.py b/tests/integration/web/info_test.py index 260fa56bbe..52424996c0 100644 --- a/tests/integration/web/info_test.py +++ b/tests/integration/web/info_test.py @@ -38,12 +38,12 @@ def test_failures_should_be_mentioned_in_search_page(client, failing_searchprovi assert failing_searchprovider in response.content.decode('utf-8') -def test_room_csv_download_should_not_produce_bytestring_representations(): +def test_room_csv_download_should_not_produce_bytestring_representations(admin_account): factory = RequestFactory() request = factory.post( reverse("room-csv"), data={"roomid": "myroom", "rows": "one;two;three\n"} ) - request.account = Account.objects.get(pk=Account.ADMIN_ACCOUNT) + request.account = admin_account request.session = MagicMock() response = create_csv(request) # type: django.http.response.HttpResponse From f5c04277a40b652910202a917f9b9a8a868c258e Mon Sep 17 00:00:00 2001 From: Johanna England Date: Tue, 12 Sep 2023 16:37:22 +0200 Subject: [PATCH 3/3] Add tests for setting default dashboard --- tests/integration/web/webfront_test.py | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/integration/web/webfront_test.py b/tests/integration/web/webfront_test.py index 35f1a99526..d64684af73 100644 --- a/tests/integration/web/webfront_test.py +++ b/tests/integration/web/webfront_test.py @@ -1,4 +1,8 @@ from mock import Mock + +from django.urls import reverse +from nav.compatibility import smart_str +from nav.models.profiles import AccountDashboard from nav.web.webfront.utils import tool_list @@ -6,3 +10,59 @@ def test_tools_should_be_readable(): admin = Mock() tools = tool_list(admin) assert len(tools) > 0 + + +def test_set_default_dashboard_should_succeed(db, client, admin_account): + """Tests that a default dashboard can be set""" + dashboard = AccountDashboard.objects.create( + name="new_default", + is_default=False, + account=admin_account, + ) + url = reverse("set-default-dashboard", args=(dashboard.pk,)) + response = client.post(url, follow=True) + + dashboard.refresh_from_db() + + assert response.status_code == 200 + assert f"Default dashboard set to «{dashboard.name}»" in smart_str(response.content) + assert dashboard.is_default + assert ( + AccountDashboard.objects.filter(account=admin_account, is_default=True).count() + == 1 + ) + + +def test_set_default_dashboard_with_multiple_previous_defaults_should_succeed( + db, client, admin_account +): + """ + Tests that a default dashboard can be set if multiple default dashboards + exist currently + """ + # By default there already exists one default dashboard for the admin user + # which is why we only have to create a second default one + default_dashboard = AccountDashboard.objects.create( + name="default_dashboard", + is_default=True, + account=admin_account, + ) + dashboard = AccountDashboard.objects.create( + name="new_default", + is_default=False, + account=admin_account, + ) + url = reverse("set-default-dashboard", args=(dashboard.pk,)) + response = client.post(url, follow=True) + + default_dashboard.refresh_from_db() + dashboard.refresh_from_db() + + assert response.status_code == 200 + assert f"Default dashboard set to «{dashboard.name}»" in smart_str(response.content) + assert dashboard.is_default + assert not default_dashboard.is_default + assert ( + AccountDashboard.objects.filter(account=admin_account, is_default=True).count() + == 1 + )