Skip to content

Add subject merge request flow tool#12839

Draft
KrishnaShuk wants to merge 2 commits into
internetarchive:masterfrom
KrishnaShuk:feat/subject-merge
Draft

Add subject merge request flow tool#12839
KrishnaShuk wants to merge 2 commits into
internetarchive:masterfrom
KrishnaShuk:feat/subject-merge

Conversation

@KrishnaShuk
Copy link
Copy Markdown
Contributor

@KrishnaShuk KrishnaShuk commented Jun 2, 2026

ref: #65

Summary

Adds a subject merge request flow that follows the existing community edit request workflow.

What changed

  • Adds subject selection from subject search results
  • Adds a dedicated /subjects/merge page
  • Stores subject merge requests in community_edits_queue as SUBJECT_MERGE
  • Adds subject filtering in the community merge request table
  • Adds UI tests for subject selection and merge action behavior

Current behavior

This branch creates and tracks subject merge requests, but it does not yet execute the actual
subject record merge.

Pending work

  • Implement the actual subject merge execution path.
  • Decide how subject redirects or canonical subject resolution should work after merge.
  • Add approval flow behavior for subject requests if it should differ from work and author merges.
  • Add broader test coverage for the full request lifecycle.

Video

Screencast.from.2026-06-03.00-00-47.mp4

Notes

This is meant to establish the direction of the tool and the queue integration first, before
wiring in the final merge execution.

@KrishnaShuk KrishnaShuk marked this pull request as draft June 2, 2026 18:18
@KrishnaShuk KrishnaShuk changed the title Add subject merge request flow Add subject merge request flow tool Jun 2, 2026
@KrishnaShuk KrishnaShuk force-pushed the feat/subject-merge branch from 5619046 to 43abd79 Compare June 2, 2026 19:08
Copy link
Copy Markdown

@accesslint accesslint Bot left a comment

Choose a reason for hiding this comment

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

Found 4 issues across 1 rule.

Comment thread openlibrary/templates/merge_request_table/table_header.html
@KrishnaShuk KrishnaShuk force-pushed the feat/subject-merge branch from 63251a8 to 43abd79 Compare June 2, 2026 19:21
Copy link
Copy Markdown

@accesslint accesslint Bot left a comment

Choose a reason for hiding this comment

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

Found 4 issues across 1 rule.

Comment thread openlibrary/templates/merge_request_table/table_header.html
@KrishnaShuk
Copy link
Copy Markdown
Contributor Author

How it currently works:

  1. Selecting Subjects: When a user searches for subjects, they can now highlight and select multiple items directly from the search results.
  2. The Toolbar: Once multiple subjects are selected, the librarian toolbar will pop up at the bottom of the screen with a new "Merge Subjects..." button.
  3. The Merge Form: Clicking that button takes the user to a dedicated merge page. Here, they can review the selected subjects, choose which one should be the primary (canonical) subject, and leave an optional comment.
  4. Queue Integration: Submitting the form writes a new "Pending" request into our community edits system safely identifying it as a subject merge.
  5. Dashboard Updates: Reviewers can now see these pending requests on the main /merges dashboard. We've also added a new "Type" dropdown to the dashboard so maintainers can cleanly filter the list between Work, Author, and Subject requests.

Technical Details:

Frontend: search/subjects.html was updated to surface data-subject-id. SelectionManager.js was given a new regex hook (^/search/subjects$) that recognizes this type and builds the /subjects/merge?records=... URL.

Form Route: The new merge_subjects class in subjects.py handles the page rendering safely by normalizing the prefixes (e.g. person:tolkien) and fetching the latest display data.

Database Insertion: The POST request from the form passes the IDs through process_merge_request, inserting a record into community_edits_queue mapped to our new SUBJECT_MERGE type constant.

Dashboard Filtering: edits.py computes the query parameters for filtering Works/Authors/Subjects in the backend, resolving web.py's strict template limits by passing the toggles down cleanly via the context dictionary into table_header.html.

What is NOT included yet:
This branch does not currently execute the actual database changes yet.

@KrishnaShuk KrishnaShuk force-pushed the feat/subject-merge branch from 7c12f59 to f58b2a7 Compare June 2, 2026 19:41
Copy link
Copy Markdown

@accesslint accesslint Bot left a comment

Choose a reason for hiding this comment

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

Found 4 issues across 1 rule.

<div class="flex-auto"></div>
<div class="mr-dropdown" id="type-menu-button">$_('Type ▾')
<div class="mr-dropdown-menu sort hidden">
<header class="dropdown-header">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Best Practice: Page has multiple banner landmarks.

Page should not have more than one banner landmark.

Details

The banner landmark (typically <header>) identifies site-oriented content like logos and search. Only one top-level banner is allowed per page. If you need multiple headers, nest them inside sectioning elements (article, section, aside) where they become scoped headers rather than page-level banners.

@mekarpeles
Copy link
Copy Markdown
Member

@jimchamp suggests:

It may be helpful to know which works are affected (instead of a count).  It's probably not feasible to print all titles, but maybe a small number would help?

It's hard to tell the type of each MR at a glance.  It may seem to a reviewer that they are reviewing a work merge, only to be taken to the merge subject view.

@KrishnaShuk
Copy link
Copy Markdown
Contributor Author

@mekarpeles

  1. It is good idea to show some work titles on the merging page. In your opinion how we could showcase the titles without cluttering up the UI?

  2. A badge/tag could be added to each MR which would visually help to distinguish between the Work, Author, and Subject requests.

@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Response Issues which require feedback from lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants