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

Save subject page body to Tag.body when tags are created #9843

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
10 changes: 6 additions & 4 deletions openlibrary/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,8 @@ def create(
tag_name,
tag_description,
tag_type,
tag_plugins,
body='',
fkey=None,
ip='127.0.0.1',
comment='New Tag',
):
Expand All @@ -1193,18 +1194,19 @@ def create(

with RunAs(patron):
web.ctx.ip = web.ctx.ip or ip
web.ctx.site.save(
t = 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'},
'fkey': fkey,
'body': body.strip(),
},
comment=comment,
)
return key
return web.ctx.site.get(key)


@dataclass
Expand Down
19 changes: 15 additions & 4 deletions openlibrary/i18n/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -690,10 +690,18 @@ msgstr ""
msgid "Name"
msgstr ""

#: subjects.html
msgid "Promote to Tag"
msgstr ""

#: subjects.html
msgid "Edit Subject Tag"
msgstr ""

#: subjects.html
msgid "Create Tag from this subject"
msgstr ""

#: subjects.html
msgid "See all works"
msgstr ""
Expand Down Expand Up @@ -7007,12 +7015,15 @@ msgid "Tag Description"
msgstr ""

#: type/tag/tag_form_inputs.html
msgid "Tag type"
msgid "Page Body"
msgstr ""

#: type/tag/view.html
#, python-format
msgid "View subject page for %(name)s."
#: type/tag/tag_form_inputs.html
msgid "Subject Key"
msgstr ""

#: type/tag/tag_form_inputs.html
msgid "Tag type"
msgstr ""

#: type/template/edit.html
Expand Down
78 changes: 62 additions & 16 deletions openlibrary/plugins/upstream/addtag.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,66 @@
from openlibrary.plugins.upstream.models import Tag
from openlibrary.plugins.upstream.addbook import safe_seeother, trim_doc
from openlibrary.plugins.upstream.utils import render_template
from openlibrary.plugins.worksearch.subjects import get_subject


@public
def get_tag_types():
return ["subject", "work", "collection"]


def validate_tag(tag):
return tag.get('name', '') and tag.get('tag_type', '')
def validate_tag(tag, for_promotion=False):
return (
(tag.get('name', '') and tag.get('fkey', None))
if for_promotion
else (tag.get('name', '') and tag.get('tag_type', ''))
)


def has_permission(user) -> bool:
"""
Can a tag be added?
"""
return user and (
user.is_librarian() or user.is_super_librarian() or user.is_admin()
)


def create_subject_tag(
name: str, description: str, fkey: str | None = None, body: str = ''
) -> Tag:
tag = Tag.create(name, description, 'subject', fkey=fkey, body=body)
if fkey and not body:
subject = get_subject(
fkey,
details=True,
filters={'public_scan_b': 'false', 'lending_edition_s': '*'},
sort='readinglog',
)
if subject and subject.work_count > 0:
tag.body = str(render_template('subjects', page=subject))
tag._save()
return tag


class promotetag(delegate.page):
path = "/tag/promote"

def GET(self):
if not (patron := get_current_user()):
raise web.seeother(f'/account/login?redirect={self.path}')
if not has_permission(patron):
raise web.unauthorized(
message='Permission denied to promote subject to tags'
)

i = web.input(name="", fkey=None, description="")

if not validate_tag(i, for_promotion=True):
raise web.badrequest()

tag = create_subject_tag(i.name, i.description, fkey=i.fkey)
raise safe_seeother(tag.key)


class addtag(delegate.page):
Expand All @@ -33,27 +84,22 @@ def GET(self):
if not (patron := get_current_user()):
raise web.seeother(f'/account/login?redirect={self.path}')

if not self.has_permission(patron):
if not has_permission(patron):
raise web.unauthorized(message='Permission denied to add tags')

i = web.input(name=None, type=None, sub_type=None)
i = web.input(name=None, type=None, sub_type=None, fkey=None)

return render_template('tag/add', i.name, i.type, subject_type=i.sub_type)

