Unit test improvements.

apps/odk_publish/import_export.py

@@ -25,7 +25,7 @@ def clean(self, value, row=None, **kwargs):
         if template_variable is not None:
             template_variable.value = value
             try:
-                template_variable.full_clean()
+                template_variable.full_clean(exclude=["app_user"])
             except ValidationError as e:
                 raise ValueError(e.messages[0])
         return template_variable
@@ -46,7 +46,7 @@ def clean(self, value, row=None, **kwargs):
             val = super().clean(value, row, **kwargs)
         except InvalidOperation:
             raise ValueError("Value must be an integer.")
-        if val < 0:
+        if val and val < 0:
             raise ValueError("Value must be positive.")
         return val
 

config/templates/odk_publish/app_user_export.html

@@ -5,7 +5,7 @@
         <form method="post" class="max-w-2xl">
             {% csrf_token %}
             <div class="grid gap-4 sm:grid-cols-1 sm:gap-6">
-                <div id="id_file_container">
+                <div id="id_format_container">
                     {{ form.format.label_tag }}
                     {{ form.format }}
                     {{ form.format.errors }}
@@ -14,8 +14,7 @@
                     <button type="submit" class="btn btn-outline btn-primary">Submit</button>
                     <a type="button"
                        class="btn btn-outline"
-                       href="{% url 'odk_publish:app-user-list' request.odk_project.pk %}"
-                       :disabled="submitting">Cancel</a>
+                       href="{% url 'odk_publish:app-user-list' request.odk_project.pk %}">Cancel</a>
                 </div>
             </div>
         </form>

tests/odk_publish/test_import_export.py

@@ -37,13 +37,24 @@ def other_project(self, template_variables):
         project.template_variables.set(template_variables)
         return project
 
+    @pytest.fixture
+    def app_user(self, project):
+        app_user = models.AppUser.objects.create(
+            name="12345",
+            project=project,
+            central_id=1,
+        )
+        for var in project.template_variables.all():
+            app_user.app_user_template_variables.create(template_variable=var, value="test")
+        return app_user
+
     def test_export(self, project, other_project, template_variables):
         expected_export_values = set()
 
         # Create 5 users for each project
         for center_id in range(11030, 11040):
             app_user = models.AppUser.objects.create(
-                name=center_id,
+                name=str(center_id),
                 project=project if center_id % 2 else other_project,
                 central_id=center_id - 11000,
             )
@@ -76,17 +87,10 @@ def test_export(self, project, other_project, template_variables):
         for i in range(len(dataset)):
             assert dataset.get(i) in expected_export_values
 
-    def test_import(self, project):
-        app_user1 = models.AppUser.objects.create(
-            name=12345,
-            project=project,
-            central_id=1,
-        )
-        for var in project.template_variables.all():
-            app_user1.app_user_template_variables.create(template_variable=var, value="test")
-
+    def test_successful_import(self, app_user):
+        project = app_user.project
         app_user2 = models.AppUser.objects.create(
-            name=67890,
+            name="67890",
             project=project,
             central_id=2,
         )
@@ -95,34 +99,147 @@ def test_import(self, project):
 
         csv_data = (
             "id,name,central_id,center_id,center_label,public_key,manager_password\n"
-            f"{app_user1.id},11031,31,11031,Center 11031,key11031,pass11031\n"
+            f"{app_user.id},11031,31,11031,Center 11031,key11031,pass11031\n"
             f"{app_user2.id},11033,33,11033,Center 11033,key11033,pass11033\n"
             ",11035,35,11035,Center 11035,key11035,pass11035\n"
             ",11037,37,11037,Center 11037,key11037,pass11037\n"
             ",11039,39,11039,Center 11039,key11039,pass11039\n"
         )
         dataset = Dataset().load(csv_data)
         resource = import_export.AppUserResource(project)
-        resource.import_data(dataset)
+        result = resource.import_data(
+            dataset, use_transactions=True, rollback_on_validation_errors=True
+        )
 
+        assert not result.has_validation_errors()
+        assert not result.has_errors()
         assert models.AppUser.objects.count() == 5
         assert (
             models.AppUserTemplateVariable.objects.count() == project.template_variables.count() * 5
         )
 
-        app_user1.refresh_from_db()
-        for i in dataset.dict:
-            pk = i.pop("id")
+        # Make sure user data has been added / updated
+        for row in dataset.dict:
+            pk = row.pop("id")
+            name = row.pop("name")
             if pk:
-                # Make sure user data has been added / updated
+                # User was already in the DB
                 app_user = models.AppUser.objects.get(pk=pk)
