Repository navigation
Conversation
Re-implements PR #510's artifact-type slug refactor on top of #853. Local code now identifies artifact types by ArtifactTypeSlug and converts to an id only at the DB/API boundary: MediaRecord, useMediaUpload, Uploader and the add-resource wizard take artifactTypeSlug instead of an artifact-type id. Step settings now store the slug. Every settings reader goes through the slug-tolerant slugFromId / new localIdFromIdOrSlug, so legacy settings that hold a remote or local id keep working without a data migration. Also drops the dead addNewArtifactType / commentId / keyTermId helpers and replaces hardcoded typename literals with the enum. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟡 Changes recommended
A critical issue remains in localIdFromIdOrSlug where an unguarded remoteIdGuid(..., memory?.keyMap as RecordKeyMap) can throw when context is undefined and may return an unmapped remote id despite the function contract, leaving callers with an invalid relationship id.
3 open issues
What changed in this PR
Refactors artifact-type handling to be slug-first across the web app, storing slugs in workflow step settings and converting to local/remote IDs only at Orbit/API boundaries.
Changes:
- Introduces slug utilities and new
useArtifactTypehelpers (localIdFromSlug,remoteIdNumFromSlug,localIdFromIdOrSlug) and removes dead helpers. - Updates recording/upload flows to pass
artifactTypeSlugend-to-end with centralized slug→id conversion inuseMediaUpload. - Updates step-settings writers to persist slugs while keeping readers tolerant of legacy stored IDs, including a mobile transcribe resolution fix.
| File | Description |
|---|---|
| web/src/utils/integrationSlug.ts | Updates integration slug construction to use ArtifactTypeSlug enum values instead of hardcoded typename strings. |
| web/src/store/importexport/actions.tsx | Refactors import/export filtering to use localIdFromSlug in place of the prior type-id resolver. |
| web/src/routes/PassageDetail.tsx | Adjusts PassageDetail route wiring to rely on slug/id-tolerant artifact-type helpers for step-related artifact selection. |
| web/src/routes/ElectronImport.tsx | Updates Electron import flow to use localIdFromSlug for artifact-type resolution. |
| web/src/crud/useOfflineSetup.ts | Seeds offline workflow setup with slug-based artifact-type settings and replaces typename literals with enum values. |
| web/src/crud/useMediaUpload.ts | Switches upload inputs to artifactTypeSlug and performs slug→local/remote id conversion only at persistence/API boundaries. |
| web/src/crud/useMediaUpload.test.ts | Updates media upload hook tests to match the new slug-based upload contract and conversion behavior. |
| web/src/crud/useMediaResCreate.ts | Uses localIdFromSlug when creating media relationships for shared resources. |
| web/src/crud/useArtifactType.ts | Implements slug-first artifact-type helpers and removes unused legacy helpers while keeping slug/id normalization available to callers. |
| web/src/crud/updateBackTranslationType.ts | Replaces hardcoded back-translation typename checks with ArtifactTypeSlug enum comparisons. |
| web/src/crud/fontChoice.ts | Makes font selection logic tolerant of step settings containing either slugs or legacy ids. |
| web/src/crud/artifactTypeSlug.ts | Adds a centralized slug list and type guard (artifactTypeSlugs, isArtifactTypeSlug) for validated slug usage. |
| web/src/context/TranscriberContext.tsx | Updates transcriber context to resolve artifact type via slug-based helpers rather than id-based plumbing. |
| web/src/context/PassageDetailContext.tsx | Adjusts PassageDetail context filtering and lookups to use localIdFromSlug for artifact types. |
| web/src/components/useRecordSave.test.ts | Updates record-save tests to pass slug-based artifact typing through upload fields. |
| web/src/components/Uploader.tsx | Changes the uploader component API to accept artifactTypeSlug and threads it through to upload handling. |
| web/src/components/TranscriptionTab.tsx | Updates transcription/export tab filtering and selection to use slug-based artifact-type resolution. |
| web/src/components/TitleTabs.tsx | Refactors title recording tab logic to drive recording/upload behavior by slugs rather than derived ids. |
| web/src/components/TitleRecord.tsx | Updates title recording component to pass artifactTypeSlug into MediaRecord instead of an artifact-type id. |
| web/src/components/TitleRecord.test.tsx | Adjusts title record tests for the slug-based MediaRecord/upload contract. |
| web/src/components/StepEditor/TranscribeStepSettings.tsx | Writes transcribe step settings as slugs and normalizes legacy stored ids to slug values on load. |
| web/src/components/StepEditor/StepEditor.tsx | Updates step editor behavior to treat artifact type settings as id-or-slug and rely on normalized slug helpers. |
| web/src/components/StepEditor/StepEditor.cy.tsx | Updates Cypress step editor coverage/comments to reflect the renamed slug-first artifact-type helpers. |
| web/src/components/StepEditor/PhraseBackTranslateStepSettings.tsx | Stores phrase back-translation artifact type as a slug and normalizes existing legacy values when editing. |
| web/src/components/StepEditor/PhraseBackTranslateStepSettings.test.tsx | Updates phrase back-translation settings tests to assert slug persistence and legacy-id normalization. |
| web/src/components/StepEditor/ParatextStepSettings.tsx | Updates Paratext step settings to persist artifact types as slugs while accepting legacy ids. |
| web/src/components/StepEditor/isDuplicatePhraseBtLanguage.ts | Refactors duplicate phrase BT language detection to compare normalized slug values rather than id mappings. |
| web/src/components/StepEditor/isDuplicatePhraseBtLanguage.test.ts | Adds/updates tests covering duplicate detection across mixed legacy-id and slug-based settings. |
| web/src/components/Sheet/SelectArtifactType.tsx | Updates artifact-type selector to emit slugs (or null for Vernacular) and use slug-backed options for selection. |
| web/src/components/ProvideRights.tsx | Threads artifactTypeSlug through the Provide Rights recording/upload path instead of id-based artifact typing. |
| web/src/components/PassageRecordPanel.tsx | Updates record panel contract to accept artifactTypeSlug and forward it into recording/upload components. |
| web/src/components/PassageDetail/Prompt/PassageDetailPromptAdmin.tsx | Refactors prompt admin recording to use a fixed ArtifactTypeSlug value for consistent upload tagging. |
| web/src/components/PassageDetail/PassageDetailStepComplete.tsx | Uses slug/id normalization helpers to derive the step-complete artifact type consistently from settings. |
| web/src/components/PassageDetail/PassageDetailRecord.tsx | Updates passage recording to use ArtifactTypeSlug.Vernacular (slug-first) rather than id-derived typing. |
| web/src/components/PassageDetail/PassageDetailRecord.test.tsx | Updates PassageDetail record tests to use the slug-based artifact typing and helper APIs. |
| web/src/components/PassageDetail/PassageDetailPhraseBackTranslate.undo.test.tsx | Adjusts undo tests to use the new slug-first helpers and settings normalization behavior. |
| web/src/components/PassageDetail/PassageDetailPhraseBackTranslate.tsx | Refactors phrase back-translation step to resolve artifact type via slugFromId for slug-or-id settings. |
| web/src/components/PassageDetail/PassageDetailParatextIntegration.tsx | Resolves Paratext integration artifact types using localIdFromIdOrSlug at the local DB relationship boundary. |
| web/src/components/PassageDetail/PassageDetailLwcTranslation.tsx | Updates LWC translation flow to normalize artifact type via slug/id helpers and pass slugs to controls/recording. |
| web/src/components/PassageDetail/PassageDetailLwcTranslation.test.tsx | Updates LWC translation tests to align with slug-based artifact typing and helper behavior. |
| web/src/components/PassageDetail/PassageDetailLwcTranscription.tsx | Refactors LWC transcription to compute local ids via localIdFromIdOrSlug and default slugs via localIdFromSlug. |
| web/src/components/PassageDetail/PassageDetailLwcTranscription.test.tsx | Updates LWC transcription tests for the slug-first settings and artifact-type resolution logic. |
| web/src/components/PassageDetail/PassageDetailItem.tsx | Threads artifactTypeSlug through item-level record/upload interactions to match the new slug-first contract. |
| web/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx | Updates guided phrase recording to resolve settings via localIdFromIdOrSlug and pass slug-based typing into recording. |
| web/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx | Updates guided phrase record step-scope tests for the slug-based artifact-type APIs. |
| web/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.nextDuringPlayback.test.tsx | Updates guided phrase record playback tests to use the new slug-first artifact-type wiring. |
| web/src/components/PassageDetail/PassageDetailGrids.tsx | Updates grids step artifact-type resolution to use localIdFromIdOrSlug for slug-or-id settings values. |
| web/src/components/PassageDetail/PassageDetailCarefulSpeech.test.tsx | Updates Careful Speech test mocks and helpers to match slug-first artifact-type usage. |
| web/src/components/PassageDetail/mobile/transcribe/PassageDetailTranscribeMobile.tsx | Fixes mobile transcribe to resolve step settings (slug or legacy id) to the correct local id like desktop does. |
| web/src/components/PassageDetail/mobile/record/ProvideRightsMobile.tsx | Updates mobile Provide Rights recording to accept artifactTypeSlug rather than an artifact-type id. |
| web/src/components/PassageDetail/mobile/record/ProvideRightsMobile.cy.tsx | Updates mobile Provide Rights Cypress coverage to use slug-based artifact typing inputs. |
| web/src/components/PassageDetail/lwcTranslation/LwcTranslationControls.tsx | Changes LWC translation controls to accept and forward artifactTypeSlug to recording/upload components. |
| web/src/components/PassageDetail/lwcTranslation/LwcTranslationControls.test.tsx | Updates LWC translation controls tests for slug-based props and recording/upload expectations. |
| web/src/components/PassageDetail/Keyterms/TargetWordAdd.tsx | Updates key term recording to tag uploads using ArtifactTypeSlug.KeyTerm directly. |
| web/src/components/PassageDetail/Internalization/useProjectResourceSave.ts | Refactors project resource save logic to resolve artifact types via localIdFromSlug. |
| web/src/components/PassageDetail/Internalization/useAddResourceWizard.ts | Updates add-resource wizard state/logic to choose artifact typing by slug for downstream uploads. |
| web/src/components/PassageDetail/Internalization/ResourceUploadStep.tsx | Threads artifactTypeSlug through the resource upload step’s record/upload chain consistent with the new contract. |
| web/src/components/PassageDetail/Internalization/ResourceUploadStep.test.tsx | Updates resource upload step tests to validate slug-based artifact typing propagation and defaults. |
| web/src/components/PassageDetail/Internalization/resourceRows.ts | Replaces hardcoded resource-related typenames with slug-based enum logic used for row classification/behavior. |
| web/src/components/PassageDetail/Internalization/AddResourceWizard.tsx | Passes wizard-selected artifactTypeSlug into ResourceUploadStep and related internalization flows. |
| web/src/components/PassageDetail/Internalization/AddResourceWizard.test.tsx | Updates add-resource wizard tests for slug-based artifact typing and updated component contracts. |
| web/src/components/PassageDetail/carefulSpeech/CarefulSpeechControls.tsx | Updates Careful Speech controls to accept artifactTypeSlug and forward it through recording/upload. |
| web/src/components/PassageDetail/carefulSpeech/CarefulSpeechControls.test.tsx | Updates Careful Speech controls tests to use slug-based props and expectations. |
| web/src/components/PassageDetail/carefulSpeech/CarefulSpeechControls.cy.tsx | Updates Careful Speech Cypress component tests for slug-first artifact typing inputs. |
| web/src/components/MediaRecord.tsx | Updates MediaRecord to accept artifactTypeSlug and pass it into the media upload hook fields. |
| web/src/components/MediaRecord.test.tsx | Refactors MediaRecord unit tests to use the slug-based props contract and vernacular defaults. |
| web/src/components/MediaRecord.recording.cy.tsx | Updates MediaRecord recording Cypress tests to use slug-based artifact typing props. |
| web/src/components/MediaRecord.load.test.tsx | Updates MediaRecord load tests for the new artifactTypeSlug prop contract. |
| web/src/components/MediaRecord.deleteRegion.cy.tsx | Updates MediaRecord delete-region Cypress tests for slug-based props and upload behavior. |
| web/src/components/Integration.tsx | Updates integration behavior to use remoteIdNumFromSlug at the API boundary and localIdFromSlug for local filtering. |
| web/src/components/Discussions/DiscussionCard.addSave.test.tsx | Updates discussion add/save tests to remove reliance on removed artifact-type id helpers and use slug-based behavior. |
| web/src/components/Discussions/CommentEditor.tsx | Refactors comment recording to tag uploads using ArtifactTypeSlug.Comment instead of id-derived artifact types. |
| web/src/components/ArtifactStatus.tsx | Updates artifact status computation to resolve artifact type via localIdFromSlug rather than legacy id helpers. |
Review effort: Balanced
Comment on lines
+140
to
+147
| const localIdFromIdOrSlug = (value: string | null | undefined) => { | ||
| if (!value) return null; | ||
| if (isArtifactTypeSlug(value)) return localIdFromSlug(value); | ||
| return ( | ||
| remoteIdGuid('artifacttype', value, memory?.keyMap as RecordKeyMap) ?? | ||
| value | ||
| ); | ||
| }; |
| @@ -27,27 +28,20 @@ export const SelectArtifactType = (props: IProps) => { | |||
| ); | |||
|
|
|||
| const handleArtifactTypeChange = (e: any) => { | |||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
Collaborator
Author
|
I want to put this off until after we figure out the mobile transcribe step. I will close for now but not delete the branch. |
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.
Re-implements the slug refactor from #510 on top of #853, so it targets #853's branch rather than develop. #510's other work, the TT-7583 autosave hardening, already landed in develop via #492.
What changed
getTypeIdis renamedlocalIdFromSlug.remoteIdNumFromSluggives the remote id for the API boundary.localIdFromIdOrSlugresolves values read from step settings.slugFromIdnow passes slugs through unchanged.artifactTypeSlugs/isArtifactTypeSlug.addNewArtifactType,commentIdandkeyTermId.artifactTypeSluginstead of an artifact-type id:useMediaUpload,MediaRecord,PassageRecordPanel,ResourceUploadStep/ add-resource wizard,Uploader, Careful Speech / LWC controls,ProvideRights(Mobile), titles, comments, key terms and the prompt. Slug → id conversion happens only insideuseMediaUpload.SelectArtifactType, Transcribe / Paratext / Phrase BT step settings, and the offline workflow seed inuseOfflineSetup.slugFromIdorlocalIdFromIdOrSlug, so existing settings that hold a remote or local id keep working with no migration. That covers Grids, guided phrase record, LWC transcription/translation, Paratext integration, PBT, step complete, the route, StepEditor labels, mobile transcribe,fontChoiceand the duplicate-PBT-language check.resourceRows,useOfflineSetup,updateBackTranslationType,integrationSlug).TranscriberProviderand the font lookup as if it were a local id. It now resolves to the local id the same way desktop does.Compatibility risk
Step settings sync to the whole team. A teammate on an older app version will read the new slug values, and older readers resolve the stored value as a remote id, so their slug-based steps (phrase BT, LWC, Grids) may not find their recordings until they update. This was a deliberate choice: slugs plus slug-tolerant readers, accepting that older clients may misread them. Worth confirming that no server-side code reads
artifactTypeIdfrom step settings.Test plan
npm run typecheckProjectResourceConfigure.test.tsxfails the same way on refactor: move add-resource flow into AddResourceWizard #853 (parse error).MediaRecord,MarkVerses,AsrSettingsandPendingUploadsDialog.retryfailed only under full-suite load and pass when run on their own.🤖 Generated with Claude Code