Merge pull request #2680 from johannaengland/bugfix/fix-multiple-defa…

…ult-dashboards

Prevent error when setting new default for multiple existing default dashboards

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))
 
 

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()

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)

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:

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):

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

tests/integration/web/webfront_test.py

@@ -1,8 +1,68 @@
 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
 
 
 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
+    )