Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project: suggest a simple config file on project import wizard #10356

Merged
merged 13 commits into from
Jun 6, 2023
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/')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed because it got tested in other test cases?

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
52 changes: 52 additions & 0 deletions readthedocs/templates/projects/import_config.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{% 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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later, we should start maintaining the code for these examples in the same docs folders. We could for instance symlink such a file into a Django template folder and then include it. And build it so we know it works :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we need a better way to track these config file suggestions. Ideally, we should use the same everywhere when possible (docs, import step, dashboard, etc)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this is merged, it's possible to unify the example file over in #10301 - I'm going to try and see how it works.

</pre>
</p>

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