You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor: move add-resource flow into AddResourceWizard - #853
Extracts the add/edit-resource flow (upload → select sections → configure) out of PassageDetailArtifacts and PassageDetailsArtifactsMobile into a shared AddResourceWizard component + useAddResourceWizard hook, removing ~1,600 duplicated lines from the two views.
Adds ResourceUploadStep, useResourceArtifactTypes, and PassageRecordPanel (replaces PassageRecordDlg).
Slims Uploader / MediaRecord / MediaUploadContent now that the wizard owns step state; drops the unused localization string.
Manual: add passage / section / general audio resource (upload + record), Back between steps, edit an existing general resource, desktop and mobile views
The reason will be displayed to describe this comment to others. Learn more.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟡 Changes recommended
A critical functional issue remains in the edit flow where visual general resources are forced into an audio/segment-based Configure step that can prevent completion, alongside smaller standards and styling nits that still need addressing.
Refactors the add/edit resource workflow by consolidating the duplicated desktop/mobile “upload → select sections → configure” flow into a shared wizard + hook, and simplifying related upload/record components to match the new ownership model.
Changes:
Introduces AddResourceWizard + useAddResourceWizard to unify add/edit general-resource flows across desktop and mobile artifact views.
Adds new step/components and helpers (ResourceUploadStep, PassageRecordPanel, useResourceArtifactTypes) and removes legacy dialog code/tests.
Streamlines upload/record plumbing (Uploader, MediaUploadContent, MediaRecord, useMediaUpload) and removes an unused localization key.
File
Description
web/src/store/upload/actions.tsx
Adjusts upload retry/queue behavior around pending uploads to better align with general/project resource handling.
web/src/store/localization/reducers.tsx
Removes an unused localization string key from the runtime localization state.
web/src/store/localization/model.tsx
Updates localization typings to remove the deleted/unused string key.
web/src/crud/useMediaUpload.ts
Aligns media upload behavior with the renamed non-file upload type constant and related versioning logic.
web/src/control/ActionRow.tsx
Updates ActionRow layout/styling behavior to better support wizard footers and caller-provided sx.
web/src/components/useRecordSave.ts
Removes the deferred “stage file instead of upload” path now superseded by the wizard flow.
web/src/components/UploadType.ts
Renames the constant for non-file upload types (NO_LIST_TYPES → NON_FILE_UPLOAD_TYPES).
web/src/components/Uploader.tsx
Simplifies Uploader into an upload-only dialog now that recording and step state live in the wizard.
web/src/components/Uploader.test.tsx
Removes tests tied to the legacy record/defer behavior that was deleted from Uploader.
web/src/components/TitleUploader.tsx
Updates TitleUploader to match the revised MediaUploadContent API (including boolean ready and renamed button text prop).
web/src/components/Sheet/ScriptureTable.tsx
Updates ScriptureTable’s Uploader usage to match the simplified Uploader props surface.
web/src/components/PassageRecordPanel.tsx
Adds a new embedded record/upload panel component used by the wizard’s first step UI.
web/src/components/PassageRecordDlg.tsx
Removes the legacy combined record/upload dialog in favor of the new panel + wizard approach.
web/src/components/PassageRecordDlg.test.tsx
Removes tests that were specific to the deleted legacy record dialog component.
General resources are no longer queued for pending-upload retry, so remove
the now-dead restore/resume chain (projectresource restore kind,
pendingProjectResourceConfig, useResumePendingProjectResourceConfig).
Fix the no-op gutterBottom CSS key in PassageRecordPanel, add a cleanup
TODO for MediaRecord's setHasTake effect, and clarify that unfinished
wizard work is dropped on save-all.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟡 Changes recommended
The change is blocked by unresolved critical and moderate findings, including a wizard flow that misroutes non-audio resources to an audio-only configuration step, an effect that mirrors derived state into parent state in a way that can create subtle loops, and removal of legacy pending-upload restore handling that could break retries for existing persisted rows.
The reason will be displayed to describe this comment to others. Learn more.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🔵 Needs a closer look
Approval is blocked by two critical unresolved issues: the new general-resource edit flow likely breaks visual media by forcing it through an audio/segment-based wizard, and ActionRow spreads sx in a way that will crash at runtime for valid array/function sx values.
General resources can be “visual” (see resourceRows.ts using isVisual(media)), but this edit path now routes every general resource into AddResourceWizard → ProjectResourceConfigure, which is explicitly audio/segment-based (e.g. it parses regions and says “Segment limits come from the audio”). That likely breaks editing visual general resources that previously skipped Configure (writeVisualResource). Consider either (1) preventing editGeneralResource for visual media and keeping the immediate-write flow, or (2) teaching the wizard to detect visual media and bypass Configure with an equivalent assignment write path.
Spreading MUI sx can crash when callers pass arrays or functions
web/src/control/ActionRow.tsx:13
sx in MUI can be an array or a function; spreading it (...sx) will throw at runtime when callers pass those valid forms. Compose the default styles with the caller-provided sx via an sx array instead.
Copilot keeps worrying about visual general resources. We already discussed that there's no such thing as non-audio general resources. No way to make them. We don't support
Address SonarQube findings in files touched by the general resource refactor
(PR #853): mark intentionally-unawaited promises with void, reject upload
promises with an Error subclass (UploadFileReject), and prefer
Number.isNaN/Number.parseFloat. No behavior change intended.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
)
* chore: Sonarqube changes adjacent to the general resource refactor
Address SonarQube findings in files touched by the general resource refactor
(PR #853): mark intentionally-unawaited promises with void, reject upload
promises with an Error subclass (UploadFileReject), and prefer
Number.isNaN/Number.parseFloat. No behavior change intended.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* chore: handle rejections of fire-and-forget promises
Per CODING_STANDARDS Async (Devin/Copilot review): Orbit writes in the
artifacts views now log and show failures; ScriptureTable save/publish waits
log or show their errors; the remaining void calls carry a comment saying
why they cannot reject.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
PassageDetailArtifactsandPassageDetailsArtifactsMobileinto a sharedAddResourceWizardcomponent +useAddResourceWizardhook, removing ~1,600 duplicated lines from the two views.ResourceUploadStep,useResourceArtifactTypes, andPassageRecordPanel(replacesPassageRecordDlg).Uploader/MediaRecord/MediaUploadContentnow that the wizard owns step state; drops the unused localization string.Test plan
npm run typecheck(passes locally)AddResourceWizard.test.tsx,ResourceUploadStep.test.tsx,useResourceArtifactTypes.test.ts,PassageDetailArtifacts.test.tsx🤖 Generated with Claude Code