Subject tag changes

openlibrary/core/models.py

@@ -1,7 +1,6 @@
 """Models of various OL objects.
 """
 
-import json
 import logging
 from collections import defaultdict
 from dataclasses import dataclass, field
@@ -1145,42 +1144,36 @@ def get_url_suffix(self):
         return self.name or "unnamed"
 
     @classmethod
-    def find(cls, tag_name, tag_type):
-        """Returns a Tag key for a given tag name and tag type."""
-        q = {'type': '/type/tag', 'name': tag_name, 'tag_type': tag_type}
-        match = list(web.ctx.site.things(q))
-        return match[0] if match else None
+    def find(cls, tag_name, tag_type=None):
+        """Returns a Tag key for a given tag name."""
+        q = {'type': '/type/tag', 'name': tag_name}
+        if tag_type:
+            q['tag_type'] = tag_type
+        matches = list(web.ctx.site.things(q))
+        return matches
 
     @classmethod
     def create(
         cls,
-        tag_name,
-        tag_description,
-        tag_type,
-        tag_plugins,
+        tag,
         ip='127.0.0.1',
         comment='New Tag',
     ):
         """Creates a new Tag object."""
         current_user = web.ctx.site.get_user()
         patron = current_user.get_username() if current_user else 'ImportBot'
         key = web.ctx.site.new_key('/type/tag')
+        tag['key'] = key
+
         from openlibrary.accounts import RunAs
 
         with RunAs(patron):
             web.ctx.ip = web.ctx.ip or ip
-            web.ctx.site.save(
-                {
-                    'key': key,
-                    'name': tag_name,
-                    'tag_description': tag_description,
-                    'tag_type': tag_type,
-                    'tag_plugins': json.loads(tag_plugins or "[]"),
-                    'type': {"key": '/type/tag'},
-                },
+            t = web.ctx.site.save(
+                tag,
                 comment=comment,
             )
-            return key
+            return t
 
 
 @dataclass

openlibrary/plugins/openlibrary/js/index.js

@@ -560,4 +560,3 @@ jQuery(function () {
             .then(module => module.initGoBackLinks(backLinks))
     }
 });
-

openlibrary/plugins/upstream/addtag.py

@@ -1,27 +1,73 @@
 """Handlers for adding and editing tags."""
 
-from typing import NoReturn
-
 import web
 
 from infogami.core.db import ValidationException
 from infogami.infobase.client import ClientException
 from infogami.utils import delegate
 from infogami.utils.view import add_flash_message, public
 from openlibrary.accounts import get_current_user
-from openlibrary.plugins.upstream import spamcheck, utils
+from openlibrary.plugins.upstream import spamcheck
 from openlibrary.plugins.upstream.addbook import safe_seeother, trim_doc
 from openlibrary.plugins.upstream.models import Tag
 from openlibrary.plugins.upstream.utils import render_template
 
 
 @public
 def get_tag_types():
-    return ["subject", "work", "collection"]
+    return TAG_TYPES
+
+
+@public
+def get_subject_tag_types():
+    return SUBJECT_SUB_TYPES
+
+
+SUBJECT_SUB_TYPES = ["subject", "person", "place", "time"]
+TAG_TYPES = SUBJECT_SUB_TYPES + ["collection"]
 
 
 def validate_tag(tag):
-    return tag.get('name', '') and tag.get('tag_type', '')
+    return tag.get('name', '') and tag.get('tag_description', '') and tag.get('tag_type', '') in get_tag_types() and tag.get('body')
+
+
+def validate_subject_tag(tag):
+    return validate_tag(tag) and tag.get('tag_type', '') in get_subject_tag_types()
+
+
+def create_tag(tag: dict):
+    if not validate_tag(tag):
+        raise ValueError("Invalid data for tag creation")
+
+    d = {
+        "type": {"key": "/type/tag"},
+        **tag,
+    }
+
+    tag = Tag.create(trim_doc(d))
+    return tag
+
+
+def create_subject_tag(tag: dict):
+    if not validate_subject_tag(tag):
+        raise ValueError("Invalid data for subject tag creation")
+
+    d = {
+        "type": {"key": "/type/tag"},
+        **tag,
+    }
+
+    tag = Tag.create(trim_doc(d))
+    return tag
+
+
+def find_match(name: str, tag_type: str) -> str:
+    """
+    Tries to find an existing tag that matches the data provided by the user.
+    Returns the key of the matching tag, or an empty string if no such tag exists.
+    """
+    matches =  Tag.find(name, tag_type=tag_type)
+    return matches[0] if matches else ''
 
 
 class addtag(delegate.page):
@@ -30,14 +76,15 @@ class addtag(delegate.page):
     def GET(self):
         """Main user interface for adding a tag to Open Library."""
         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(message='Permission denied to add tags')
 
         i = web.input(name=None, type=None, sub_type=None)
 
