TP2000-1130 Move current workbasket from Session to custom User model (…

…#1123)

* Update User model references

* Use custom User model

* TP2000-1152-handling-invalid-workbaskets (#1113)

* Update middleware to check for workbasket changing state

* Update to use decorator rather than middleware, add pytest fixtures

* Update tests that require a session workbasket to run

* Move views and urls to workbasket app and update template

* Add tests for when workbasket status changes

* Tidy up following Pauls comments

* Update models and templates to find workbasket in user model

* Update test fixtures for workbasket being in user model

* Tidy up and test updates

* Update referencing to User model

* Updating bdd tests for new user model

* Add and update view and model unit tests

* Update require_current_workbasket decorator docstring

* Add docstring, move template for NoActiveWorkBasket view

* Amend current workbasket id retrieval in template

* Amend custom User model migration

* Remake migration adding current_workbasket field to User model

* Remove unused ValidateSessionWorkBasketMiddleware

* Make current_workbasket optional

* Add User model to admin

* Use historical models to fix migration tests

* Move ContentType data migration so it may be applied

* Rename function to remove a users current workbasket

* Amend docstrings

* Remove reference to session middleware that is no longer used

* Update workbaskets models following Pauls review

* Bring back user workbasket middleware as extra security

* Move User model from workbaskets app to common app

* Add forgotten content type data migration

* Remove setup_content_type fixture following patch to migrator fixture

* Amend middleware util method name

* Remove uneeded DoesNotExist try except block

---------

Co-authored-by: Dale Cannon <[email protected]>

additional_codes/tests/bdd/test_edit_additional_codes.py

@@ -15,13 +15,24 @@
 
 @pytest.fixture
 @when("I edit additional code X000")
-def model_edit_page(client, additional_code_X000):
-    return client.get(additional_code_X000.get_url("edit"))
+def model_edit_page(client_with_current_workbasket, additional_code_X000):
+    return client_with_current_workbasket.get(additional_code_X000.get_url("edit"))
+
+
+@pytest.fixture
+@when("I edit additional code X000")
+def model_edit_page_invalid_user(
+    client_with_current_workbasket_no_permissions,
+    additional_code_X000,
+):
+    return client_with_current_workbasket_no_permissions.get(
+        additional_code_X000.get_url("edit"),
+    )
 
 
 @then("I am not permitted to edit")
-def edit_permission_denied(model_edit_page):
-    assert model_edit_page.status_code == 403
+def edit_permission_denied(model_edit_page_invalid_user):
+    assert model_edit_page_invalid_user.status_code == 403
 
 
 @then("I see an edit form")
@@ -31,8 +42,12 @@ def edit_permission_granted(model_edit_page):
 
 @pytest.fixture
 @when("I set the end date before the start date on additional code X000")
-def end_date_before_start(client, response, additional_code_X000):
-    response["response"] = client.post(
+def end_date_before_start(
+    client_with_current_workbasket,
+    response,
+    additional_code_X000,
+):
+    response["response"] = client_with_current_workbasket.post(
         additional_code_X000.get_url("edit"),
         validity_period_post_data(
             start=date(2021, 1, 1),
@@ -44,8 +59,13 @@ def end_date_before_start(client, response, additional_code_X000):
 @when(
     "I set the start date of additional code X000 to overlap the previous additional code",
 )
-def submit_overlapping(client, response, additional_code_X000, old_additional_code):
-    response["response"] = client.post(
+def submit_overlapping(
+    client_with_current_workbasket,
+    response,
+    additional_code_X000,
+    old_additional_code,
+):
+    response["response"] = client_with_current_workbasket.post(
         additional_code_X000.get_url("edit"),
         validity_period_post_data(
             start=old_additional_code.valid_between.lower,

additional_codes/tests/test_forms.py

@@ -7,7 +7,7 @@
 
 
 # https://uktrade.atlassian.net/browse/TP2000-296
-def test_additional_code_create_sid(session_with_workbasket, date_ranges):
+def test_additional_code_create_sid(session_request_with_workbasket, date_ranges):
     """Tests that additional code type is NOT considered when generating a new
     sid."""
     type_1 = factories.AdditionalCodeTypeFactory.create()
@@ -21,7 +21,10 @@ def test_additional_code_create_sid(session_with_workbasket, date_ranges):
         "start_date_1": date_ranges.normal.lower.month,
         "start_date_2": date_ranges.normal.lower.year,
     }
-    form = forms.AdditionalCodeCreateForm(data=data, request=session_with_workbasket)
+    form = forms.AdditionalCodeCreateForm(
+        data=data,
+        request=session_request_with_workbasket,
+    )
 
     assert form.is_valid()
 
@@ -30,7 +33,10 @@ def test_additional_code_create_sid(session_with_workbasket, date_ranges):
     assert new_additional_code.sid != additional_code.sid
 
 
-def test_additional_code_create_valid_data(session_with_workbasket, date_ranges):
+def test_additional_code_create_valid_data(
+    session_request_with_workbasket,
+    date_ranges,
+):
     """Tests that AdditionalCodeCreateForm.is_valid() returns True when passed
     required fields and additional_code_description values in cleaned data."""
     code_type = factories.AdditionalCodeTypeFactory.create()
@@ -42,7 +48,10 @@ def test_additional_code_create_valid_data(session_with_workbasket, date_ranges)
         "start_date_1": date_ranges.normal.lower.month,
         "start_date_2": date_ranges.normal.lower.year,
     }
-    form = forms.AdditionalCodeCreateForm(data=data, request=session_with_workbasket)
+    form = forms.AdditionalCodeCreateForm(
+        data=data,
+        request=session_request_with_workbasket,
+    )
 
     assert form.is_valid()
     assert form.cleaned_data["additional_code_description"].description == "description"

additional_codes/tests/test_views.py

@@ -133,7 +133,7 @@ def test_additional_codes_detail_views(
     view,
     url_pattern,
     valid_user_client,
-    session_with_workbasket,
+    session_request_with_workbasket,
 ):
     """Verify that additional code detail views are under the url
     additional_codes/ and don't return an error."""
@@ -228,7 +228,7 @@ def test_additional_code_details_list_no_measures(valid_user_client):
     assert num_measures == 0
 
 
-def test_additional_code_description_create(valid_user_client):
+def test_additional_code_description_create(client_with_current_workbasket):
     """Tests that `AdditionalCodeDescriptionCreate` view returns 200 and creates
     a description for the current version of an additional code."""
     additional_code = factories.AdditionalCodeFactory.create()
@@ -250,10 +250,10 @@ def test_additional_code_description_create(valid_user_client):
     }
 
     with override_current_transaction(Transaction.objects.last()):
-        get_response = valid_user_client.get(url)
+        get_response = client_with_current_workbasket.get(url)
         assert get_response.status_code == 200
 
-        post_response = valid_user_client.post(url, data)
+        post_response = client_with_current_workbasket.post(url, data)
         assert post_response.status_code == 302
 
     assert AdditionalCodeDescription.objects.filter(

certificates/tests/bdd/test_edit_certificates.py

@@ -10,13 +10,24 @@
 
 @pytest.fixture
 @when("I edit certificate X000")
-def model_edit_page(client, certificate_X000):
-    return client.get(certificate_X000.get_url("edit"))
+def model_edit_page(client_with_current_workbasket, certificate_X000):
+    return client_with_current_workbasket.get(certificate_X000.get_url("edit"))
+
+
+@pytest.fixture
+@when("I edit certificate X000")
+def model_edit_page_invalid_user(
+    client_with_current_workbasket_no_permissions,
+    certificate_X000,
+):
+    return client_with_current_workbasket_no_permissions.get(
+        certificate_X000.get_url("edit"),
+    )
 
 
 @then("I am not permitted to edit")
-def edit_permission_denied(model_edit_page):
-    assert model_edit_page.status_code == 403
+def edit_permission_denied(model_edit_page_invalid_user):
+    assert model_edit_page_invalid_user.status_code == 403
 
 
 @then("I see an edit form")
@@ -26,8 +37,8 @@ def edit_permission_granted(model_edit_page):
 
 @pytest.fixture
 @when("I set the end date before the start date on certificate X000")
-def end_date_before_start(client, response, certificate_X000):
-    response["response"] = client.post(
+def end_date_before_start(client_with_current_workbasket, response, certificate_X000):
+    response["response"] = client_with_current_workbasket.post(
         certificate_X000.get_url("edit"),
         {
             "start_date_0": "1",

certificates/tests/test_forms.py

@@ -10,7 +10,7 @@
 
 
 def test_form_save_creates_new_certificate(
-    session_with_workbasket,
+    session_request_with_workbasket,
 ):
     """Tests that the certificate create form creates a new certificate, and
     that two certificates of the same type are created with different sid's."""
@@ -35,7 +35,7 @@ def test_form_save_creates_new_certificate(
     }
     form = forms.CertificateCreateForm(
         data=certificate_b_data,
-        request=session_with_workbasket,
+        request=session_request_with_workbasket,
     )
     certificate_b = form.save(commit=False)
 
@@ -45,7 +45,7 @@ def test_form_save_creates_new_certificate(
 
 
 def test_certificate_type_does_not_increment_id(
-    session_with_workbasket,
+    session_request_with_workbasket,
 ):
     """Tests that when two certificates are made with different types, the sids
     are not incremented."""
@@ -74,7 +74,7 @@ def test_certificate_type_does_not_increment_id(
     for certificate in certificates:
         form = forms.CertificateCreateForm(
             data=certificate,
-            request=session_with_workbasket,
+            request=session_request_with_workbasket,
         )
         saved_certificate = form.save(commit=False)
         completed_certificates.append(saved_certificate)
@@ -87,7 +87,7 @@ def test_certificate_type_does_not_increment_id(
     assert completed_certificates[1].sid == "001"
 
 
-def test_certificate_create_form_validates_data(session_with_workbasket):
+def test_certificate_create_form_validates_data(session_request_with_workbasket):
     """A test to check that the create form validates data and ciphers out
     incorrect submissions."""
 
@@ -101,7 +101,7 @@ def test_certificate_create_form_validates_data(session_with_workbasket):
     }
     form = forms.CertificateCreateForm(
         data=certificate_data,
-        request=session_with_workbasket,
+        request=session_request_with_workbasket,
     )
     error_string = [
         "Select a valid choice. That choice is not one of the available choices.",
@@ -119,7 +119,7 @@ def test_certificate_create_form_validates_data(session_with_workbasket):
     assert not form.is_valid()
 
 
-def test_certificate_create_with_custom_sid(session_with_workbasket):
+def test_certificate_create_with_custom_sid(session_request_with_workbasket):
     """Tests that a certificate can be created with a custom sid inputted by the
     user."""
     certificate_type = factories.CertificateTypeFactory.create()
@@ -133,14 +133,14 @@ def test_certificate_create_with_custom_sid(session_with_workbasket):
     }
     form = forms.CertificateCreateForm(
         data=data,
-        request=session_with_workbasket,
+        request=session_request_with_workbasket,
     )
     certificate = form.save(commit=False)
 
     assert certificate.sid == "A01"
 
 
-def test_certificate_create_ignores_non_numeric_sid(session_with_workbasket):
+def test_certificate_create_ignores_non_numeric_sid(session_request_with_workbasket):
     """Tests that a certificate is created with a numeric sid when a certificate
     of the same type with a non-numeric sid already exists."""
     certificate_type = factories.CertificateTypeFactory.create()
@@ -154,14 +154,14 @@ def test_certificate_create_ignores_non_numeric_sid(session_with_workbasket):
     }
     form = forms.CertificateCreateForm(
         data=data,
-        request=session_with_workbasket,
+        request=session_request_with_workbasket,
     )
     certificate = form.save(commit=False)
 
     assert certificate.sid == "001"
 
 
-def test_validation_error_raised_for_duplicate_sid(session_with_workbasket):
+def test_validation_error_raised_for_duplicate_sid(session_request_with_workbasket):
     """Tests that a validation error is raised on create when a certificate of
     the same type with the same sid already exists."""
     certificate_type = factories.CertificateTypeFactory.create()
@@ -176,7 +176,7 @@ def test_validation_error_raised_for_duplicate_sid(session_with_workbasket):
     }
     form = forms.CertificateCreateForm(
         data=data,
-        request=session_with_workbasket,
+        request=session_request_with_workbasket,
     )
 
     assert not form.is_valid()

certificates/tests/test_views.py

@@ -46,7 +46,7 @@ def test_certificate_description_delete_form(use_delete_form):
 
 
 def test_certificate_create_form_creates_certificate_description_object(
-    valid_user_api_client,
+    api_client_with_current_workbasket,
 ):
     # Post a form
     create_url = reverse("certificate-ui-create")
@@ -60,7 +60,7 @@ def test_certificate_create_form_creates_certificate_description_object(
         "description": "A participation certificate",
     }
 
-    valid_user_api_client.post(create_url, form_data)
+    api_client_with_current_workbasket.post(create_url, form_data)
     #  get the certificate we have made, and the certificate description matching our description on the form
     certificate = models.Certificate.objects.all()[0]
     certificate_description = models.CertificateDescription.objects.filter(
@@ -84,7 +84,7 @@ def test_certificate_detail_views(
     view,
     url_pattern,
     valid_user_client,
-    session_with_workbasket,
+    session_request_with_workbasket,
 ):
     """Verify that certificate detail views are under the url certificates/ and
     don't return an error."""
@@ -128,7 +128,7 @@ def test_description_create_get_initial():
         assert initial["described_certificate"] == new_version
 
 
-def test_description_create_get_context_data(valid_user_api_client):
+def test_description_create_get_context_data(api_client_with_current_workbasket):
     """Test that posting to certificate create endpoint with valid data returns
     a 302 and creates new description matching certificate."""
     certificate = factories.CertificateFactory.create(description=None)
@@ -145,7 +145,7 @@ def test_description_create_get_context_data(valid_user_api_client):
         "validity_start_2": 2022,
     }
     assert not models.CertificateDescription.objects.exists()
-    response = valid_user_api_client.post(url, post_data)
+    response = api_client_with_current_workbasket.post(url, post_data)
 
     assert response.status_code == 302
     assert models.CertificateDescription.objects.filter(

commodities/tests/test_migrations.py

@@ -2,21 +2,20 @@
 
 import pytest
 
-from common.tests.factories import QueuedWorkBasketFactory
 from common.util import TaricDateRange
 from common.validators import UpdateType
 
 
 @pytest.mark.django_db()
-def test_main_migration_works(migrator, setup_content_types):
+def test_main_migration_works(migrator):
     """Ensures that the description date fix for TOPS-745 migration works."""
 
     # before migration
     old_state = migrator.apply_initial_migration(
         ("commodities", "0011_TOPS_745_migration_dependencies"),
     )
 
-    setup_content_types(old_state.apps)
+    target_workbasket_id = 238
 
     GoodsNomenclatureDescription = old_state.apps.get_model(
         "commodities",
@@ -26,12 +25,13 @@ def test_main_migration_works(migrator, setup_content_types):
     Transaction = old_state.apps.get_model("common", "Transaction")
     Workbasket = old_state.apps.get_model("workbaskets", "WorkBasket")
     VersionGroup = old_state.apps.get_model("common", "VersionGroup")
+    User = old_state.apps.get_model("common", "User")
 
-    QueuedWorkBasketFactory.create().save()
-    workbasket = Workbasket.objects.create(id=238, author_id=1)
+    user = User.objects.create()
+    workbasket = Workbasket.objects.create(id=target_workbasket_id, author=user)
     new_transaction = Transaction.objects.create(
         workbasket=workbasket,
-        order=Transaction.objects.order_by("order").last().order + 1,
+        order=1,
     )
 
     gn_older_version = GoodsNomenclature.objects.create(
@@ -86,14 +86,12 @@ def test_main_migration_works(migrator, setup_content_types):
 
 
 @pytest.mark.django_db()
-def test_main_migration_ignores_if_no_data(migrator, setup_content_types):
+def test_main_migration_ignores_if_no_data(migrator):
     # before migration
     old_state = migrator.apply_initial_migration(
         ("commodities", "0011_TOPS_745_migration_dependencies"),
     )
 
-    setup_content_types(old_state.apps)
-
     GoodsNomenclatureDescription = old_state.apps.get_model(
         "commodities",
         "GoodsNomenclatureDescription",