TT-7631 Keep note category, title, and graphic in sync on the plan sheet TT-7634 Graphic Picker from library shows error on open - #533
Open
sarahentzel wants to merge 5 commits into
Open
Conversation
Persist NOTE|{category} on the passage when a note shared resource is created or updated,
and refresh the sheet row (reference, graphic, title, pencil) without marking the grid dirty.
Merge sheet rebuilds so a stale bare NOTE snapshot cannot wipe NOTE|{category}, but skip merge while a new row has no passage id so save-then-edit does not duplicate it.
Note details load note categories on first paint; skip unusable graphic URLs and HTML library responses.
sarahentzel
marked this pull request as ready for review
August 24, 2026 20:56
sarahentzel
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves note/shared-resource behavior in the renderer so that note category/title/graphic stay in sync on the plan sheet without marking the grid dirty, and hardens the graphic picker/library fetch flow against invalid/HTML responses and unusable graphic URLs.
Changes:
- Persist
NOTE|{category}in the underlying passage reference during shared-resource create/update, and refresh the active sheet row (reference/graphic/title/pencil) without flagging unsaved grid changes. - Improve sheet rebuild/merge behavior to avoid stale “bare NOTE” snapshots wiping categorized note references, and avoid merging while new rows lack a passage id.
- Harden graphic/library UX by validating URL-building prerequisites, filtering unusable graphic URLs, and handling non-JSON/HTML library responses.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/src/crud/useSharedResUpdate.ts | Updates passage reference on shared note updates and triggers sheet-row refresh callback. |
| src/renderer/src/crud/useSharedResCreate.ts | Updates passage reference on shared note create and triggers sheet-row refresh callback. |
| src/renderer/src/crud/useGraphicUrlBuilder.ts | Avoids building graphic API URLs when the base endpoint is missing. |
| src/renderer/src/crud/useGraphicFind.ts | Improves category lookup robustness for NOTE references (localized vs slug). |
| src/renderer/src/components/Sheet/SelectArtifactCategory.tsx | Adds cancellation handling while loading categories on first paint. |
| src/renderer/src/components/Sheet/ScriptureTable.tsx | Refreshes sheet rows without marking dirty; adds merge gating and ownership checks for graphic rights/fullsize URLs. |
| src/renderer/src/components/Sheet/PlanSheet.tsx | Ensures grid refill occurs when publishing/permission flags change. |
| src/renderer/src/components/Sheet/getSheet.ts | Merges sheet rebuilds more safely to preserve categorized NOTE references and avoid incorrect graphic recomputation. |
| src/renderer/src/components/ResourceEdit/ResourceTabs.tsx | Simplifies note detection and formatting for linked note read-only logic. |
| src/renderer/src/components/ResourceEdit/ResourceOverview.tsx | Keeps dialog state note aligned with derived note-ness and consolidates effects. |
| src/renderer/src/components/ResourceEdit/ResourceCategory.tsx | Fixes category type selection for notes vs resources and tightens imports. |
| src/renderer/src/components/ResourceEdit/ResourceCategory.test.tsx | Adds unit test coverage for selecting note vs resource categories. |
| src/renderer/src/components/isUsableGraphicUrl.ts | Adds a helper to reject incomplete/invalid S3 graphic keys while allowing data/blob URLs. |
| src/renderer/src/components/isUsableGraphicUrl.test.ts | Adds unit tests for the usable-URL helper. |
| src/renderer/src/components/GraphicPicker.tsx | Handles missing URLs and non-JSON/HTML responses from the library endpoint more gracefully. |
| src/renderer/src/components/apmGraphic.ts | Filters unusable graphic URIs/URLs before returning them to callers. |
| src/renderer/src/tests/getSheet.test.ts | Adds regression tests for NOTE merge behavior and correct graphic attachment during merges. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
44
to
62
| if (sharedResource.attributes.note) { | ||
| const catRec = findRecord(memory, 'artifactcategory', category) as | ||
| | ArtifactCategory | ||
| | undefined; | ||
| if (catRec && onUpdRef) { | ||
| const catText = localizedArtifactCategory( | ||
| catRec.attributes?.categoryname | ||
| newRef = catRec | ||
| ? `NOTE|${localizedArtifactCategory(catRec.attributes?.categoryname)}` | ||
| : 'NOTE'; | ||
| if (passage && passage.attributes.reference !== newRef) { | ||
| ops.push( | ||
| ...UpdateRecord( | ||
| t, | ||
| { | ||
| ...passage, | ||
| attributes: { ...passage.attributes, reference: newRef }, | ||
| }, | ||
| user | ||
| ) | ||
| ); | ||
| const passage = sharedResource.relationships.passage | ||
| .data as RecordIdentity; | ||
| onUpdRef(passage.id, `NOTE|${catText}`, sharedResource); | ||
| } |
Comment on lines
111
to
123
| useEffect(() => { | ||
| let cancelled = false; | ||
| setGettingCategories(true); | ||
| getCategorys().then((cats) => { | ||
| if (cancelled) return; | ||
| setArtifactCategorys(cats); | ||
| setGettingCategories(false); | ||
| }); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [artifactCategories, scripture, org, type]); |
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.
Persist NOTE|{category} on the passage when a note shared resource is created or updated, and refresh the sheet row (reference, graphic, title, pencil) without marking the grid dirty. Merge sheet rebuilds so a stale bare NOTE snapshot cannot wipe NOTE|{category}, but skip merge while a new row has no passage id so save-then-edit does not duplicate it. Note details load note categories on first paint; skip unusable graphic URLs and HTML library responses.