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

Subject tag changes #10138

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Dec 9, 2024

Supplants #9843

General Changes

  • Two general tag types defined:
    • Collection tags
      • Tag type: collection
    • Subject tags
      • Tag types: subject, person, place, time
  • Subject tags now have a body property
    • This will be rendered as the main content on a tag page

Tag model

  • Tag.find() updated to return multiple values

    • Now returns a list of matching tag keys
    • tag_type no longer a required parameter
  • Tag.create() updated to take a dict representation of a Tag

    • Tag dict validations are expected to occur before create is called
    • Tag.create() now returns the newly created Tag object (instead of a key)
      • Prevents redundant calls to Infogami

Tag page templates

General Tag view - /templates/type/tag/view.html

  • Updated means of deriving a related subject_key
  • Renders Tag.body to the page (if the tag is a subject tag)
  • Sanitizes and formats the description and body

Type-specific edit page and form controls - /templates/type/tag/{tag_type}

  • Two new directories:
    • /templates/type/tag/subject
    • /templates/type/tag/collection
  • Each directory has:
    • edit.html - Form used for editing an existing tag or creating a new tag
    • edit_form_inputs - Contains form inputs appropriate for the tag type

Generic "Add Tag" form

  • Now populated with subject and collection edit_form_inputs templates
  • When selected subject type changes, unrelated form elements are hidden

Subject page

  • Disambiguation section added
    • Included links to the Tag views of subject tags with a similar name
  • Tagging flow:
    • "Edit" button copy changed to "Tag"
    • "Tag" button is only present on subject page if no subject tag of the same name and type was found when the page was rendered
    • General flow:
      • Patron clicks "Tag" on a subject page
      • Patron fills out and submits the "Add Tag" form
      • Patron is redirected to the newly created Tag page
        • Link to associated subject page included on this page

addtag.py

  • unflatten calls on patron input have been removed
    • unflatten is used specifically for book edit form data
  • Added type-specific validation functions
  • Added handler for type-specific edits

Missing from this PR

  • Subject tag body is not automatically generated
    • Attempted to add the same components that are used by the subject page, but HTML becomes malformed after sanitize and format calls
  • Types in URLs (/tags/OL123T/{tag_type}/{tag_label})
    • Requires refactor of ReadableUrlProcessor

Technical

Testing

Screenshot

Stakeholders

Removes `work` tag type, and adds four distinct subject types.

Adds `body` input to the "add tag" form.

Fixes bug where incorrect subject page link is rendered on tag page.

Replace subject page "Edit" button with "Tag" button, when no associated tag is found.  If there is a tag, a link to the tag page is included beneath the tag description.

Update the Tag.create method to take a `tag` dictionary, and to return the newly created tag.
Creates handler for paths like `/tag/{tag_type}/add`, and new templates for adding and editing
specific tag types.

The generic "Add Tag" form template has been updated to show and hide the appropriate form
elements whenever the selected tag type is changed.  Additionally, existing tag form templates
have been updated to take a `page` object (instead of one parameter for each property a `Tag` may
have).

The subject page "Tag" link has been updated to point to `/tag/{tag_type}/add`, where `tag_type`
is one of our four subject types.  Patron's who follow the link will be served the form for
adding subject tags (instead of the generalized add tag form, available at `/tag/add`).
The subject page handler now makes a call to fetch related
tags after the subject has been returned from Solr.  Data
fetched from the `/subjects` `json` endpoint are no longer
decorated with tag data.
Adds new `validate_subject_tag` and `create_tag`
functions.  Updates existing `create_subject_tag`
function to take a dictionary.

Updates existing handlers to use the correct tag
validation and creation methods.
Form would fail to submit when `collection` type was chosen and all fields filled.
The issue was the required subject tag body, which is only required for subject tags.
Inputs specific to a tag sub-type are now removed if that tag type is not selected.
- Sanitize tag description in tag view
- Update subject key derivation in tag view
- Reduce number of DB calls required to decorate a subject page with tags
- Tag edit `GET` handler returns edit form specific to the existing tag's type
- Use correct sub-type validations on tag edit
- Remove `utils.unflatten` call when creating tag
  - `utils.unflatten` is made specifically for processing the book edit form
@jimchamp jimchamp force-pushed the subject-tag-changes branch from e52b1fe to 1c17c70 Compare December 9, 2024 18:44
@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.

Project coverage is 17.33%. Comparing base (347bff9) to head (0f30126).
Report is 136 commits behind head on master.

Files with missing lines Patch % Lines
openlibrary/plugins/openlibrary/js/tag-form.js 0.00% 22 Missing and 4 partials ⚠️
openlibrary/plugins/openlibrary/js/index.js 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10138      +/-   ##
==========================================
+ Coverage   17.12%   17.33%   +0.20%     
==========================================
  Files          89       90       +1     
  Lines        4752     4822      +70     
  Branches      831      855      +24     
==========================================
+ Hits          814      836      +22     
- Misses       3428     3461      +33     
- Partials      510      525      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jimchamp jimchamp force-pushed the subject-tag-changes branch from 01aaaf0 to 7f45814 Compare December 9, 2024 18:54
@jimchamp jimchamp force-pushed the subject-tag-changes branch 4 times, most recently from 53a99e9 to 7e861fd Compare December 9, 2024 19:58
@jimchamp jimchamp force-pushed the subject-tag-changes branch from fe8c8a4 to 12fca88 Compare December 9, 2024 21:34
@jimchamp jimchamp force-pushed the subject-tag-changes branch from c6dc94f to 0d81650 Compare December 9, 2024 21:55
@jimchamp jimchamp marked this pull request as draft December 13, 2024 23:43
@jimchamp jimchamp marked this pull request as ready for review December 13, 2024 23:43
@jimchamp jimchamp added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants