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.

Sign up for GitHub

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

Jump to bottom

Regroup advanced settings #5489

Merged
merged 8 commits into from
Mar 22, 2019
Merged

Regroup advanced settings #5489

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8138b37
Add crispy forms
stsewd Mar 19, 2019
59a2577
Add fieldset
stsewd Mar 19, 2019
a74e12a
Merge branch 'master' into use-crispy-forms-advanced-settings
stsewd Mar 19, 2019
ca83b9f
Better wording
stsewd Mar 19, 2019
94d1f23
Grammar
davidfischer Mar 21, 2019
4b3c621
Point to v2
stsewd Mar 22, 2019
72ade58
Merge branch 'master' into use-crispy-forms-advanced-settings
stsewd Mar 22, 2019
746fd52
Other -> Default
ericholscher Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 27 additions & 17 deletions readthedocs/projects/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
# -*- coding: utf-8 -*-

"""Project forms."""

from random import choice
from re import fullmatch
from urllib.parse import urlparse

from crispy_forms.helper import FormHelper
from crispy_forms.layout import Fieldset, Layout, HTML
from django import forms
from django.conf import settings
from django.contrib.auth.models import User
Expand All @@ -22,7 +21,6 @@
from readthedocs.oauth.models import RemoteRepository
from readthedocs.projects import constants
from readthedocs.projects.exceptions import ProjectSpamError
from readthedocs.projects.templatetags.projects_tags import sort_version_aware
from readthedocs.projects.models import (
Domain,
EmailHook,
Expand All @@ -32,6 +30,7 @@
ProjectRelationship,
WebHook,
)
from readthedocs.projects.templatetags.projects_tags import sort_version_aware
from readthedocs.redirects.models import Redirect


Expand Down Expand Up @@ -196,27 +195,18 @@ class ProjectAdvancedForm(ProjectTriggerBuildMixin, ProjectForm):

"""Advanced project option form."""

python_interpreter = forms.ChoiceField(
choices=constants.PYTHON_CHOICES,
initial='python',
help_text=_(
'The Python interpreter used to create the virtual '
'environment.',
),
)

class Meta:
model = Project
fields = (
# Global settings.
per_project_settings = (
'default_version',
'default_branch',
'privacy_level',
'analytics_code',
'show_version_warning',
'single_version',

# Options that can be set per-version using a config file.
)
# These that can be set per-version using a config file.
per_version_settings = (
'documentation_type',
'requirements_file',
'python_interpreter',
Expand All @@ -226,10 +216,30 @@ class Meta:
'enable_pdf_build',
'enable_epub_build',
)
fields = (
*per_project_settings,
*per_version_settings,
)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

self.helper = FormHelper()
help_text = render_to_string(
'projects/project_advanced_settings_helptext.html'
)
self.helper.layout = Layout(
Fieldset(
_("Global settings"),
*self.Meta.per_project_settings,
),
Fieldset(
_("Other settings"),
stsewd marked this conversation as resolved.
Show resolved Hide resolved
HTML(help_text),
*self.Meta.per_version_settings,
),
)

default_choice = (None, '-' * 9)
all_versions = self.instance.versions.values_list(
'identifier',
Expand Down
1 change: 1 addition & 0 deletions readthedocs/settings/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ def INSTALLED_APPS(self): # noqa
'textclassifier',
'annoying',
'django_extensions',
'crispy_forms',
'messages_extends',
'django_elasticsearch_dsl',

Expand Down
3 changes: 2 additions & 1 deletion readthedocs/templates/projects/project_advanced.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{% extends "projects/project_edit_base.html" %}
{% load i18n %}
{% load crispy_forms_tags %}

{% block project-advanced-active %}active{% endblock %}
{% block nav-dashboard %} class="active"{% endblock %}
Expand All @@ -13,7 +14,7 @@

{% block project_edit_content %}
<form method="post" action=".">{% csrf_token %}
{{ form.as_p }}
{% crispy form %}
<p>
<input style="display: inline;" type="submit" value="{% trans "Save" %}">
Copy link
Member

Choose a reason for hiding this comment

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

@stsewd
{% crispy form %} renders

<form>
  <!-- csrf token -->
  <!-- content -->
</form>

And we have wrapped this in outer <form> tag and it is finally rendering something like this.
Screenshot from 2019-03-27 21-50-05

and so the Save button, which is rendered outside <form> tag, is not working anymore. We need to fix this before merging this in rel branch

docs: https://django-crispy-forms.readthedocs.io/en/latest/crispy_tag_forms.html#fundamentals

Copy link
Member Author

Choose a reason for hiding this comment

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

#5544

</p>
Expand Down
10 changes: 10 additions & 0 deletions readthedocs/templates/projects/project_advanced_settings_helptext.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% load i18n %}
<p class="empty">
Copy link
Member Author

Choose a reason for hiding this comment

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

Used the same patter that we have in settings https://readthedocs.org/dashboard/docs/edit/

{% blocktrans with config_file_link="https://docs.readthedocs.io/page/config-file" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd link directly to the v2 config file docs: https://docs.readthedocs.io/page/config-file/v2.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, you might consider adding trimmed to the blocktrans tag.

These settings can be configured using a
<a href="{{ config_file_link }}">configuration file</a>.
That's the recommended way of set up your project.
stsewd marked this conversation as resolved.
Show resolved Hide resolved
Settings in the configuration
file override the settings listed here.
{% endblocktrans %}
</p>
1 change: 1 addition & 0 deletions requirements/pip.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ stripe==2.21.0
git+https://github.com/mozilla/unicode-slugify@b696c37#egg=unicode-slugify

django-formtools==2.1
django-crispy-forms==1.7.2

# docker is pinned to 3.1.3 because we found some strange behavior
# related to timeouts on EXEC with 3.2.1 and 3.3.0 that's not present
Expand Down