[PB Extension] Create FW project#2394
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a lexicon creation form, localized UI strings, a creation command, MiniLCM project creation integration, project-type URL routing, and selection web-view states for creating and selecting lexicons. ChangesLexicon creation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
myieye
left a comment
There was a problem hiding this comment.
Generally looks fine to me. 👍
|
|
||
| function isValidLangTag(tag: string): boolean { | ||
| try { | ||
| return Intl.getCanonicalLocales(tag).length === 1; |
There was a problem hiding this comment.
What's the === 1 about? I'd expect >= 1.
| async getProjects(): Promise<IProjectModel[]> { | ||
| return (await this.fetchPath('localProjects')) as IProjectModel[]; | ||
| const projects = (await this.fetchPath('localProjects')) as IProjectModel[]; | ||
| projects.forEach((p) => FwLiteApi.projectTypeByCode.set(p.code, p.crdt ? 'Harmony' : 'FwData')); |
There was a problem hiding this comment.
If I'm not mistaken, these projects are sometimes flagged as both crdt and fwdata.
There was a problem hiding this comment.
Correct, and if the project has both flags, don't we want to default to Harmony interaction?
26cae9e to
77b6250
Compare
Address review feedback on #2394: - validateLexiconCode checked `.analysis` truthiness, but writing-system fields are arrays, so an empty array always passed. Require a vernacular writing system instead (analysis is optional; selectLexicon handles its absence). - The create-lexicon form now blocks codes that already exist before the create round-trips, showing a localized message instead of a raw 400. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`fetchUrl` now reads the response as text before conditionally parsing JSON, so endpoints that return an empty 200 (like DELETE entry and the new project/create route) no longer throw a SyntaxError. `FwLiteApi.createProject` wraps `POST /api/project/create`, which was added in #2281 to resolve #1920. Closes #2061. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds an end-to-end flow for creating a new FW Lite CRDT project from within the SelectLexicon dialog: - New `lexicon.createLexicon` PAPI command wrapping FwLiteApi.createProject - New CreateLexicon form component (name, auto-derived code, vernacular WS pre-filled from the Paratext project language, optional analysis WS) - "Create new lexicon" button in LexiconComboBox opens the form - On successful creation the new lexicon is auto-selected and the dialog shows the standard "Lexicon selection saved" confirmation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…key order - fw-lite-api: add static projectTypeByCode cache (shared across FwLiteApi instances including EntryService) so CRDT projects created via createProject route through mini-lcm/Harmony/... instead of mini-lcm/FwData/... getProjects() populates the cache; createProject() seeds 'Harmony' immediately so the writingSystems call in selectLexicon succeeds without a re-fetch - fw-lite-api: include response body text in error messages so backend validation failures (duplicate code, invalid lang tag, etc.) reach the logger - create-lexicon: validate vernacular and analysis language tags with Intl.getCanonicalLocales() before enabling the submit button - localized-string-keys / localizedStrings.json: restore alphabetical ordering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- deriveCode now replaces invalid characters with '-' instead of removing them, matching the backend SanitizeProjectCode behavior - WebView onCreated catches selectLexicon failure: logs the error, refreshes the lexicon list, and falls back to the combo box so the user can manually select the project that was created on disk Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Svelte frontend routes FwData projects via /paratext/fwdata/{code}
and CRDT/Harmony projects via /paratext/project/{code}. The standalone
getBrowseUrl always used 'fwdata', producing a broken URL for any
Harmony lexicon.
Move getBrowseUrl into FwLiteApi as a method so it can consult the
projectTypeByCode cache and emit 'project' vs 'fwdata' accordingly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Hints the default analysis writing system on the Create new lexicon form, per review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Display the backend error message (e.g. duplicate code, invalid language tag) instead of only logging it, per review feedback. Also drop the now- pointless useCallback around handleSubmit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ping
- createLexicon now returns the backend validation message (e.g. "Project
already exists", "not a valid IETF language tag") instead of a generic
failure, so the create-lexicon form shows the actual reason. Also logs the
real message rather than JSON.stringify(e), which rendered Errors as {}.
- Type the SelectLexicon WebView provider and its caller as
LexiconWebViewOptions so vernacularLanguage is part of the declared contract.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cons The in-memory projectTypeByCode map is empty after an extension restart, so a previously created Harmony/CRDT lexicon was misrouted to the FwData endpoints (browse, search, and entry operations all failed) until the user reopened the lexicon selector. checkLexiconCode and getBrowseUrl now resolve the type via a new resolveProjectType helper, which reloads the project list from the backend on a cache miss before falling back to FwData. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
selectLexicon awaited the command but ignored its result, so onCreated and
LexiconComboBox.saveSetting showed a "saved" confirmation even if the command
returned { success: false }. Throw on a non-success result so the create flow
falls back to the combo box and the combo box surfaces its save error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on #2394: - validateLexiconCode checked `.analysis` truthiness, but writing-system fields are arrays, so an empty array always passed. Require a vernacular writing system instead (analysis is optional; selectLexicon handles its absence). - The create-lexicon form now blocks codes that already exist before the create round-trips, showing a localized message instead of a raw 400. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
doesProjectMatchLangTag used Object.keys on the vernacular IWritingSystem[],
which yields index strings ('0', '1', ...) rather than language tags, so
getProjectsMatchingLanguage never matched and always fell back to all
projects. Map over the array and read each writing system's wsId instead.
Also drops a redundant comment on the lexicon-code validator.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ee4770c to
07ce2d1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platform.bible-extension/src/components/create-lexicon.tsx (1)
107-119: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd validation feedback for code pattern mismatches.
When a user manually edits the code field and types uppercase letters or invalid characters,
CODE_PATTERN.test(code)fails and the submit button is disabled, but no error message is shown — onlycodeExistshas a visible error. Consider either auto-lowercasing the input or displaying a validation message when the code fails the pattern.♻️ Suggested fix: auto-lowercase code input
<Input id="createLexiconCode" onChange={(e) => { - setCode(e.target.value); + setCode(e.target.value.toLowerCase()); setCodeEdited(true); }} value={code} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform.bible-extension/src/components/create-lexicon.tsx` around lines 107 - 119, Show validation feedback when the manually edited value in the code input fails CODE_PATTERN, alongside the existing codeExists message, and ensure the submit-disabled condition uses the same validation state. Alternatively, normalize input to lowercase in the createLexiconCode onChange handler before calling setCode, while preserving setCodeEdited.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@platform.bible-extension/src/components/create-lexicon.tsx`:
- Around line 107-119: Show validation feedback when the manually edited value
in the code input fails CODE_PATTERN, alongside the existing codeExists message,
and ensure the submit-disabled condition uses the same validation state.
Alternatively, normalize input to lowercase in the createLexiconCode onChange
handler before calling setCode, while preserving setCodeEdited.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 46132e95-f38d-433c-a579-dc5d00cad5b6
📒 Files selected for processing (10)
platform.bible-extension/contributions/localizedStrings.jsonplatform.bible-extension/src/components/create-lexicon.tsxplatform.bible-extension/src/components/lexicon-combo-box.tsxplatform.bible-extension/src/main.tsplatform.bible-extension/src/types/lexicon.d.tsplatform.bible-extension/src/types/localized-string-keys.tsplatform.bible-extension/src/utils/fw-lite-api.tsplatform.bible-extension/src/utils/project-manager.tsplatform.bible-extension/src/web-views/index.tsxplatform.bible-extension/src/web-views/select-lexicon.web-view.tsx
Previously an invalid code (uppercase, spaces, symbols) silently disabled Submit with no feedback, unlike the codeExists case which already had a visible error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Auto-lowercase the code as typed (matching Lexbox's create-project zod schema) instead of erroring on uppercase, and add Lexbox's 4-character minimum length with its own message. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Lets users create a brand-new FW Lite lexicon directly from the lexicon selector in Platform.Bible instead of only picking an existing one. This wires up the backend create endpoint and the project-type-aware URL routing the flow needs, and fixes a few adjacent bugs (empty-response parsing, language-tag filtering, lexicon-code validation) surfaced along the way.
Summary
FwLiteApi.createProject(name, code, vernacularWs, analysisWs?)to the Platform.Bible extension's HTTP client, wrapping thePOST /api/project/createendpoint added in Wire up template-based project creation via JSON snapshot import #2281.fetchUrl: it previously calledresponse.json()unconditionally, which throws aSyntaxErroron empty-body responses (HTTP 200 with no content). The fix reads the body as text first and only parses JSON when the body is non-empty. On error responses it now throws with the response body, so backend validation messages survive. This also silently affecteddeleteEntry.FwDatavsHarmony/CRDT) so lexicon and browse URLs route to the correct backend endpoints. The lookup is self-healing: because the cache is in-memory only, on a miss (e.g. after an extension restart) it reloads the project list from the backend before falling back toFwData, so a previously created CRDT lexicon stays reachable without the user reopening the selector.lexicon.lexiconCodeproject-settings validator, which accepted any resolvable code because it tested an always-truthy array (.analysis); it now requires at least one vernacular writing system (analysis writing systems remain optional and are handled byselectLexicon).Changes
platform.bible-extension/src/utils/fw-lite-api.tsfetchUrl:results.json()→results.text()+ conditionalJSON.parse; throws the response body on error responsesFwLiteApi.createProject: new public method usingURLSearchParamsfor the create endpointprojectTypeByCodecache +resolveProjectType:checkLexiconCodeandgetBrowseUrl(now async) resolve a project's type, repopulating from the backend on a cache missdoesProjectMatchLangTag: reads each vernacular writing system'swsIdinstead ofObject.keyson the array (which returned index strings), sogetProjectsMatchingLanguageactually filters — a path the select-lexicon flow newly exercises by passingprojectIdtolexicon.lexiconsplatform.bible-extension/src/main.ts— registers thelexicon.createLexiconcommand (returns the backend error message on failure);validateLexiconCodenow checks for a vernacular writing system instead of the always-truthy.analysisarrayplatform.bible-extension/src/types/lexicon.d.ts—'lexicon.createLexicon'command handler type; optionalerroronSuccessHolderplatform.bible-extension/src/types/localized-string-keys.ts+contributions/localizedStrings.json— 9 new%lexicon_createLexicon_*%stringsplatform.bible-extension/src/components/create-lexicon.tsx— new form component; pre-checks the entered code against existing lexicons and surfaces backend error messagesplatform.bible-extension/src/components/lexicon-combo-box.tsx— optionalonCreateNewprop adds the "Create new lexicon" buttonplatform.bible-extension/src/utils/project-manager.ts— passes the project's vernacular language tag when opening the SelectLexicon WebViewplatform.bible-extension/src/web-views/select-lexicon.web-view.tsx— wires up the create flow with auto-selection on success, passing existing codes for the duplicate pre-checkplatform.bible-extension/src/web-views/index.tsx— types the SelectLexicon provider asLexiconWebViewOptionssovernacularLanguageis part of the declared contractImplemented by Claude Sonnet 4.6 & Opus 4.8.
Devin review: https://app.devin.ai/review/sillsdev/languageforge-lexbox/pull/2394
Closes #2061