Import: remove extra/advanced step from project import wizard (#10603)

* Import: remove extra/advanced step from project import wizard

This commit removes the "Extra/Advanced" step and moves the "language" attribute
to the "Basics" step -- the first one, since that field is important.

Closes #10392

* Update tests

Author: humitos

readthedocs/projects/forms.py

@@ -86,21 +86,15 @@ class ProjectBasicsForm(ProjectForm):
 
     class Meta:
         model = Project
-        fields = ("name", "repo", "default_branch")
+        fields = ("name", "repo", "default_branch", "language")
 
     remote_repository = forms.IntegerField(
         widget=forms.HiddenInput(),
         required=False,
     )
 
     def __init__(self, *args, **kwargs):
-        show_advanced = kwargs.pop('show_advanced', False)
         super().__init__(*args, **kwargs)
-        if show_advanced:
-            self.fields['advanced'] = forms.BooleanField(
-                required=False,
-                label=_('Edit advanced project options'),
-            )
         self.fields['repo'].widget.attrs['placeholder'] = self.placehold_repo()
         self.fields['repo'].widget.attrs['required'] = True
 

readthedocs/projects/views/mixins.py

@@ -100,12 +100,11 @@ class ProjectImportMixin:
 
     """Helpers to import a Project."""
 
-    def finish_import_project(self, request, project, tags=None):
+    def finish_import_project(self, request, project):
         """
         Perform last steps to import a project into Read the Docs.
 
         - Add the user from request as maintainer
-        - Set all the tags to the project
         - Send Django Signal
         - Trigger initial build
 
@@ -115,18 +114,11 @@ def finish_import_project(self, request, project, tags=None):
         :param project: Project instance just imported (already saved)
         :param tags: tags to add to the project
         """
-        if not tags:
-            tags = []
-
         project.users.add(request.user)
-        for tag in tags:
-            project.tags.add(tag)
-
         log.info(
             'Project imported.',
             project_slug=project.slug,
             user_username=request.user.username,
-            tags=tags,
         )
 
         # TODO: this signal could be removed, or used for sync task

readthedocs/projects/views/private.py

@@ -54,7 +54,6 @@
     ProjectAdvertisingForm,
     ProjectBasicsForm,
     ProjectConfigForm,
-    ProjectExtraForm,
     ProjectRelationshipForm,
     RedirectForm,
     TranslationForm,
@@ -262,9 +261,7 @@ class ImportWizardView(ProjectImportMixin, PrivateViewMixin, SessionWizardView):
     form_list = [
         ("basics", ProjectBasicsForm),
         ("config", ProjectConfigForm),
-        ("extra", ProjectExtraForm),
     ]
-    condition_dict = {'extra': lambda self: self.is_advanced()}
 
     initial_dict_key = 'initial-data'
 
@@ -298,8 +295,6 @@ def get_form_kwargs(self, step=None):
         """Get args to pass into form instantiation."""
         kwargs = {}
         kwargs['user'] = self.request.user
-        if step == 'basics':
-            kwargs['show_advanced'] = True
         return kwargs
 
     def get_template_names(self):
@@ -314,32 +309,17 @@ def done(self, form_list, **kwargs):
         other side effects for now, by signalling a save without commit. Then,
         finish by added the members to the project and saving.
         """
-        form_data = self.get_all_cleaned_data()
-        extra_fields = ProjectExtraForm.Meta.fields
         basics_form = form_list[0]
         # Save the basics form to create the project instance, then alter
         # attributes directly from other forms
         project = basics_form.save()
 
-        # Remove tags to avoid setting them in raw instead of using ``.add``
-        tags = form_data.pop('tags', [])
-
-        for field, value in list(form_data.items()):
-            if field in extra_fields:
-                setattr(project, field, value)
-        project.save()
-
-        self.finish_import_project(self.request, project, tags)
+        self.finish_import_project(self.request, project)
 
         return HttpResponseRedirect(
             reverse('projects_detail', args=[project.slug]),
         )
 
-    def is_advanced(self):
-        """Determine if the user selected the `show advanced` field."""
-        data = self.get_cleaned_data_for_step('basics') or {}
-        return data.get('advanced', True)
-
 
 class ImportView(PrivateViewMixin, TemplateView):
 

readthedocs/rtd_tests/tests/test_project_forms.py

@@ -74,28 +74,31 @@ def test_import_repo_url(self):
         with override_settings(ALLOW_PRIVATE_REPOS=False):
             for url, valid in public_urls:
                 initial = {
-                    'name': 'foo',
-                    'repo_type': 'git',
-                    'repo': url,
+                    "name": "foo",
+                    "repo_type": "git",
+                    "repo": url,
+                    "language": "en",
                 }
                 form = ProjectBasicsForm(initial)
                 self.assertEqual(form.is_valid(), valid, msg=url)
 
         with override_settings(ALLOW_PRIVATE_REPOS=True):
             for url, valid in private_urls:
                 initial = {
-                    'name': 'foo',
-                    'repo_type': 'git',
-                    'repo': url,
+                    "name": "foo",
+                    "repo_type": "git",
+                    "repo": url,
+                    "language": "en",
                 }
                 form = ProjectBasicsForm(initial)
                 self.assertEqual(form.is_valid(), valid, msg=url)
 
     def test_empty_slug(self):
         initial = {
-            'name': "''",
-            'repo_type': 'git',
-            'repo': 'https://github.com/user/repository',
+            "name": "''",
+            "repo_type": "git",
+            "repo": "https://github.com/user/repository",
+            "language": "en",
         }
         form = ProjectBasicsForm(initial)
         self.assertFalse(form.is_valid())
@@ -112,9 +115,10 @@ def test_changing_vcs_should_not_change_latest_is_not_none(self):
 
         form = ProjectBasicsForm(
             {
-                'repo': 'http://github.com/test/test',
-                'name': 'name',
-                'repo_type': REPO_TYPE_GIT,
+                "repo": "http://github.com/test/test",
+                "name": "name",
+                "repo_type": REPO_TYPE_GIT,
+                "language": "en",
             },
             instance=project,
         )
@@ -144,11 +148,14 @@ def test_length_of_tags(self):
         self.assertDictEqual(form.errors, {'tags': [error_msg]})
 
     def test_strip_repo_url(self):
-        form = ProjectBasicsForm({
-            'name': 'foo',
-            'repo_type': 'git',
-            'repo': 'https://github.com/rtfd/readthedocs.org/'
-        })
+        form = ProjectBasicsForm(
+            {
+                "name": "foo",
+                "repo_type": "git",
+                "repo": "https://github.com/rtfd/readthedocs.org/",
+                "language": "en",
+            }
+        )
         self.assertTrue(form.is_valid())
         self.assertEqual(
             form.cleaned_data['repo'],

readthedocs/rtd_tests/tests/test_project_views.py

@@ -69,10 +69,11 @@ class TestBasicsForm(WizardTestCase):
 
     def setUp(self):
         self.user = get(User)
-        self.step_data['basics'] = {
-            'name': 'foobar',
-            'repo': 'http://example.com/foobar',
-            'repo_type': 'git',
+        self.step_data["basics"] = {
+            "name": "foobar",
+            "repo": "http://example.com/foobar",
+            "repo_type": "git",
+            "language": "en",
         }
         self.step_data["config"] = {
             "confirm": True,
@@ -199,10 +200,6 @@ def setUp(self):
         }
 
     def test_initial_params(self):
-        extra_initial = {
-            'description': 'An amazing project',
-            'project_url': "https://foo.bar",
-        }
         config_initial = {
             "confirm": True,
         }
@@ -213,7 +210,7 @@ def test_initial_params(self):
             'default_branch': 'main',
             'remote_repository': '',
         }
-        initial = dict(**extra_initial, **config_initial, **basic_initial)
+        initial = dict(**config_initial, **basic_initial)
         self.client.force_login(self.user)
 
         # User selects a remote repo to import.
@@ -223,61 +220,17 @@ def test_initial_params(self):
         form = resp.context_data['form']
         self.assertEqual(form.initial, basic_initial)
 
-        # User selects advanced.
-        basic_initial['advanced'] = True
-        step_data = {
-            f'basics-{k}': v
-            for k, v in basic_initial.items()
-        }
-        step_data[f'{self.wizard_class_slug}-current_step'] = 'basics'
-        resp = self.client.post(self.url, step_data)
-
-        step_data = {f"config-{k}": v for k, v in config_initial.items()}
-        step_data[f"{self.wizard_class_slug}-current_step"] = "config"
-        resp = self.client.post(self.url, step_data)
-
-        # The correct initial data for the advanced form is set.
-        form = resp.context_data['form']
-        self.assertEqual(form.initial, extra_initial)
-
     def test_form_pass(self):
         """Test all forms pass validation."""
         resp = self.post_step("basics")
         self.assertWizardResponse(resp, "config")
         resp = self.post_step("config", session=list(resp._request.session.items()))
-        self.assertWizardResponse(resp, "extra")
-        self.assertEqual(resp.status_code, 200)
-        resp = self.post_step('extra', session=list(resp._request.session.items()))
         self.assertIsInstance(resp, HttpResponseRedirect)
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(resp['location'], '/projects/foobar/')
 
         proj = Project.objects.get(name='foobar')
         self.assertIsNotNone(proj)
-        data = self.step_data['basics']
-        del data['advanced']
-        del self.step_data['extra']['tags']
-        self.assertCountEqual(
-            [tag.name for tag in proj.tags.all()],
-            ['bar', 'baz', 'foo'],
-        )
-        data.update(self.step_data['extra'])
-        for (key, val) in list(data.items()):
-            self.assertEqual(getattr(proj, key), val)
-
-    def test_form_missing_extra(self):
-        """Submit extra form with missing data, expect to get failures."""
-        # Remove extra data to trigger validation errors
-        self.step_data['extra'] = {}
-
-        resp = self.post_step("basics")
-        self.assertWizardResponse(resp, "config")
-        resp = self.post_step("config", session=list(resp._request.session.items()))
-        self.assertWizardResponse(resp, "extra")
-        resp = self.post_step("extra", session=list(resp._request.session.items()))
-
-        self.assertWizardFailure(resp, 'language')
-        self.assertWizardFailure(resp, 'documentation_type')
 
     def test_remote_repository_is_added(self):
         remote_repo = get(RemoteRepository, default_branch="default-branch")
@@ -292,8 +245,6 @@ def test_remote_repository_is_added(self):
         resp = self.post_step("basics")
         self.assertWizardResponse(resp, "config")
         resp = self.post_step("config", session=list(resp._request.session.items()))
-        self.assertWizardResponse(resp, "extra")
-        resp = self.post_step("extra", session=list(resp._request.session.items()))
         self.assertIsInstance(resp, HttpResponseRedirect)
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(resp['location'], '/projects/foobar/')