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

Do not allow the creation of new tags #6326

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 3 additions & 6 deletions kitsune/questions/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,12 +387,9 @@ def add_tags(self, request, pk=None):
try:
add_existing_tag(tag, question.tags)
except Tag.DoesNotExist:
if request.user.has_perm("taggit.add_tag"):
question.tags.add(tag)
else:
raise GenericAPIException(
status.HTTP_403_FORBIDDEN, "You are not authorized to create new tags."
)
raise GenericAPIException(
status.HTTP_403_FORBIDDEN, "You are not authorized to create new tags."
)

data = [{"name": tag.name, "slug": tag.slug} for tag in question.tags.all()]
return Response(data)
Expand Down
6 changes: 3 additions & 3 deletions kitsune/questions/jinja2/questions/question_details.html
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ <h3 class="sumo-card-heading">{{ _('Post a Reply') }}</h3>
{% set tags = question.my_tags %}
{% if tags or can_tag %}
<div class="sidebox tight cf" id="tags">
<div class="tags"{% if can_tag %} data-tag-vocab-json="{{ tag_vocab() }}"{% endif %}{% if can_create_tags %} data-can-create-tags="1"{% endif %}>
<div class="tags"{% if can_tag %} data-tag-vocab-json="{{ tag_vocab() }}"{% endif %}>
{% if can_tag %}
<form action="{{ url('questions.remove_tag', question.id) }}"
data-action-async="{{ url('questions.remove_tag_async', question.id) }}"
Expand Down Expand Up @@ -422,12 +422,12 @@ <h3 class="sumo-card-heading">{{ _('Post a Reply') }}</h3>
class="tag-adder">
{% csrf_token %}
<div class="field is-condensed">
<label for="id_tag_input">{{ _('Add a tag') }}:</label>
<label for="id_tag_input">{{ _('Apply a tag') }}:</label>
<input id="id_tag_input" type="text" name="tag-name" size="12" maxlength="100"
class="searchbox autocomplete-tags {% if tag_adding_error %} invalid{% endif %}"
value="{{ tag_adding_value }}" />
</div>
<input class="sumo-button button-sm primary-button" type="submit" value="{{ _('Add') }}" />
<input class="sumo-button button-sm primary-button" type="submit" value="{{ _('Apply') }}" />
</form>
{% endif %}
</div>
Expand Down
18 changes: 0 additions & 18 deletions kitsune/questions/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from actstream.models import Follow
from rest_framework.exceptions import APIException
from rest_framework.test import APIClient
from taggit.models import Tag

from kitsune.products.tests import ProductFactory, TopicFactory
from kitsune.questions import api
Expand Down Expand Up @@ -472,23 +471,6 @@ def test_unfollow(self):
self.assertEqual(res.status_code, 204)
self.assertEqual(Follow.objects.filter(user=u).count(), 0)

def test_add_tags(self):
q = QuestionFactory()
self.assertEqual(0, q.tags.count())

u = UserFactory()
add_permission(u, Tag, "add_tag")
add_permission(u, Question, "tag_question")
self.client.force_authenticate(user=u)

res = self.client.post(
reverse("question-add-tags", args=[q.id]),
content_type="application/json",
data=json.dumps({"tags": ["test", "more", "tags"]}),
)
self.assertEqual(res.status_code, 200)
self.assertEqual(3, q.tags.count())

def test_remove_tags_without_perms(self):
q = QuestionFactory()
q.tags.add("test")
Expand Down
12 changes: 3 additions & 9 deletions kitsune/questions/tests/test_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -971,20 +971,14 @@ def setUp(self):
self.question = QuestionFactory()
TagFactory(name="red", slug="red")

def test_add_new_tag(self):
"""Assert adding a nonexistent tag sychronously creates & adds it."""
self.client.post(_add_tag_url(self.question.id), data={"tag-name": "nonexistent tag"})
tags_eq(Question.objects.get(id=self.question.id), ["nonexistent tag"])

def test_add_async_new_tag(self):
"""Assert adding an nonexistent tag creates & adds it."""
def test_add_async_new_tag_permission_error(self):
"""Assert adding an nonexistent tag returns a permission error."""
response = self.client.post(
_add_async_tag_url(self.question.id),
data={"tag-name": "nonexistent tag"},
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 200)
tags_eq(Question.objects.get(id=self.question.id), ["nonexistent tag"])
self.assertEqual(response.status_code, 400)