-        return render_template('tag/add', i.name, i.type, subject_type=i.sub_type)
+        return render_template('type/tag/form', i)
 
     def has_permission(self, user) -> bool:
         """
@@ -52,7 +99,7 @@ def POST(self):
             name="",
             tag_type="",
             tag_description="",
-            tag_plugins="",
+            body="",
         )
 
         if spamcheck.is_spam(i, allow_privileged_edits=True):
@@ -66,42 +113,24 @@ def POST(self):
         if not self.has_permission(patron):
             raise web.unauthorized(message='Permission denied to add tags')
 
-        i = utils.unflatten(i)
-
-        if not validate_tag(i):
+        if not self.validate_input(i):
             raise web.badrequest()
 
-        match = self.find_match(i)  # returns None or Tag (if match found)
-
-        if match:
-            # tag match
-            return self.tag_match(match)
-        else:
-            # no match
-            return self.no_match(i)
-
-    def find_match(self, i: web.utils.Storage):
-        """
-        Tries to find an existing tag that matches the data provided by the user.
-        """
-        return Tag.find(i.name, i.tag_type)
+        if match := find_match(i.name, i.tag_type):
+            # A tag with the same 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>'
+            )
+            return render_template('type/tag/form', i)
 
-    def tag_match(self, match: list) -> NoReturn:
-        """
-        Action for when an existing tag has been found.
-        Redirect user to the found tag's edit page to add any missing details.
-        """
-        tag = web.ctx.site.get(match[0])
-        raise safe_seeother(tag.key + "/edit")
+        tag = create_tag(i)
+        raise safe_seeother(tag.key)
 
-    def no_match(self, i: web.utils.Storage) -> NoReturn:
-        """
-        Action to take when no tags are found.
-        Creates a new Tag.
-        Redirects the user to the tag's home page
-        """
-        key = Tag.create(i.name, i.tag_description, i.tag_type, i.tag_plugins)
-        raise safe_seeother(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):
@@ -114,41 +143,69 @@ def GET(self, key):
                 web.ctx.fullpath,
                 "Permission denied to edit " + key + ".",
             )
-
+        i = web.input(redir=None)
         tag = web.ctx.site.get(key)
         if tag is None:
             raise web.notfound()
 
-        return render_template('type/tag/edit', tag)
+        return render_template("type/tag/form", tag, redirect=i.redir)
 
     def POST(self, key):
+        if not web.ctx.site.can_write(key):
+            return render_template(
+                "permission_denied",
+                web.ctx.fullpath,
+                "Permission denied to edit " + key + ".",
+            )
         tag = web.ctx.site.get(key)
         if tag is None:
             raise web.notfound()
 
-        i = web.input(_comment=None)
-        formdata = self.process_input(i)
+        i = web.input(_comment=None, redir=None)
+        formdata = trim_doc(i)
+        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)
+            if match:
+                # A tag with the same 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>'
+                )
+                return render_template("type/tag/form", formdata, redirect=i.redir)
+
         try:
-            if not formdata or not validate_tag(formdata):
-                raise web.badrequest()
-            elif "_delete" in i:
+            if "_delete" in i:
                 tag = web.ctx.site.new(
                     key, {"key": key, "type": {"key": "/type/delete"}}
                 )
                 tag._save(comment=i._comment)
                 raise safe_seeother(key)
-            else:
-                tag.update(formdata)
-                tag._save(comment=i._comment)
-                raise safe_seeother(key)
+            tag.update(formdata)
+            tag._save(comment=i._comment)
+            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)
+            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)
+
+
+class tag_search(delegate.page):
+    path = "/tags/-/([^/]+):([^/]+)"
 
-    def process_input(self, i):
-        i = utils.unflatten(i)
-        tag = trim_doc(i)
-        return tag
+    def GET(self, type, name):
+        # TODO : Search is case sensitive
+        # TODO : Handle spaces and special characters
+        matches = Tag.find(name, tag_type=type)
+        if matches:
+            return web.seeother(matches[0])
+        return render_template("notfound", f"/tags/-/{type}:{name}", create=False)
 
 
 def setup():

openlibrary/plugins/worksearch/subjects.py

@@ -12,7 +12,7 @@
 from infogami.utils import delegate
 from infogami.utils.view import render_template, safeint
 from openlibrary.core.lending import add_availability
-from openlibrary.core.models import Subject
+from openlibrary.core.models import Subject, Tag
 from openlibrary.solr.query_utils import query_dict_to_str
 from openlibrary.utils import str_to_key
 
@@ -44,6 +44,7 @@ def GET(self, key):
             web.ctx.status = "404 Not Found"
             page = render_template('subjects/notfound.tmpl', key)
         else:
+            self.decorate_with_tags(subj)
             page = render_template("subjects", page=subj)
 
         return page
@@ -58,6 +59,20 @@ def normalize_key(self, key):
             key = key.replace("/times/", "/time:")
         return key
 
+    def decorate_with_tags(self, subject) -> None:
+        tag_keys = Tag.find(subject.name)
+        if tag_keys:
+            tags = web.ctx.site.get_many(tag_keys)
+            subject.disambiguations = tags
+
+            if filtered_tags := [tag for tag in tags if tag.tag_type == subject.subject_type]:
+                subject.tag = filtered_tags[0]
+                # Remove matching subject tag from disambiguated tags:
+                subject.disambiguations = list(set(tags) - {subject.tag})
+
+            for tag in subject.disambiguations:
+                tag.subject_key = f"/subjects/{tag.name}" if tag.tag_type == "subject" else f"/subjects/{tag.tag_type}:{tag.name}"
+
 
 class subjects_json(delegate.page):
     path = '(/subjects/[^/]+)'
@@ -336,18 +351,6 @@ def get_subject(
                     subject[meta.key].pop(i)
                     break
 
-            q = {"type": "/type/tag", "name": subject.name, "tag_type": "subject"}
-            match = web.ctx.site.things(q)
-            if match:
-                tag = web.ctx.site.get(match[0])
-                match = {
-                    'name': tag.name,
-                    'id': tag.key,
-                    'description': tag.tag_description,
-                    'plugins': tag.tag_plugins,
-                }
-                subject.tag = match
-
         return subject
 
     def get_meta(self, key) -> 'SubjectMeta':

openlibrary/templates/site/body.html

@@ -27,7 +27,7 @@
       $#print errors (hidden by default as styles are loaded via JS)
       <div class="flash-messages">
       $for flash in get_flash_messages():
-        <div class="$flash.type"><span>$flash.message</span></div>
+        <div class="$flash.type"><span>$:flash.message</span></div>
       </div>
       $# Announcement banner will only be rendered if announcement and storage_key variables are set.
       $# Be sure to escape any single quotes inside of the announcement HTML string.