From 5325ef18fef03945ba67d85bbcfd96914a0953b3 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 25 May 2023 17:13:08 +0200 Subject: [PATCH 01/13] Project: suggest a simple config file on project import wizard As part of the "Import Wizard" steps, we add an extra step now that shows a simple suggestion for a config file v2 (the same we currently have in our documentation) that uses `build.os: ubuntu-22.04` and `build.tools.python: "3.11"`. This is an initial work to show the value we can add to this wizard with something pretty simple. There is more work required on the copy for this intermediate step and the UX (I added a checkbox for now to force the user to read the message, not ideal, but works for now). Also, it would be good to find a way to highlight the YAML syntaxis using nice colors and add more useful copy to that intermediate page. Related #10352 --- readthedocs/projects/forms.py | 15 ++++++ readthedocs/projects/views/private.py | 10 ++-- .../templates/projects/import_config.html | 50 +++++++++++++++++++ 3 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 readthedocs/templates/projects/import_config.html diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index a962cb5e90e..cb3c64424cf 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -191,6 +191,21 @@ def clean_tags(self): return tags +class ProjectConfigForm(forms.Form): + + """Simple intermediate step to communicate about the .readthedocs.yaml file.""" + + confirm = forms.BooleanField( + help_text="I've already added a '.readthedocs.yaml' file to my project", + required=True, + ) + + def __init__(self, *args, **kwargs): + # Remove 'user' field since it's not expected by BaseForm. + kwargs.pop("user") + super().__init__(*args, **kwargs) + + class ProjectAdvancedForm(ProjectTriggerBuildMixin, ProjectForm): """Advanced project option form.""" diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 368798bd7c0..c6ef3e0a8d8 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -53,6 +53,7 @@ ProjectAdvancedForm, ProjectAdvertisingForm, ProjectBasicsForm, + ProjectConfigForm, ProjectExtraForm, ProjectRelationshipForm, RedirectForm, @@ -260,8 +261,9 @@ class ImportWizardView(ProjectImportMixin, PrivateViewMixin, SessionWizardView): """ form_list = [ - ('basics', ProjectBasicsForm), - ('extra', ProjectExtraForm), + ("basics", ProjectBasicsForm), + ("config", ProjectConfigForm), + ("extra", ProjectExtraForm), ] condition_dict = {'extra': lambda self: self.is_advanced()} @@ -315,9 +317,7 @@ def done(self, form_list, **kwargs): """ form_data = self.get_all_cleaned_data() extra_fields = ProjectExtraForm.Meta.fields - # expect the first form; manually wrap in a list in case it's a - # View Object, as it is in Python 3. - basics_form = list(form_list)[0] + 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() diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html new file mode 100644 index 00000000000..1ec048061cd --- /dev/null +++ b/readthedocs/templates/projects/import_config.html @@ -0,0 +1,50 @@ +{% extends "projects/import_base.html" %} +{% load i18n %} + +{% block content %} +

{% trans "Project configuration file (.readthedocs.yaml)" %}

+ +

+ {% blocktrans trimmed %} + Make sure your project has a .readthedocs.yaml at the root of your repository. This file is required by Read the Docs to be able to build your documentation. You can read more about this at https://docs.readthedocs.io/en/stable/config-file/v2.html + {% endblocktrans %} +

+ +

+ Here you have an example for a Sphinx project: + +

+# .readthedocs.yaml
+# Read the Docs configuration file
+# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details
+
+# Required
+version: 2
+
+# Set the version of Python and other tools you might need
+build:
+  os: ubuntu-22.04
+  tools:
+    python: "3.11"
+    # You can also specify other tool versions:
+    # nodejs: "19"
+    # rust: "1.64"
+    # golang: "1.19"
+
+# Build documentation in the docs/ directory with Sphinx
+sphinx:
+   configuration: docs/conf.py
+
+# If using Sphinx, optionally build your docs in additional formats such as PDF
+# formats:
+#    - pdf
+
+# Optionally declare the Python requirements required to build your docs
+python:
+   install:
+   - requirements: docs/requirements.txt
+    
+

+ +{{ block.super }} +{% endblock %} From 94262ab7a1d98cfc386f862d2607bef14ec70dce Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 29 May 2023 12:32:25 +0200 Subject: [PATCH 02/13] Template: update link --- readthedocs/templates/projects/import_config.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html index 1ec048061cd..152cb130049 100644 --- a/readthedocs/templates/projects/import_config.html +++ b/readthedocs/templates/projects/import_config.html @@ -6,7 +6,7 @@

