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

Allow only post requests for delete views #6242

Merged
merged 3 commits into from Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 4 additions & 10 deletions readthedocs/projects/views/private.py
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
7 changes: 6 additions & 1 deletion readthedocs/templates/projects/project_translations.html
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
6 changes: 4 additions & 2 deletions readthedocs/templates/projects/project_version_detail.html
Expand Up @@ -19,11 +19,13 @@ <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 %}
{% 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