feat(modtools): Backend foundation for bulk version editing (Part 1/3)#2984
feat(modtools): Backend foundation for bulk version editing (Part 1/3)#2984dcschreiber wants to merge 58 commits intomasterfrom
Conversation
Update deprecated MongoDB operations: - sefaria/model/history.py: update() → update_many() - sefaria/helper/text.py: update() → update_many() Both maintain identical functionality with updated PyMongo 4.x API. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add audit trail for version metadata edits (license, status, priority, etc.) which were previously untracked. Changes now appear on /activity page with a new "Version Metadata" filter option. - Add log_version_metadata() in sefaria/model/history.py - Add update_version_metadata() in sefaria/tracker.py for centralized updates - Update version_bulk_edit_api, flag_text_api, lock_text_api to use tracker - Add activity feed template block and filter dropdown option - Enhanced error handling in index_post for better error messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add 3 new API endpoints for moderator tools: - /api/version-indices: Get indices that have versions matching a versionTitle - /api/version-bulk-edit: Bulk update Version metadata for multiple indices - /api/check-index-dependencies: Check dependencies before index changes (for future use) Features: - Field whitelisting prevents arbitrary attribute injection - Partial success handling with detailed success/failure arrays - Activity logging via tracker.update_version_metadata() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- sefaria/export.py: Fix missing user_id parameter in import_versions_from_file() - sefaria/helper/link.py: Add bytes decoding for CSV uploads (handles both bytes and str) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
These changes were present in the original PR but are not modtools-specific: - .gitignore: Add CLAUDE.md to ignore list - helm-chart: PostgreSQL backup script updates - linker.v3: Minor fix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
stevekaplan123
left a comment
There was a problem hiding this comment.
Does this PR do anything on its own? If someone were to use modtools or existing functionality if this were merged into master, would everything work the same? Otherwise, it's a problem because it will mean we have introduced a new bug(s). What we might want to do is to create a fourth branch that each of these merges into and then at the end we merge it all into master.
The enhanced error handling was for disabled NodeTitleEditor/BulkIndexEditor. Reverting to the original 3-line function until those tools are re-enabled.
|
@stevekaplan123 no other than the improvements we did there, there should be no change to master. So no degradation, but I still made these mergers a chain so we can:
|
Address PR feedback by eliminating separate log_version_metadata function. Instead, log_update now accepts optional history_noun kwarg to override the default klass.history_noun. The version_metadata case is handled in _log_general alongside index, link, and note patterns. Changes: - log_update accepts history_noun kwarg override - _log_general handles version_metadata case for queryable fields - Removed log_version_metadata function - update_version_metadata now calls log_update with history_noun param - Updated docstring to explain why this function exists Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…, StatusMessage) Add reusable UI components for the modtools panel: - ModToolsSection: Collapsible section wrapper with consistent styling - HelpButton: Question mark icon that opens modal with documentation - StatusMessage: Type-based message display (success, error, warning, info) These components provide a consistent UI pattern for all modtools. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive stylesheet (~1600 lines) for modtools components: - Styling for shared components (ModToolsSection, HelpButton, StatusMessage) - Form input styles with RTL support - Collapsible section animations - Modal overlay for help content 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract 6 tools from ModeratorToolsPanel.jsx to individual files: - BulkDownloadText, BulkUploadCSV, WorkflowyModeratorTool - UploadLinksFromCSV, DownloadLinks (renamed from GetLinks), RemoveLinksFromCsv Changes: - Create utils/stripHtmlTags.js for shared HTML sanitization - Main file reduced from ~681 to ~92 lines - Modernized BulkDownloadText and BulkUploadCSV to functional components - Updated modtools/index.js with new exports All existing functionality preserved, just reorganized for maintainability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move CSS loading from JavaScript import (Webpack bundled) to a static <link> tag in base.html. This ensures the CSS is collected by Django's collectstatic and served properly in CI environments. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adjusted design system tokens and component styles: - Reduced spacing tokens by ~50% (xs: 2px, sm: 4px, md: 8px, lg: 12px, xl: 16px) - Reduced input padding from 12px 16px to 8px 12px - Reduced button padding from 12px 24px to 8px 16px - Reduced font sizes (inputs: 13px, labels: 13px, buttons: 13px) - Reduced input height from 48px to 36px - Reduced section title font size from 24px to 18px - Reduced page header font size from 28px to 22px - Reduced checkbox size from 18px to 14px - Made border radius smaller (sm instead of md for most elements) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace inline styles with design system classes (fieldGroup, hasError, fieldError) - Remove <br /> tags between form fields - Wrap form sections with fieldGroupSection class - Reduce gap spacing in legacy form CSS - Make fieldset and fieldGroupSection more compact Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove stripHtmlTags from shared/index.js (use String.prototype.stripHtml() instead) - Delete stripHtmlTags.test.js (testing built-in method not needed) - Update MODTOOLS_GUIDE.md to reference s2.css instead of modtools.css Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR establishes the backend foundation for bulk version editing capabilities. It introduces PyMongo 4.x compatibility updates, a new activity feed logging system for version metadata changes, and REST API endpoints to support bulk editing operations across multiple text indices.
Changes:
- New API endpoints for querying and bulk-editing version metadata across multiple indices
- Activity feed integration for tracking version metadata changes with new "edit version_metadata" revision type
- PyMongo 4.x migration replacing deprecated
update()withupdate_many()
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sefaria/views.py | Adds version_indices_api and version_bulk_edit_api endpoints with field whitelisting and partial success handling |
| sefaria/urls_shared.py | URL routing for new API endpoints |
| sefaria/tracker.py | New update_version_metadata() function centralizing metadata updates and history logging |
| sefaria/model/history.py | Adds version_metadata logging support and PyMongo 4.x migration |
| sefaria/history.py | Adds version_metadata filter type for activity queries |
| reader/views.py | Refactors flag_text_api and lock_text_api to use tracker.update_version_metadata() |
| sefaria/helper/text.py | PyMongo 4.x migration in split_text_section() |
| sefaria/helper/link.py | Bytes/string handling for CSV uploads |
| sefaria/export.py | Adds user_id parameter to import_versions_from_file() |
| templates/elements/activity_feed.html | Template block for displaying version_metadata events |
| templates/activity.html | Filter dropdown option for Version Metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vobj = Version().load({"title": title, "language": lang, "versionTitle": version}) | ||
|
|
||
| # Build updates dict for tracker function | ||
| updates = {} | ||
| if flags.get("newVersionTitle"): | ||
| vobj.versionTitle = flags.get("newVersionTitle") | ||
| updates["versionTitle"] = flags.get("newVersionTitle") | ||
| for flag in _attributes_to_save: | ||
| if flag in flags: | ||
| if flag == 'license' and flags[flag] == "": | ||
| delattr(vobj, flag) | ||
| updates[flag] = None # None signals deletion in tracker | ||
| else: | ||
| setattr(vobj, flag, flags[flag]) | ||
| vobj.save() | ||
| return jsonResponse({"status": "ok"}) | ||
| updates[flag] = flags[flag] | ||
|
|
||
| _attributes_to_save = Version.optional_attrs + ["versionSource", "direction", "isSource", "isPrimary"] | ||
| if updates: | ||
| tracker.update_version_metadata(user_id, vobj, updates) |
There was a problem hiding this comment.
The flag_text_api's update_version function doesn't check if the Version was found before using it. If Version().load() returns None (version doesn't exist), vobj will be None and building the updates dict will still proceed, then calling tracker.update_version_metadata will cause an AttributeError.
Add a check after line 2503:
if not vobj:
return jsonResponse({"error": "Version not found"})
There was a problem hiding this comment.
Fixed, see the other comment
| vobj = Version().load({"title": title, "language": lang, "versionTitle": version}) | ||
|
|
||
| if request.GET.get("action", None) == "unlock": | ||
| vobj.status = None | ||
| updates = {"status": None} # None signals deletion in tracker | ||
| else: | ||
| vobj.status = "locked" | ||
| updates = {"status": "locked"} | ||
|
|
||
| vobj.save() | ||
| tracker.update_version_metadata(request.user.id, vobj, updates) |
There was a problem hiding this comment.
The lock_text_api doesn't check if the Version was found before calling tracker.update_version_metadata. If Version().load() returns None (version doesn't exist), calling tracker.update_version_metadata(request.user.id, vobj, updates) on line 2481 will cause an AttributeError when trying to access vobj.contents() in the tracker function.
Add a check after line 2474:
if not vobj:
return jsonResponse({"error": "Version not found"})
There was a problem hiding this comment.
@dcschreiber Do we have validation anywhere that the user must add existing versions? I suspect we already have this validation somewhere in one of the PRs, so that CoPilot's comment here is irrelevant but I just wnat to confirm.
There was a problem hiding this comment.
[Claude] No, this validation didn't exist anywhere - Added if not vobj checks to both.
stevekaplan123
left a comment
There was a problem hiding this comment.
I agreed with some of CoPilot's comments and left comments of my own.
- Add Version existence checks in lock_text_api and flag_text_api - Add language field to version_bulk_edit_api (disambiguates same versionTitle across languages) - Reject empty updates dict in version_bulk_edit_api - Reject clearing required Version fields (derived from Version.required_attrs) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…36475/modtools-pr2-ui-refactor
…/sc-36475/modtools-pr3-bulk-version-editor
…36475/modtools-pr2-ui-refactor
…/sc-36475/modtools-pr3-bulk-version-editor
Clarify that versionTitle is a database identifier and cannot be bulk edited. Improve mark-for-deletion messaging to explain the two-step process: marking adds a note, then a developer completes the deletion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
title + versionTitle uniquely identifies a version (pkeys in Version model), so language is not needed for the lookup. Requiring it caused errors when the frontend didn't select a language filter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…36475/modtools-pr2-ui-refactor
…/sc-36475/modtools-pr3-bulk-version-editor
…ulk-version-editor feat(modtools): BulkVersionEditor feature (Part 3/3)
…i-refactor feat(modtools): UI refactoring and shared component library (Part 2/3)
The modtools page was missing the @ensure_csrf_cookie decorator, so the CSRF cookie was not guaranteed to be set when the page loaded. The new fetch-based CSV upload reads this cookie to set the X-CSRFToken header; without it, Django returns a 403 HTML response that the JS then fails to parse as JSON. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-fix fix(modtools): ensure CSRF cookie is set for CSV upload
b2997ed
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Keep update_version_metadata from branch and master's post_modify_text signature with explicit skip_links and count_after params. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Part 1 of 3: Backend Foundation
Related PRs:
Merge order: PR1 → PR2 → PR3 (all target master, merge in sequence)
!! Don't Merge into Master Until After MDL and Post MDL !!
Summary
This PR adds the backend foundation:
update()→update_many())edit version_metadata)New API Endpoints (sefaria/views.py)
Three new endpoints were added to support bulk version editing:
/api/version-indices(GET)?versionTitle=Tanach with Nikkud&language=he→{indices: ["Genesis", "Exodus", ...], metadata: {...}}library.get_index()for efficiency)/api/version-bulk-edit(POST){versionTitle, language, indices: [...], updates: {...}}{status: "ok"|"partial"|"error", successes: [...], failures: [{index, error}, ...]}VERSION_BULK_EDIT_ALLOWED_FIELDS) prevent arbitrary attribute injectionnullvalues are treated as "delete this field" - backend usesdelattrto remove from MongoDBtracker.update_version_metadata()to log changes to activity feed/api/check-index-dependencies/<title>(GET) - Disabled in frontendActivity Feed / History Logging
Added audit logging for version metadata changes. Previously, metadata edits (license, status, priority, etc.) were not tracked anywhere in the system.
New rev_type:
edit version_metadataChanges are now logged to the MongoDB
historycollection and displayed on the/activitypage.Implementation:
sefaria/model/history.pylog_version_metadata(user, old_dict, new_dict, **kwargs)- Creates history entry with old/new values, title, version, languagesefaria/model/__init__.pylog_version_metadatato exportssefaria/tracker.pyupdate_version_metadata(user, version, updates, **kwargs)- Centralized function: captures old state → applies updates → saves → logs to historyEndpoints updated to use tracker:
version_bulk_edit_api(sefaria/views.py) - The new bulk editorflag_text_api(reader/views.py) - Flag text featurelock_text_api(reader/views.py) - Lock/unlock text featureActivity Feed Display:
templates/elements/activity_feed.html- Added template block for version_metadata eventstemplates/activity.html- Added "Version Metadata" filter option to dropdownsefaria/history.py- Addedversion_metadatacase tofilter_type_to_query()Enhanced reader/views.py
Index API Error Handling:
Enhanced the
index_postfunction with categorized error responses:{update: {...}}) and new format (direct index data)dependency_error,validation_error,title_validation_error,general_errorVersion Metadata Logging:
Updated
flag_text_apiandlock_text_apito usetracker.update_version_metadata()for history logging.PyMongo 4.x Migration
Updated deprecated MongoDB operations:
sefaria/model/history.py:210:update()→update_many()sefaria/helper/text.py:322:update()→update_many()Minor Improvements
sefaria/export.py: Fixed missinguser_idparameter inimport_versions_from_file()sefaria/helper/link.py: Added bytes decoding for CSV uploads (handles both bytes and str)Files Changed
Backend (Python):
sefaria/views.py- New API endpoints, uses tracker for history loggingreader/views.py- Enhanced Index API error handling, flag_text_api and lock_text_api now use trackersefaria/urls_shared.py- URL routing for new endpointssefaria/tracker.py- Newupdate_version_metadata()functionsefaria/model/history.py- PyMongo 4.x migration, newlog_version_metadata()functionsefaria/model/__init__.py- Addedlog_version_metadataexportsefaria/history.py- Addedversion_metadatafilter typesefaria/helper/text.py- PyMongo 4.x migrationsefaria/helper/link.py- CSV bytes handlingsefaria/export.py- user_id parameter fixTemplates:
templates/elements/activity_feed.html- Added version_metadata display blocktemplates/activity.html- Added "Version Metadata" filter option🤖 Generated with Claude Code
Next PR
➡️ Part 2 is a draft: #2985 - Remove from draft status when this PR is merged.