{% trans "Project configuration file (.readthedocs.yaml)" %} {% blocktrans trimmed %} - Make sure your project has a .readthedocs.yaml at the root of your repository. This file is required by Read the Docs to be able to build your documentation. You can read more about this at https://docs.readthedocs.io/en/stable/config-file/v2.html + Make sure your project has a .readthedocs.yaml at the root of your repository. This file is required by Read the Docs to be able to build your documentation. You can read more about this in our documentation. {% endblocktrans %}

From 9a819fae6cb81b9f72c3e3f1d9124f1278059ec5 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 29 May 2023 12:39:53 +0200 Subject: [PATCH 03/13] Template: add more style to the wizard config suggestion --- readthedocs/projects/forms.py | 1 + .../templates/projects/import_config.html | 27 +++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index cb3c64424cf..8cd4dbbb4f5 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -196,6 +196,7 @@ class ProjectConfigForm(forms.Form): """Simple intermediate step to communicate about the .readthedocs.yaml file.""" confirm = forms.BooleanField( + label="", help_text="I've already added a '.readthedocs.yaml' file to my project", required=True, ) diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html index 152cb130049..9b1b13dfc37 100644 --- a/readthedocs/templates/projects/import_config.html +++ b/readthedocs/templates/projects/import_config.html @@ -11,9 +11,10 @@

{% trans "Project configuration file (.readthedocs.yaml)" %}

- Here you have an example for a Sphinx project: + Here you have an example for a common Sphinx project: -

+    
+        
 # .readthedocs.yaml
 # Read the Docs configuration file
 # See https://docs.readthedocs.io/en/stable/config-file/v2.html for details
@@ -21,7 +22,7 @@ 

{% trans "Project configuration file (.readthedocs.yaml)" %}{% trans "Project configuration file (.readthedocs.yaml)" %} +# - epub + +# Optionally, but recommended, +# declare the Python requirements required to build your documentation +# See https://docs.readthedocs.io/en/stable/guides/reproducible-builds.html +# python: +# install: +# - requirements: docs/requirements.txt + +