def has_permission(self, user) -> bool:
"""
Can a tag be added?
"""
return user and (
user.is_librarian() or user.is_super_librarian() or user.is_admin()
return render_template(
'tag/add', i.name, i.type, subject_type=i.sub_type, fkey=i.fkey
)

def POST(self):
i = web.input(
name="",
tag_type="",
tag_description="",
tag_plugins="",
body="",
fkey=None,
)

if spamcheck.is_spam(i, allow_privileged_edits=True):
Expand All @@ -64,7 +110,7 @@ def POST(self):
if not (patron := get_current_user()):
raise web.seeother(f'/account/login?redirect={self.path}')

if not self.has_permission(patron):
if not has_permission(patron):
raise web.unauthorized(message='Permission denied to add tags')

i = utils.unflatten(i)
Expand Down Expand Up @@ -101,8 +147,8 @@ def no_match(self, i: web.utils.Storage) -> NoReturn:
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)
tag = create_subject_tag(i.name, i.tag_description, fkey=i.fkey, body=i.body)
raise safe_seeother(tag.key)


class tag_edit(delegate.page):
Expand Down
15 changes: 7 additions & 8 deletions openlibrary/plugins/worksearch/subjects.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ def GET(self, key):
if (nkey := self.normalize_key(key)) != key:
raise web.redirect(nkey)

q = {"type": "/type/tag", "fkey": key, "tag_type": "subject"}
if match := web.ctx.site.things(q):
tag = web.ctx.site.get(match[0])
raise web.redirect(tag.key)