def test_add_new_case_insensitive(self):
"""Adding a tag differing only in case from existing ones shouldn't
Expand Down
18 changes: 5 additions & 13 deletions kitsune/questions/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1429,22 +1429,14 @@ def _add_tag(request, question_id):

Tag name (case-insensitive) must be in request.POST['tag-name'].

If there is no such tag and the user is not allowed to make new tags, raise
Tag.DoesNotExist. If no tag name is provided, return None. Otherwise,
return the canonicalized tag name.
If no tag name is provided or Tag.DoesNotExist is raised, return None.
Otherwise, return the canonicalized tag name.

"""
tag_name = request.POST.get("tag-name", "").strip()
if tag_name:
if tag_name := request.POST.get("tag-name", "").strip():
question = get_object_or_404(Question, pk=question_id)
try:
canonical_name = add_existing_tag(tag_name, question.tags)
except Tag.DoesNotExist:
if request.user.has_perm("taggit.add_tag"):
question.tags.add(tag_name) # implicitly creates if needed
canonical_name = tag_name
else:
raise
# This raises Tag.DoesNotExist if the tag doesn't exist.
canonical_name = add_existing_tag(tag_name, question.tags)

return question, canonical_name

Expand Down
51 changes: 25 additions & 26 deletions kitsune/sumo/static/sumo/js/tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import _keys from "underscore/modules/keys";
* Scripts to support tagging.
*/