{{ block.super }} From 672620e33def3172df766716f18a771f14e91f0e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 29 May 2023 12:43:52 +0200 Subject: [PATCH 04/13] Template: show the file as code --- readthedocs/projects/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 8cd4dbbb4f5..a4764c05c41 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -197,7 +197,7 @@ class ProjectConfigForm(forms.Form): confirm = forms.BooleanField( label="", - help_text="I've already added a '.readthedocs.yaml' file to my project", + help_text="I've already added a .readthedocs.yaml file to my project", required=True, ) From 4e44e800e08ff2ca767c2978cd3a2212ead09489 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 30 May 2023 12:05:00 +0200 Subject: [PATCH 05/13] Form: swap label/help_text --- readthedocs/projects/forms.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index a4764c05c41..3f67b2c4e80 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -196,8 +196,7 @@ class ProjectConfigForm(forms.Form): """Simple intermediate step to communicate about the .readthedocs.yaml file.""" confirm = forms.BooleanField( - label="", - help_text="I've already added a .readthedocs.yaml file to my project", + label="I've already added a .readthedocs.yaml file to my project", required=True, ) From 9d9226bc65d3380c4f3f74fa3730f10e00b59a2c Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 30 May 2023 13:16:05 +0200 Subject: [PATCH 06/13] Tests: update wizard to add a new step --- .../rtd_tests/tests/test_project_views.py | 80 +++++++++---------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 12194d168c8..2e31682ace5 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -24,40 +24,11 @@ @mock.patch('readthedocs.projects.tasks.builds.update_docs_task', mock.MagicMock()) class TestImportProjectBannedUser(RequestFactoryTestMixin, TestCase): - wizard_class_slug = 'import_wizard_view' url = '/dashboard/import/manual/' - def setUp(self): - super().setUp() - data = { - 'basics': { - 'name': 'foobar', - 'repo': 'http://example.com/foobar', - 'repo_type': 'git', - }, - 'extra': { - 'description': 'Describe foobar', - 'language': 'en', - 'documentation_type': 'sphinx', - }, - } - self.data = {} - for key in data: - self.data.update({('{}-{}'.format(key, k), v) - for (k, v) in list(data[key].items())}) - self.data['{}-current_step'.format(self.wizard_class_slug)] = 'extra' - - def test_not_banned_user(self): - """User without profile and isn't banned.""" - req = self.request(method='post', path='/projects/import', data=self.data) - req.user = get(User, profile=None) - resp = ImportWizardView.as_view()(req) - self.assertEqual(resp.status_code, 302) - self.assertEqual(resp['location'], '/projects/foobar/') - def test_banned_user(self): """User is banned.""" - req = self.request(method='post', path='/projects/import', data=self.data) + req = self.request(method="post", path=self.url, data=self.data) req.user = get(User) req.user.profile.banned = True req.user.profile.save() @@ -81,6 +52,9 @@ def setUp(self): 'repo': 'http://example.com/foobar', 'repo_type': 'git', } + self.step_data["config"] = { + "confirm": True, + } def tearDown(self): Project.objects.filter(slug='foobar').delete() @@ -114,6 +88,10 @@ def test_form_import_from_remote_repo(self): def test_form_pass(self): """Only submit the basics.""" resp = self.post_step('basics') + self.assertEqual(resp.status_code, 200) + + resp = self.post_step("config", session=list(resp._request.session.items())) + self.assertIsInstance(resp, HttpResponseRedirect) self.assertEqual(resp.status_code, 302) self.assertEqual(resp['location'], '/projects/foobar/') @@ -135,6 +113,9 @@ def test_remote_repository_is_added(self): ) self.step_data['basics']['remote_repository'] = remote_repo.pk resp = self.post_step('basics') + self.assertEqual(resp.status_code, 200) + + resp = self.post_step("config", session=list(resp._request.session.items())) self.assertIsInstance(resp, HttpResponseRedirect) self.assertEqual(resp.status_code, 302) self.assertEqual(resp['location'], '/projects/foobar/') @@ -184,7 +165,10 @@ class TestAdvancedForm(TestBasicsForm): def setUp(self): super().setUp() - self.step_data['basics']['advanced'] = True + self.step_data["basics"]["advanced"] = True + self.step_data["config"] = { + "confirm": True, + } self.step_data['extra'] = { 'description': 'Describe foobar', 'language': 'en', @@ -197,6 +181,9 @@ def test_initial_params(self): 'description': 'An amazing project', 'project_url': "https://foo.bar", } + config_initial = { + "confirm": True, + } basic_initial = { 'name': 'foobar', 'repo': 'https://github.com/foo/bar', @@ -204,7 +191,7 @@ def test_initial_params(self): 'default_branch': 'main', 'remote_repository': '', } - initial = dict(**extra_initial, **basic_initial) + initial = dict(**extra_initial, **config_initial, **basic_initial) self.client.force_login(self.user) # User selects a remote repo to import. @@ -223,14 +210,21 @@ def test_initial_params(self): 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, '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") + 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) @@ -254,9 +248,11 @@ def test_form_missing_extra(self): # Remove extra data to trigger validation errors self.step_data['extra'] = {} - resp = self.post_step('basics') - self.assertWizardResponse(resp, 'extra') - resp = self.post_step('extra', session=list(resp._request.session.items())) + 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') @@ -270,10 +266,12 @@ def test_remote_repository_is_added(self): user=self.user, account=socialaccount ) - self.step_data['basics']['remote_repository'] = remote_repo.pk - resp = self.post_step('basics') - self.assertWizardResponse(resp, 'extra') - resp = self.post_step('extra', session=list(resp._request.session.items())) + self.step_data["basics"]["remote_repository"] = remote_repo.pk + 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/') From 9eeeb5fe440b7f75d59ff749e5da91e3ae7f56fc Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 31 May 2023 09:37:25 +0200 Subject: [PATCH 07/13] Remove checkbox from suggested YAML file page --- readthedocs/projects/forms.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 3f67b2c4e80..3d236d51321 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -195,11 +195,6 @@ class ProjectConfigForm(forms.Form): """Simple intermediate step to communicate about the .readthedocs.yaml file.""" - confirm = forms.BooleanField( - label="I've already added a .readthedocs.yaml file to my project", - required=True, - ) - def __init__(self, *args, **kwargs): # Remove 'user' field since it's not expected by BaseForm. kwargs.pop("user") From 4090fa0fe16fb39c496e4f1b3c206beb3ae44b38 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 31 May 2023 09:37:47 +0200 Subject: [PATCH 08/13] Suggested YAML file CSS style --- readthedocs/templates/projects/import_config.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html index 9b1b13dfc37..03b246f73bf 100644 --- a/readthedocs/templates/projects/import_config.html +++ b/readthedocs/templates/projects/import_config.html @@ -13,7 +13,7 @@

{% trans "Project configuration file (.readthedocs.yaml)" %} Here you have an example for a common Sphinx project: -
+    
         
 # .readthedocs.yaml
 # Read the Docs configuration file

From e9b78db0cf4960c1c56424f831b7d2ff5dd81915 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann 
Date: Wed, 31 May 2023 09:42:39 +0200
Subject: [PATCH 09/13] Test: re-add `self.data` to test class

---
 .../rtd_tests/tests/test_project_views.py     | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py