-                assert app_user.name == i.pop("name")
-                assert app_user.central_id == int(i.pop("central_id"))
-                assert (
-                    dict(
-                        app_user.app_user_template_variables.values_list(
-                            "template_variable__name", "value"
-                        )
+                assert app_user.name == name
+            else:
+                # New user. Get by name
+                app_user = project.app_users.get(name=name)
+            assert app_user.central_id == int(row.pop("central_id"))
+            assert (
+                dict(
+                    app_user.app_user_template_variables.values_list(
+                        "template_variable__name", "value"
                     )
-                    == i
                 )
+                == row
+            )
+
+    def test_cannot_import_other_projects_users(self, app_user, other_project):
+        app_user2 = models.AppUser.objects.create(
+            name=67890,
+            project=other_project,
+            central_id=2,
+        )
+        csv_data = (
+            "id,name,central_id,center_id,center_label,public_key,manager_password\n"
+            f"{app_user.id},11031,31,11031,Center 11031,key11031,pass11031\n"
+            f"{app_user2.id},11033,33,11033,Center 11033,key11033,pass11033"
+        )
+        dataset = Dataset().load(csv_data)
+        resource = import_export.AppUserResource(app_user.project)
+        result = resource.import_data(
+            dataset, use_transactions=True, rollback_on_validation_errors=True
+        )
+
+        # Import should fail because app_user2 is linked to other_project
+        assert result.has_validation_errors()
+        assert len(result.invalid_rows) == 1
+        assert result.invalid_rows[0].number == 2  # Row number of the row with the error
+        assert result.invalid_rows[0].error_dict == {
+            "id": [f"An app user with ID {app_user2.id} does not exist in the current project."]
+        }
+
+        # Nothing should be updated, including the user belonging to the correct project
+        app_user.refresh_from_db()
+        assert app_user.name == "12345"
+        assert app_user.central_id == 1
+        assert (
+            list(app_user.app_user_template_variables.values_list("value", flat=True))
+            == ["test"] * 4
+        )
+
+        app_user2.refresh_from_db()
+        assert app_user2.name == "67890"
+        assert app_user2.central_id == 2
+        assert app_user2.app_user_template_variables.count() == 0
+
+    def test_blank_template_variables_deleted(self, app_user):
+        assert app_user.app_user_template_variables.count() == 4
+
+        csv_data = (
+            "id,name,central_id,center_id,center_label,public_key,manager_password\n"
+            f"{app_user.id},11031,31,11031,Center 11031,,"
+        )
+
+        dataset = Dataset().load(csv_data)
+        resource = import_export.AppUserResource(app_user.project)
+        result = resource.import_data(
+            dataset, use_transactions=True, rollback_on_validation_errors=True
+        )
+
+        assert not result.has_validation_errors()
+        assert not result.has_errors()
+
+        # Ensure "public_key" and "manager_password" variables have been deleted
+        # for the user, while "center_id" and "center_label" have been updated
+        variables = dict(
+            app_user.app_user_template_variables.values_list("template_variable__name", "value")
+        )
+        assert len(variables) == 2
+        assert variables["center_id"] == "11031"
+        assert variables["center_label"] == "Center 11031"
+
+    def test_validation_errors(self, app_user):
+        # CSV with 6 invalid rows and 1 valid row
+        csv_data = (
+            "id,name,central_id,center_id,center_label,public_key,manager_password\n"
+            f"{app_user.id},,,,,,\n"  # Existing user has no name and no central_id
+            f",new,,,,,\n"  # New user has name but no central_id
+            ",,1,,,,\n"  # New user has central_id but no name
+            ",new1,xx,,,,\n"  # New user has a non-integer central_id
+            f",new2,2,{'1' * 1025},,,\n"  # New user has a center_id with more than 1024 characters
+            f",{app_user.name},2,,,,\n"  # New user has the same name as the existing user
+            ",new3,3,,,,\n"  # New user has both name and central_id, so is valid
+        )
+
+        dataset = Dataset().load(csv_data)
+        resource = import_export.AppUserResource(app_user.project)
+        result = resource.import_data(
+            dataset, use_transactions=True, rollback_on_validation_errors=True
+        )
+        expected_errors = [
+            (
+                1,
+                {
+                    "central_id": ["This field cannot be null."],
+                    "name": ["This field cannot be blank."],
+                },
+            ),
+            (2, {"central_id": ["This field cannot be null."]}),
+            (3, {"name": ["This field cannot be blank."]}),
+            (4, {"central_id": ["Value must be an integer."]}),
+            (5, {"center_id": ["Ensure this value has at most 1024 characters (it has 1025)."]}),
+            (6, {"__all__": ["App user with this Project and Name already exists."]}),
+        ]
+
+        assert result.has_validation_errors()
+        assert len(result.invalid_rows) == 6
+
+        for index, (row_number, row_errors) in enumerate(expected_errors):
+            assert result.invalid_rows[index].number == row_number
+            assert result.invalid_rows[index].error_dict == row_errors