Addressing Egene's review requests

cadasta/organization/forms.py

@@ -349,8 +349,8 @@ def clean_questionnaire(self):
         new_form = self.data.get('questionnaire')
         current_form = self.initial.get('questionnaire')
 
-        if (self.instance.has_records and new_form is not None and
-                new_form != current_form):
+        if (new_form is not None and new_form != current_form and
+                self.instance.has_records):
             raise ValidationError(
                 _("Data has already been contributed to this project. To "
                   "ensure data integrity, uploading a new questionnaire is "
@@ -368,7 +368,7 @@ def save(self, *args, **kwargs):
                     original_file=original_file,
                     project=self.instance
                 )
-        elif not self.instance.has_records:
+        elif new_form is not None and not self.instance.has_records:
             self.instance.current_questionnaire = ''
 
         return super().save(*args, **kwargs)

cadasta/organization/tests/test_views_default_projects.py

@@ -935,6 +935,8 @@ class ProjectEditDetailsTest(ViewTestCase, UserTestCase,
 
     def setup_models(self):
         self.project = ProjectFactory.create(current_questionnaire='abc')
+        self.questionnaire = QuestionnaireFactory.create(project=self.project,
+                                                         id='abc')
 
     def setup_url_kwargs(self):
         return {
@@ -945,7 +947,11 @@ def setup_url_kwargs(self):
     def setup_template_context(self):
         return {'project': self.project,
                 'object': self.project,
-                'form': forms.ProjectEditDetails(instance=self.project)}
+                'form': forms.ProjectEditDetails(
+                    instance=self.project,
+                    initial={'questionnaire': self.questionnaire.xls_form.url,
+                             'original_file': self.questionnaire.original_file}
+                )}
 
     def test_get_with_authorized_user(self):
         user = UserFactory.create()
@@ -956,6 +962,20 @@ def test_get_with_authorized_user(self):
         assert response.content == self.expected_content
         assert 'Select the questionnaire' in self.expected_content
 
+    def test_get_empty_questionnaire_with_authorized_user(self):
+        user = UserFactory.create()
+        assign_policies(user)
+
+        self.project.current_questionnaire = ''
+        self.project.save()
+
+        form = forms.ProjectEditDetails(instance=self.project)
+
+        response = self.request(user=user)
+        assert response.status_code == 200
+        assert response.content == self.render_content(form=form)
+        assert 'Select the questionnaire' in self.expected_content
+
     def test_get_with_blocked_questionnaire_upload(self):
         user = UserFactory.create()
         assign_policies(user)
@@ -1028,15 +1048,14 @@ def test_post_empty_questionnaire_with_blocked_questionnaire_upload(self):
         SpatialUnitFactory.create(project=self.project)
         user = UserFactory.create()
         assign_policies(user)
-        post_data = self.post_data.copy()
-        del post_data['questionnaire']
-        assert 'questionnaire' not in post_data.keys()
-        response = self.request(user=user, method='POST')
+        response = self.request(user=user, method='POST',
+                                post_data={'questionnaire': None})
 
-        assert response.status_code == 200
+        assert response.status_code == 302
+        assert self.expected_success_url in response.location
         self.project.refresh_from_db()
-        assert self.project.name != self.post_data['name']
-        assert self.project.description != self.post_data['description']
+        assert self.project.name == self.post_data['name']
+        assert self.project.description == self.post_data['description']
         assert self.project.current_questionnaire == 'abc'
 
     def test_post_invalid_form(self):