# this needs to be updated to include:
# q=public_scan_b:true+OR+lending_edition_s:*
subj = get_subject(
Expand Down Expand Up @@ -337,17 +342,11 @@ def get_subject(
subject[meta.key].pop(i)
break

q = {"type": "/type/tag", "name": subject.name, "tag_type": "subject"}
q = {"type": "/type/tag", "fkey": subject.key, "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
subject.tag = tag

return subject

Expand Down
23 changes: 7 additions & 16 deletions openlibrary/templates/subjects.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,21 @@
$ q = query_param('q')

<div id="scrollHere"></div>

<!--
Able to fetch Tag with the same name as the subject and use it's data to enrich the page via page.tag
-->

<div class="page-heading-search-box">
$if can_add_tag:
<div class="subjectTagEdit">
$ edit_tag_url = page.tag.get('id') + '/-/edit' if 'tag' in page else '/tag/add'
$if not has_tag:
$ edit_tag_url += '?name=%s&type=subject&sub_type=%s' % (page.name, page.subject_type)
$ edit_tag_url = page.tag.get('key') + '/-/edit' if has_tag else '/tag/promote?name=%s&type=subject&sub_type=%s&fkey=%s' % (page.name, page.subject_type, page.key)
$ btn_cta = _("Edit") if has_tag else _("Promote to Tag")
$ elem_title = _("Edit Subject Tag") if has_tag else _("Create Tag from this subject")
$ link_track = "CTAClick|SubjectTagEdit" if has_tag else "CTAClick|SubjectTagAdd"
<a
class="editTagButton"
href="$edit_tag_url"
title="$_('Edit Subject Tag')"
data-ol-link-track="CTAClick|Edit"
title="$elem_title"
data-ol-link-track="$link_track"
accesskey="e"
rel="nofollow"
>$_("Edit")</a>
>$btn_cta</a>
</div>
<h1 class="inline">
$page.name
Expand All @@ -35,12 +31,7 @@ <h1 class="inline">
<strong><span><a href="/search?$page.subject_type=$page.name.replace('&','%26')" title="$_('See all works')">$ungettext("%(count)d work", "%(count)d works", page.work_count, count=page.work_count)</a></span></strong>
</span>
</span>
$if has_tag:
<h3 itemprop="description">
$page.tag['description']
</h3>
Comment on lines -38 to -41
Copy link
Collaborator Author

@jimchamp jimchamp Sep 6, 2024

Choose a reason for hiding this comment

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

I think that we do want this somewhere on the page, just not in a heading element (and maybe somewhere else on the page).

Maybe displaying this before tag.body in /type/tag/view.html is the right spot? This would ensure that:

  1. The description is always present in the tag's view
  2. The description only appears on the page once when tag.body is a rendered subject page

<a href="#search" class="shift">$_("Search for books with subject %(name)s.", name=page.name)</a>

<form action="/search" class="olform pagesearchbox">
$:render_template("search/searchbox", q=q)
<input type="hidden" name="${page.subject_type}_facet" value="$page.name"/>
Expand Down
4 changes: 2 additions & 2 deletions openlibrary/templates/tag/add.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
$def with (name, type, subject_type=None)
$def with (name, type, subject_type=None, fkey=None)

$var title: $_("Add a tag")

Expand All @@ -11,7 +11,7 @@ <h1>$_("Add a tag to Open Library")</h1>

<div id="contentBody">
<form method="post" action="" id="addtag" class="olform addtag1">
$:render_template("type/tag/tag_form_inputs", tag_name=name, tag_type=type)
$:render_template("type/tag/tag_form_inputs", tag_name=name, tag_type=type, fkey=fkey)
<div class="formElement bottom">
<br/>
<div class="cclicense">
Expand Down
2 changes: 1 addition & 1 deletion openlibrary/templates/type/tag/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ <h1>$_("Edit Tag")</h1>
<div id="contentBody">

<form id="edittag" name="edit" method="post" action="" class="olform">
$:render_template("type/tag/tag_form_inputs", tag_name=page.name, tag_type=page.tag_type, tag_description=page.tag_description)
$:render_template("type/tag/tag_form_inputs", tag_name=page.name, tag_type=page.tag_type, tag_description=page.tag_description, fkey=page.fkey, body=page.body, show_type_picker=False)
<div class="clearfix"></div>

<div class="formElement bottom">
Expand Down
45 changes: 33 additions & 12 deletions openlibrary/templates/type/tag/tag_form_inputs.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
$def with (tag_name='', tag_type='', tag_description='')
$def with (tag_name='', tag_type='', tag_description='', fkey=None, body='', show_type_picker=True)

<div class="formElement">
<div class="label">
Expand All @@ -15,22 +15,43 @@
<label for="tag_description">$_("Tag Description")</label>
</div>
<div class="input">
<textarea id="tag_description" name="tag_description" class="markdown" rows="5" cols="80">$tag_description</textarea>
<textarea id="tag_description" name="tag_description" rows="5" cols="80">$tag_description</textarea>
</div>
</div>
</fieldset>

<fieldset class="major">
<div class="formElement">
<div class="label">
<label for="page_body">$_("Page Body")</label>
</div>
<div class="input">
<textarea id="page_body" name="page_body" rows="5" cols="50" class="markdown">$body</textarea>
</div>
</div>
</fieldset>

<div class="formElement">
<div class="label"><label for="tag_type">$_("Tag type")</label></div>
<div class="label">
<label for="fkey">$_("Subject Key")</label>
</div>
<div class="input">
<select name="tag_type" id="tag_type" required>
<option value="">$_("Select")</option>
$ tag_types = get_tag_types()
$for t_type in tag_types:
$if t_type == tag_type:
<option value="$t_type" selected>$t_type</option>
$else:
<option value="$t_type">$t_type</option>
</select>
<input type="text" value="$fkey" disabled>
</div>
</div>

$if show_type_picker:
<div class="formElement">
<div class="label"><label for="tag_type">$_("Tag type")</label></div>
<div class="input">
<select name="tag_type" id="tag_type" required>
<option value="">$_("Select")</option>
$ tag_types = get_tag_types()
$for t_type in tag_types:
$if t_type == tag_type:
<option value="$t_type" selected>$t_type</option>
$else:
<option value="$t_type">$t_type</option>
</select>
</div>
</div>
28 changes: 12 additions & 16 deletions openlibrary/templates/type/tag/view.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,15 @@
$var title: $tag.name
$ title = tag.name

<div id="contentHead">
$:macros.databarView(tag)
<h1 itemprop="name">
$title
</h1>
<h3 itemprop="description">
$tag.tag_description
</h3>
$if tag.tag_type == 'subject':
$ subject_name = title.replace(' ', "_")
<footer>
<a href="/subjects/$subject_name">
$_('View subject page for %(name)s.', name=title)
</a>
</footer>
</div>
$if tag.body:
$:tag.body
$else:
<div id="contentHead">
$:macros.databarView(tag)
<h1 itemprop="name">
$title
</h1>
<h3 itemprop="description">
$tag.tag_description
</h3>
</div>
Loading