fix(restacker): refactor and use ProjectContext permissions checks - #909
Open
jakeaturner wants to merge 8 commits into
Open
fix(restacker): refactor and use ProjectContext permissions checks#909jakeaturner wants to merge 8 commits into
jakeaturner wants to merge 8 commits into
Conversation
…elated API logic for improved performance and structure
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors project permission and book-attachment checks to use the centralized ProjectContext helper (and its permission checks) across Restacker/BookService/Remixer flows, reducing direct project/user lookups in those endpoints.
Changes:
- Extend
ProjectContextto always load LibreTexts book attachment fields and add helpers to validate/compose a project’s attachedbookID. - Update
BookService.canAccessPageto useProjectContextfor project membership checks. - Refactor multiple Restacker endpoints to use
ProjectContext+ProjectError-based error handling; minor Remixer cleanup for bookID composition.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/api/services/project-context.ts | Adds book-attachment fields to permission shape and introduces helpers for attached book ID + validation + new error code. |
| server/api/services/book-service.ts | Replaces ad-hoc user lookup + permission check with ProjectContext.canMember in canAccessPage. |
| server/api/restacker.ts | Refactors Restacker endpoints to ProjectContext permission checks and centralized error handling; adjusts restacker run parameters from attached book. |
| server/api/remixer.ts | Minor refactor to reuse a bookID variable and clarifying comment about when project-book enforcement applies. |
Suppressed comments (2)
server/api/services/project-context.ts:151
- Docstring: the composed book ID is
library-coverID(notlibrary-bookID). The current wording is confusing and doesn’t match the actual return value (${libreLibrary}-${libreCoverID}).
* Returns the book ID attached to the project, or null if no book is attached.
* @param this The ProjectContext instance.
* @returns An object containing the library, coverID, and the composed bookID (library-bookID), or null if no book is attached.
*/
server/api/services/project-context.ts:130
assertBookBelongsToProjectcurrently treats any string containing a hyphen as parseable (becausegetLibraryAndPageFromBookIDonly checkssplit('-').length > 1). That means values likelib-123-extraare considered valid and will pass if the first two segments match the project’s book, even though they aren’t validlib-coverIDidentifiers percheckBookIDFormat.
// Check that book ID can be parsed into a valid subdomain and path
const [subdomain, path] = getLibraryAndPageFromBookID(bookID);
if (!subdomain || !path) {
debug("[ProjectContext.assertBookBelongsToProject] Invalid bookID format:", bookID);
return false;
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+626
to
+629
| const bookService = new BookService({ bookID }); | ||
|
|
||
| const currentTags = await bookService.getPageTags(pageID); | ||
| const preservedTags = currentTags | ||
| .map((tag) => tag["@value"]) | ||
| .filter( | ||
| (tag) => | ||
| !tag.startsWith("license:") && !tag.startsWith("licenseversion:"), | ||
| const currentTags = await bookService.getPageTags(pageID); | ||
| const preservedTags = currentTags |
Comment on lines
+43
to
+45
| case "invalidbookid": | ||
| message = "Requested book ID is invalid or does not belong to this project"; | ||
| break; |
yghaemi
previously approved these changes
Aug 14, 2026
jakeaturner
dismissed
yghaemi’s stale review
August 17, 2026 17:27
The merge-base changed after approval.
jakeaturner
force-pushed
the
staging
branch
2 times, most recently
from
August 18, 2026 20:18
3068ccf to
78c3f6f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.