Skip to content

Commit

Permalink
Do not allow the creation of new tags
Browse files Browse the repository at this point in the history
  • Loading branch information
akatsoulas committed Nov 4, 2024
1 parent 5d6c2e3 commit 3d9e2cd
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 81 deletions.
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

0 comments on commit 3d9e2cd

Please sign in to comment.