Consolidate tag edit/add forms

internetarchive · Dec 17, 2024 · 2b2e593 · 2b2e593
1 parent 1b8ad73
commit 2b2e593
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 195 deletions.
diff --git a/openlibrary/plugins/upstream/addtag.py b/openlibrary/plugins/upstream/addtag.py
@@ -6,7 +6,6 @@
 from infogami.infobase.client import ClientException
 from infogami.utils import delegate
 from infogami.utils.view import add_flash_message, public
-from web.template import TemplateResult
 
 from openlibrary.accounts import get_current_user
 from openlibrary.plugins.upstream import spamcheck
@@ -86,7 +85,7 @@ def GET(self):
 
         i = web.input(name=None, type=None, sub_type=None)
 
-        return render_template('tag/add', i)
+        return render_template('type/tag/form', i)
 
     def has_permission(self, user) -> bool:
         """
@@ -115,7 +114,7 @@ def POST(self):
         if not self.has_permission(patron):
             raise web.unauthorized(message='Permission denied to add tags')
 
-        if not validate_tag(i):
+        if not self.validate_input(i):
             raise web.badrequest()
 
         if match := find_match(i.name, i.tag_type):
@@ -124,11 +123,16 @@ def POST(self):
                 'error',
                 f'A matching tag with the same name and type already exists: <a href="{match}">{match}</a>'
             )
-            return render_template('tag/add', i)
+            return render_template('type/tag/form', i)
 
         tag = create_tag(i)
         raise safe_seeother(tag.key)
 
+    def validate_input(self, i):
+        if i.tag_type in get_subject_tag_types():
+            return validate_subject_tag(i)
+        return validate_tag(i)
+
 
 class tag_edit(delegate.page):
     path = r"(/tags/OL\d+T)/edit"
@@ -145,7 +149,7 @@ def GET(self, key):
         if tag is None:
             raise web.notfound()
 
-        return self.get_template_for_type(tag, i)
+        return render_template("type/tag/form", tag, redirect=i.redir)
 
     def POST(self, key):
         if not web.ctx.site.can_write(key):
@@ -160,7 +164,7 @@ def POST(self, key):
 
         i = web.input(_comment=None, redir=None)
         formdata = trim_doc(i)
-        if not formdata or not self.validate(formdata, tag.tag_type):
+        if not formdata or not self.validate(formdata, formdata.tag_type):
             raise web.badrequest()
         if tag.tag_type != formdata.tag_type:
             match = find_match(formdata.name, formdata.tag_type)
@@ -170,7 +174,7 @@ def POST(self, key):
                     'error',
                     f'A matching tag with the same name and type already exists: <a href="{match}">{match}</a>'
                 )
-                return self.get_template_for_type(formdata, formdata)
+                return render_template("type/tag/form", formdata, redirect=i.redir)
 
         try:
             if "_delete" in i:
@@ -184,80 +188,14 @@ def POST(self, key):
             raise safe_seeother(i.redir if i.redir else key)
         except (ClientException, ValidationException) as e:
             add_flash_message('error', str(e))
-            return render_template("type/tag/edit", tag, redirect=i.redir)
+            return render_template("type/tag/form", tag, redirect=i.redir)
 
     def validate(self, data, tag_type):
         if tag_type in SUBJECT_SUB_TYPES:
             return validate_subject_tag(data)
         else:
             return validate_tag(data)
 
-    def get_template_for_type(self, tag, web_input) -> TemplateResult:
-        if tag.tag_type in SUBJECT_SUB_TYPES:
-            return render_template("type/tag/subject/edit", tag, redirect=web_input.redir)
-        elif tag.tag_type == "collection":
-            return render_template("type/tag/collection/edit", tag)
-
-        return render_template('type/tag/edit', tag, redirect=web_input.redir)
-
-
-class add_typed_tag(delegate.page):
-    path = "/tag/([^/]+)/add"
-
-    def GET(self, tag_type):
-        if not tag_type in get_tag_types():
-            raise web.badrequest()
-        if not (patron := get_current_user()):
-            # NOTE : will lose any query params on login redirect
-            raise web.seeother(f'/account/login?redirect={self.path}')
-        if not self.has_permission(patron):
-            raise web.unauthorized()
-
-        i = web.input(tag_type=tag_type, redir=None)
-        if tag_type in get_subject_tag_types():
-            return render_template("type/tag/subject/edit", i, redirect=i.redir)
-        return render_template(f"type/tag/{tag_type}/edit", i)
-
-    def POST(self, tag_type):
-        i = web.input(tag_type=tag_type, redir=None)
-        if not self.validate_input(i):
-            raise web.badrequest()
-        if spamcheck.is_spam(i, allow_privileged_edits=True):
-            return render_template(
-                "message.html", "Oops", 'Something went wrong. Please try again later.'
-            )
-        if not (patron := get_current_user()):
-            raise web.seeother('/account/login')
-        if not self.has_permission(patron):
-            raise web.unauthorized()
-
-        i = trim_doc(i)
-        if match := find_match(i.name, i.tag_type):
-            # A tag with this name and type already exists
-            add_flash_message(
-                'error',
-                f'A matching tag with the same name and type already exists: <a href="{match}">{match}</a>'
-            )
-            if tag_type in get_subject_tag_types():
-                return render_template("type/tag/subject/edit", i, redirect=i.redir)
-            return render_template(f"type/tag/{tag_type}/edit", i)
-
-        if i.tag_type in get_subject_tag_types():
-            tag = create_subject_tag(i)
-        else:
-            tag = create_tag(i)
-        raise safe_seeother(i.redir if i.redir else tag.key)
-
-    def has_permission(self, user):
-        return user and (
-            user.is_librarian() or user.is_super_librarian() or user.is_admin()
-        )
-
-    def validate_input(self, i):
-        if i.tag_type in get_subject_tag_types():
-            return validate_subject_tag(i)
-        return validate_tag(i)
-
 
 class tag_search(delegate.page):
     path = "/tags/-/([^/]+):([^/]+)"

diff --git a/openlibrary/templates/subjects.html b/openlibrary/templates/subjects.html
@@ -10,7 +10,7 @@
 <div class="page-heading-search-box">
     $if can_add_tag:
       <div class="subjectTagEdit">
-        $ link_url = "%s/-/edit?redir=%s" % (page.tag.key, ctx.path) if has_tag else "/tag/%s/add?name=%s&redir=%s" % (page.subject_type, page.name, ctx.path)
+        $ link_url = "%s/-/edit?redir=%s" % (page.tag.key, ctx.path) if has_tag else "/tag/add?name=%s&redir=%s&tag_type=%s" % (page.name, ctx.path, page.subject_type)
         $ link_cta = _("Edit")
         $ elem_title = _("Edit tag for this subject") if has_tag else _("Create tag for this subject")
         $ link_track = "CTAClick|SubjectPageTagEdit"

diff --git a/openlibrary/templates/tag/add.html b/openlibrary/templates/tag/add.html
diff --git a/openlibrary/templates/type/tag/collection/edit.html b/openlibrary/templates/type/tag/collection/edit.html
diff --git a/openlibrary/templates/type/tag/edit.html b/openlibrary/templates/type/tag/edit.html
diff --git a/openlibrary/templates/type/tag/form.html b/openlibrary/templates/type/tag/form.html
@@ -0,0 +1,38 @@
+$def with (page, redirect=None)
+
+$putctx("robots", "noindex,nofollow")
+
+$ is_editing = page and page.get("key", "")
+$var title: $(_("Edit tag") if is_editing else _("Add a tag"))
+$ heading = _("Edit tag") if is_editing else _("Add a tag to Open Library")
+
+<div id="contentHead">
+    $if is_editing:
+        $:macros.databarEdit(page)
+
+    <h1>$heading</h1>
+    $if not is_editing:
+        <p class="instruct">$_("We require a minimum set of fields to create a new collection tag.")</p>
+</div>
+
+<div id="contentBody">
+    <form name="edit" method="post" class="olform">
+        $if redirect:
+            <input type="hidden" name="redir" value="$redirect">
+        $:render_template("type/tag/tag_form_inputs", page)
+        <div class="clearfix"></div>
+
+        <div class="formElement bottom">
+            $if is_editing:
+                $:macros.EditButtons(comment=page.comment_)
+            $else:
+                <div class="cclicense">
+                    $:_('By saving a change to this wiki, you agree that your contribution is given freely to the world under <a href="https://creativecommons.org/publicdomain/zero/1.0/" target="_blank" title="This link to Creative Commons will open in a new window">CC0</a>. Yippee!')
+                </div>
+                <div class="input">
+                    <button type="submit" title="$_('Add this tag now')" class="larger">$_('Add')</button>
+                    <a href="javascript:;" class="attn go-back-link">$_('Cancel')</a>
+                </div>
+        </div>
+    </form>
+</div>
diff --git a/openlibrary/templates/type/tag/subject/edit.html b/openlibrary/templates/type/tag/subject/edit.html
diff --git a/openlibrary/templates/type/tag/tag_form_inputs.html b/openlibrary/templates/type/tag/tag_form_inputs.html
@@ -1,4 +1,4 @@
-$def with (page, type_picker_options={"show_picker": True, "subjects_only": False})
+$def with (page)
 
 $ tag_name = page and page.get('name', '')
 <div class="formElement">
@@ -34,15 +34,13 @@
     </div>
 </fieldset>
 
-$ hidden = 'hidden' if not type_picker_options.get("show_picker") else ''
 $ tag_type = page and page.get('tag_type', '')
-<div class="formElement $hidden">
+<div class="formElement">
     <div class="label"><label for="tag_type">$_("Tag type")</label></div>
     <div class="input">
         <select name="tag_type" required>
             <option value="">$_("Select")</option>
-            $ subjects_only = type_picker_options.get("subjects_only")
-            $ tag_types = get_subject_tag_types() if subjects_only else get_tag_types()
+            $ tag_types = get_tag_types()
             $for t_type in tag_types:
                 $if t_type == tag_type:
                     <option value="$t_type" selected>$t_type</option>