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

Require Tag type on creation, update #9783

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Require Tag type on creation, update #9783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0194dbf
Update edit view's tag type options
jimchamp Aug 20, 2024
0a9c033
Add frontend validation for tag type
jimchamp Aug 20, 2024
ad2cf9c
Add backend tag property validation
jimchamp Aug 20, 2024
2346f81
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 20, 2024
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
2 changes: 1 addition & 1 deletion openlibrary/i18n/messages.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3395,7 +3395,7 @@ msgstr ""
msgid "ISBN may be invalid. Click \"Add\" again to submit."
msgstr ""

#: books/add.html tag/add.html
#: books/add.html tag/add.html type/tag/edit.html
msgid "Select"
msgstr ""

Expand Down
10 changes: 9 additions & 1 deletion openlibrary/plugins/upstream/addtag.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ def get_tag_types():
return ["subject", "work", "collection"]


def validate_tag(tag):
return tag.get('name', '') and tag.get('tag_type', '')


class addtag(delegate.page):
path = '/tag/add'

Expand Down Expand Up @@ -62,6 +66,10 @@ def POST(self):
raise web.unauthorized(message='Permission denied to add tags')

i = utils.unflatten(i)

if not i.tag_name or not i.tag_type:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will DRY this to use validate_tag when work begins on #9784. Today, the "Tag Name" input is identified differently in both forms (name in one; tag_name in the other).

raise web.badrequest()

match = self.find_match(i) # returns None or Tag (if match found)

if match:
Expand Down Expand Up @@ -120,7 +128,7 @@ def POST(self, key):
i = web.input(_comment=None)
formdata = self.process_input(i)
try:
if not formdata:
if not formdata or not validate_tag(formdata):
raise web.badrequest()
elif "_delete" in i:
tag = web.ctx.site.new(
Expand Down
2 changes: 1 addition & 1 deletion openlibrary/templates/tag/add.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ <h1>$_("Add a tag to Open Library")</h1>
<div class="formElement">
<div class="label"><label for="id_value">$_("Tag type")</label></div>
<div class="input">
<select name="tag_type" id="tag_type">
<select name="tag_type" id="tag_type" required>
<option value="">$_("Select")</option>
$ tag_types = get_tag_types()
$for tag_type in tag_types:
Expand Down
8 changes: 5 additions & 3 deletions openlibrary/templates/type/tag/edit.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ <h1>$_("Edit Tag")</h1>
<div class="formElement">
<div class="label"><label for="id_value">$_("Tag type")</label></div>
<div class="input">
<select name="tag_type" id="tag_type">
<option value="$page.tag_type">$page.tag_type</option>
<select name="tag_type" id="tag_type" required>
<option value="">$_("Select")</option>
$ tag_types = get_tag_types()
$for tag_type in tag_types:
$if tag_type != page.tag_type:
$if tag_type == page.tag_type:
<option value="$tag_type" selected>$tag_type</option>
$else:
Comment on lines +38 to +44
Copy link
Collaborator Author

@jimchamp jimchamp Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

These changes make this tag type selector roughly the same as the selector in the "add tag" view, and were largely made to support #9784.

<option value="$tag_type">$tag_type</option>
</select>
</div>
Expand Down
Loading