(function($) {
(function ($) {

// Initialize tagging features.
function init() {
Expand All @@ -20,7 +20,7 @@ import _keys from "underscore/modules/keys";
// the .tags block surrounding each set of add and remove forms.
function initVocab() {
$('div.tags[data-tag-vocab-json]').each(
function() {
function () {
var $tagContainer = $(this);
var parsedVocab = $tagContainer.data('tag-vocab-json');
$tagContainer.data('tagVocab', _keys(parsedVocab));
Expand All @@ -37,7 +37,6 @@ import _keys from "underscore/modules/keys";
var $adder = $addForm.find('input.adder'),
$input = $addForm.find('input.autocomplete-tags'),
$tagsDiv = $input.closest('div.tags'),
canCreateTags = $tagsDiv.data('can-create-tags') !== undefined,
vocab = $tagsDiv.data('tagVocab'),
$tagList = inputToTagList($input);

Expand All @@ -52,7 +51,7 @@ import _keys from "underscore/modules/keys";
inVocab = inArrayCaseInsensitive(tagName, vocab) !== -1,
isOnscreen = tagIsOnscreen(tagName, $tagList);
$adder.attr('disabled', !tagName.length || isOnscreen ||
(!canCreateTags && !inVocab));
(!inVocab));
}

return tendAddButton;
Expand All @@ -69,18 +68,18 @@ import _keys from "underscore/modules/keys";
function vocabCallback(request, response) {
var appliedTags = getAppliedTags($tagList),
vocabMinusApplied = $.grep(vocab,
function(e, i) {
return $.inArray(e, appliedTags) === -1;
}
);
function (e, i) {
return $.inArray(e, appliedTags) === -1;
}
);
response(filter(vocabMinusApplied, request.term));
}

return vocabCallback;
}

$('input.autocomplete-tags').each(
function() {
function () {
var $input = $(this),
tender = makeButtonTender($input.closest('form'));

Expand All @@ -103,11 +102,11 @@ import _keys from "underscore/modules/keys";
function initTagRemoval() {
// Attach a tag-removal function to each clickable "x":
$('div.tags').each(
function() {
function () {
var $div = $(this),
async = !$div.hasClass('deferred');
$div.find('.tag').each(
function() {
function () {
attachRemoverHandlerTo($(this), async);
}
);
Expand All @@ -116,18 +115,18 @@ import _keys from "underscore/modules/keys";

// Prevent the form, if it exists, from submitting so our AJAX handler
// is always called:
$('form.remove-tag-form').on("submit", function() { return false; });
$('form.remove-tag-form').on("submit", function () { return false; });
}

// Attach onclick removal handlers to every .remove element in $tag.
function attachRemoverHandlerTo($container, async) {
$container.find('.remover').on("click",
function() {
$container.find('.remover').on("click",
function () {
var $remover = $(this),
$tag = $remover.closest('.tag'),
tagName = $tag.find('.tag-name').text(),
csrf = $remover.closest('form')
.find('input[name=csrfmiddlewaretoken]').val();
.find('input[name=csrfmiddlewaretoken]').val();

function makeTagDisappear() {
$tag.remove();
Expand All @@ -140,7 +139,7 @@ import _keys from "underscore/modules/keys";
$.ajax({
type: 'POST',
url: $remover.closest('form.remove-tag-form').data('action-async'),
data: {name: tagName, csrfmiddlewaretoken: csrf},
data: { name: tagName, csrfmiddlewaretoken: csrf },
success: makeTagDisappear,
error: function makeTagReappear() {
$tag.removeClass('in-progress');
Expand Down Expand Up @@ -201,7 +200,7 @@ import _keys from "underscore/modules/keys";
$.ajax({
type: 'POST',
url: $container.data('action-async'),
data: {'tag-name': tagName, csrfmiddlewaretoken: csrf},
data: { 'tag-name': tagName, csrfmiddlewaretoken: csrf },
success: function solidifyTag(data) {
// Make an onscreen tag non-ghostly,
// canonicalize its name,
Expand All @@ -210,8 +209,8 @@ import _keys from "underscore/modules/keys";
var url = data.tagUrl,
tagNameSpan = $tag.find('.tag-name');
tagNameSpan.replaceWith($("<a class='tag-name' />")
.attr('href', url)
.text(tagNameSpan.text()));
.attr('href', url)
.text(tagNameSpan.text()));
$tag.removeClass('in-progress');
attachRemoverHandlerTo($tag, true);
},
Expand All @@ -231,7 +230,7 @@ import _keys from "underscore/modules/keys";
// Dim all Add buttons. We'll undim them upon valid input.
$('div.tags input.adder:enabled').attr('disabled', true);

$('.tag-adder').each(function() {
$('.tag-adder').each(function () {
var $this = $(this),
async = !$this.hasClass('deferred');
function handler() {
Expand All @@ -256,14 +255,14 @@ import _keys from "underscore/modules/keys";
// Ripped off from jquery.ui.autocomplete.js. Why can't I get at these
// via, e.g., $.ui.autocomplete.filter?

function escapeRegex( value ) {
return value.replace( /([\^\$\(\)\[\]\{\}\*\.\+\?\|\\])/gi, '\\$1' );
function escapeRegex(value) {
return value.replace(/([\^\$\(\)\[\]\{\}\*\.\+\?\|\\])/gi, '\\$1');
}

function filter(array, term) {
var matcher = new RegExp( escapeRegex(term), 'i' );
return $.grep( array, function(value) {
return matcher.test( value.label || value.value || value );
var matcher = new RegExp(escapeRegex(term), 'i');
return $.grep(array, function (value) {
return matcher.test(value.label || value.value || value);
});
}

Expand All @@ -286,7 +285,7 @@ import _keys from "underscore/modules/keys";
function getAppliedTags($tagList) {
var tagNames = [];
$tagList.find('.tag .tag-name').each(
function(i, e) {
function (i, e) {
tagNames.push($(e).text());
}
);
Expand Down
7 changes: 1 addition & 6 deletions kitsune/tags/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ class TagWidget(Widget):
def make_link(self, slug):
return "#"

# Allow adding new tags to the vocab:
can_create_tags = False

# TODO: Add async_remove_url and async_add_url kwargs holding URLs to
# direct async remove and add requests to. The client app is then
# responsible for routing to those and doing the calls to remove/add
Expand Down Expand Up @@ -88,8 +85,6 @@ def render(self, name, value, attrs=None, renderer=None):
if not self.read_only:
vocab = [t.name for t in Tag.objects.only("name").all()]
output += ' data-tag-vocab-json="%s"' % escape(json.dumps(vocab))
if self.can_create_tags:
output += ' data-can-create-tags="1"'
output += ">"

if not self.read_only:
Expand Down Expand Up @@ -155,7 +150,7 @@ class TagField(MultipleChoiceField):

def valid_value(self, value):
"""Check the validity of a single tag."""
return self.widget.can_create_tags or Tag.objects.filter(name=value).exists()
return Tag.objects.filter(name=value).exists()

def to_python(self, value):
"""Ignore the input field if it's blank; don't make a tag called ''."""
Expand Down