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

Allow only post requests for delete views #6242

Merged
merged 3 commits into from
Oct 9, 2019
Merged

Allow only post requests for delete views #6242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d8813b
allow only post request for delete views
saadmk11 Oct 3, 2019
5b1bd80
Fixed tests
saadmk11 Oct 3, 2019
2df26fd
comment added
saadmk11 Oct 9, 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
14 changes: 4 additions & 10 deletions readthedocs/projects/views/private.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ def form_valid(self, form):

class ProjectVersionDeleteHTML(ProjectVersionMixin, GenericModelView):

http_method_names = ['get', 'post']
http_method_names = ['post']

def get(self, request, *args, **kwargs):
def post(self, request, *args, **kwargs):
version = self.get_object()
if not version.active:
version.built = False
Expand All @@ -233,9 +233,6 @@ def get(self, request, *args, **kwargs):
)
return HttpResponseRedirect(self.get_success_url())

def post(self, request, *args, **kwargs):
return self.get(request, *args, **kwargs)


class ImportWizardView(
ProjectImportMixin, ProjectSpamMixin, PrivateViewMixin,
Expand Down Expand Up @@ -640,17 +637,14 @@ def get_context_data(self, **kwargs):

class ProjectTranslationsDelete(ProjectTranslationsMixin, GenericView):

http_method_names = ['get', 'post']
http_method_names = ['post']

def get(self, request, *args, **kwargs):
def post(self, request, *args, **kwargs):
project = self.get_project()
translation = self.get_translation(kwargs['child_slug'])
project.translations.remove(translation)
return HttpResponseRedirect(self.get_success_url())

def post(self, request, *args, **kwargs):
return self.get(request, *args, **kwargs)

def get_translation(self, slug):
project = self.get_project()
translation = get_object_or_404(
Expand Down
8 changes: 4 additions & 4 deletions readthedocs/rtd_tests/tests/test_privacy_urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,6 @@ class PrivateProjectAdminAccessTest(PrivateProjectMixin, TestCase):
'/dashboard/import/manual/demo/': {'status_code': 302},
'/dashboard/pip/': {'status_code': 301},
'/dashboard/pip/subprojects/delete/sub/': {'status_code': 302},
'/dashboard/pip/translations/delete/sub/': {'status_code': 302},

# This depends on an inactive project
'/dashboard/pip/version/latest/delete_html/': {'status_code': 400},

# 405's where we should be POST'ing
'/dashboard/pip/users/delete/': {'status_code': 405},
Expand All @@ -252,6 +248,8 @@ class PrivateProjectAdminAccessTest(PrivateProjectMixin, TestCase):
'/dashboard/pip/integrations/{integration_id}/sync/': {'status_code': 405},
'/dashboard/pip/integrations/{integration_id}/delete/': {'status_code': 405},
'/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/': {'status_code': 405},
'/dashboard/pip/translations/delete/sub/': {'status_code': 405},
'/dashboard/pip/version/latest/delete_html/': {'status_code': 405},
}

def get_url_path_ctx(self):
Expand Down Expand Up @@ -288,6 +286,8 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase):
'/dashboard/pip/integrations/{integration_id}/sync/': {'status_code': 405},
'/dashboard/pip/integrations/{integration_id}/delete/': {'status_code': 405},
'/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/': {'status_code': 405},
'/dashboard/pip/translations/delete/sub/': {'status_code': 405},
'/dashboard/pip/version/latest/delete_html/': {'status_code': 405},
}

# Filtered out by queryset on projects that we don't own.
Expand Down
7 changes: 6 additions & 1 deletion readthedocs/templates/projects/project_translations.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ <h3> {% trans "Existing Translations" %} </h3>
{{ lang_project.name }} ({{ lang_project.get_language_display }})
</a>
<ul class="module-item-menu">
<li><a href="{% url "projects_translations_delete" project.slug lang_project.slug %}">{% trans "Remove" %}</a></li>
<li>
<form method="post" action="{% url "projects_translations_delete" project.slug lang_project.slug %}">
{% csrf_token %}
<input type="submit" value="{% trans 'Remove' %}">
</form>
</li>
</ul>
</li>
{% empty %}
Expand Down
7 changes: 5 additions & 2 deletions readthedocs/templates/projects/project_version_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ <h2> Editing {{ version.slug }} </h2>

{% if request.user|is_admin:project %}
{% if not version.active and version.built %}
<form name="version_delete_html" method="post" action="{% url "project_version_delete_html" project.slug version.slug %}">
{% csrf_token %}
</form>
<p class="empty">
{% url "project_version_delete_html" project.slug version.slug as version_delete_url %}
{# We are submitting the form using javascript because it breaks the UI design if we use buttons #}
{% blocktrans trimmed %}
This version is inactive but its documentation is still available online.
You can <a href="{{ version_delete_url }}">delete this version's documentation</a> if you want to remove it completely.
You can <a href="#" onclick="document.forms['version_delete_html'].submit();">delete this version's documentation</a> if you want to remove it completely.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using this here because using a button here doesn't look good also it breaks lot of CSS styles. Would love to get some Suggestions. :)

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a suggestion for this, and also this is something that users shouldn't hit that much.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense for now.

I'd appreciate if you add this comment into the template itself (using {# #}) so we know why this done in this way in the future. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@humitos Added comment.

{% endblocktrans %}
</p>
{% endif %}
Expand Down