index 2e31682ace5..fa6637f2493 100644
--- a/readthedocs/rtd_tests/tests/test_project_views.py
+++ b/readthedocs/rtd_tests/tests/test_project_views.py
@@ -26,6 +26,27 @@ class TestImportProjectBannedUser(RequestFactoryTestMixin, TestCase):
 
     url = '/dashboard/import/manual/'
 
+    def setUp(self):
+        super().setUp()
+        data = {
+            "basics": {
+                "name": "foobar",
+                "repo": "http://example.com/foobar",
+                "repo_type": "git",
+            },
+            "extra": {
+                "description": "Describe foobar",
+                "language": "en",
+                "documentation_type": "sphinx",
+            },
+        }
+        self.data = {}
+        for key in data:
+            self.data.update(
+                {("{}-{}".format(key, k), v) for (k, v) in list(data[key].items())}
+            )
+        self.data["{}-current_step".format(self.wizard_class_slug)] = "extra"
+
     def test_banned_user(self):
         """User is banned."""
         req = self.request(method="post", path=self.url, data=self.data)

From 5727471a8a5e8d3b6cf36504f176ec7db8ceb943 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann 
Date: Wed, 31 May 2023 14:37:57 +0200
Subject: [PATCH 10/13] Use CSS class to style the YAML shown at import step

---
 media/css/core.css                                | 4 ++++
 readthedocs/templates/projects/import_config.html | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/media/css/core.css b/media/css/core.css
index fa58300a696..fba3c2eebbd 100644
--- a/media/css/core.css
+++ b/media/css/core.css
@@ -1397,3 +1397,7 @@ div.highlight pre .vc { color: #bb60d5 } /* Name.Variable.Class */
 div.highlight pre .vg { color: #bb60d5 } /* Name.Variable.Global */
 div.highlight pre .vi { color: #bb60d5 } /* Name.Variable.Instance */
 div.highlight pre .il { color: #40a070 } /* Literal.Number.Integer.Long */
+
+pre.small {
+    font-size: 0.85em;
+}
diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html
index 03b246f73bf..4c88906ab7e 100644
--- a/readthedocs/templates/projects/import_config.html
+++ b/readthedocs/templates/projects/import_config.html
@@ -13,7 +13,7 @@ 

{% trans "Project configuration file (.readthedocs.yaml)" %} Here you have an example for a common Sphinx project: -
+    
         
 # .readthedocs.yaml
 # Read the Docs configuration file

From 9bdcbec5419b192b7a13dc0b23d06376c98e08e1 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann 
Date: Wed, 31 May 2023 14:42:03 +0200
Subject: [PATCH 11/13] Add required variable in tests

---
 readthedocs/rtd_tests/tests/test_project_views.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py
index fa6637f2493..c0aa6b1ef19 100644
--- a/readthedocs/rtd_tests/tests/test_project_views.py
+++ b/readthedocs/rtd_tests/tests/test_project_views.py
@@ -24,7 +24,8 @@
 @mock.patch('readthedocs.projects.tasks.builds.update_docs_task', mock.MagicMock())
 class TestImportProjectBannedUser(RequestFactoryTestMixin, TestCase):
 
-    url = '/dashboard/import/manual/'
+    wizard_class_slug = "import_wizard_view"
+    url = "/dashboard/import/manual/"
 
     def setUp(self):
         super().setUp()

From dbb0aa92db36c823595f82f7590697a50fa404ec Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann 
Date: Thu, 1 Jun 2023 09:23:00 +0200
Subject: [PATCH 12/13] Minor style for import config step

---
 readthedocs/templates/projects/import_config.html | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html
index 4c88906ab7e..06b404aa280 100644
--- a/readthedocs/templates/projects/import_config.html
+++ b/readthedocs/templates/projects/import_config.html
@@ -14,8 +14,7 @@ 

{% trans "Project configuration file (.readthedocs.yaml)" %} - -# .readthedocs.yaml + # .readthedocs.yaml # Read the Docs configuration file # See https://docs.readthedocs.io/en/stable/config-file/v2.html for details @@ -41,13 +40,11 @@

{% trans "Project configuration file (.readthedocs.yaml)" %} +# - requirements: docs/requirements.txt

From c9bd407978ac1032917b80e14ba7c6a3e2f4bd3d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 6 Jun 2023 15:50:27 +0200 Subject: [PATCH 13/13] Split phrase to avoid scrolling --- readthedocs/templates/projects/import_config.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/templates/projects/import_config.html b/readthedocs/templates/projects/import_config.html index 06b404aa280..5489d24bb85 100644 --- a/readthedocs/templates/projects/import_config.html +++ b/readthedocs/templates/projects/import_config.html @@ -40,7 +40,8 @@

{% trans "Project configuration file (.readthedocs.yaml)" %}