Skip to content

Commit

Permalink
Project: suggest a simple config file on project import wizard (#10356)
Browse files Browse the repository at this point in the history
* 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

* Template: update link

* Template: add more style to the wizard config suggestion

* Template: show the file as code

* Form: swap label/help_text

* Tests: update wizard to add a new step

* Remove checkbox from suggested YAML file page

* Suggested YAML file CSS style

* Test: re-add `self.data` to test class

* Use CSS class to style the YAML shown at import step

* Add required variable in tests

* Minor style for import config step

* Split phrase to avoid scrolling
  • Loading branch information
humitos authored Jun 6, 2023
1 parent be72682 commit 911234e
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 38 deletions.
4 changes: 4 additions & 0 deletions media/css/core.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
10 changes: 10 additions & 0 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,16 @@ def clean_tags(self):
return tags


class ProjectConfigForm(forms.Form):

"""Simple intermediate step to communicate about the .readthedocs.yaml file."""

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."""
Expand Down
10 changes: 5 additions & 5 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
ProjectAdvancedForm,
ProjectAdvertisingForm,
ProjectBasicsForm,
ProjectConfigForm,
ProjectExtraForm,
ProjectRelationshipForm,
RedirectForm,
Expand Down Expand Up @@ -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()}

Expand Down Expand Up @@ -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()
Expand Down
86 changes: 53 additions & 33 deletions readthedocs/rtd_tests/tests/test_project_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,40 +24,33 @@
@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/'
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',
"basics": {
"name": "foobar",
"repo": "http://example.com/foobar",
"repo_type": "git",
},
'extra': {
'description': 'Describe foobar',
'language': 'en',
'documentation_type': 'sphinx',
"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/')
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='/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()
Expand All @@ -81,6 +74,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()
Expand Down Expand Up @@ -114,6 +110,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/')
Expand All @@ -135,6 +135,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/')
Expand Down Expand Up @@ -184,7 +187,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',
Expand All @@ -197,14 +203,17 @@ 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',
'repo_type': 'git',
'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.
Expand All @@ -223,14 +232,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)
Expand All @@ -254,9 +270,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')
Expand All @@ -270,10 +288,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/')
Expand Down
53 changes: 53 additions & 0 deletions readthedocs/templates/projects/import_config.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{% extends "projects/import_base.html" %}
{% load i18n %}

{% block content %}
<h3>{% trans "Project configuration file (<code>.readthedocs.yaml</code>)" %}</h3>

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

<p class="info">
Here you have an example for a common Sphinx project:

<pre class="small">
<code># .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 OS, Python version 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

# Optionally build your docs in additional formats such as PDF and ePub
# formats:
# - pdf
# - 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</code>
</pre>
</p>

{{ block.super }}
{% endblock %}

0 comments on commit 911234e

Please sign in to comment.