refactor(wikibase-schema-editor): harmonize editing terms #126
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughRemoves useLanguageDropZone/useSchemaDropZone composables, their tests, and the LanguageDropZone component. Rewrites SchemaDropZone to use schemaStore/dragDropStore with typed props and centralized validation. Updates TermSection and TermsEditor to manage language selection, display/remove mappings, and bind store-backed values. Adjusts schema.store typings and minor control flow. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SDZ as SchemaDropZone.vue
participant DDS as dragDropStore
participant VAL as Validation
participant SS as schemaStore
U->>SDZ: Drag over/drop Column
SDZ->>DDS: Read drag state
SDZ->>VAL: validateForDrop(currentTarget, existingAliases?)
VAL-->>SDZ: isValid
alt Drop valid
SDZ->>SS: addLabel/Description/AliasMapping(languageCode, column)
else Invalid
SDZ-->>U: Ignore drop
end
sequenceDiagram
participant U as User
participant TS as TermSection.vue
participant SDZ as SchemaDropZone.vue
participant SS as schemaStore
U->>TS: Select language
TS-->>SDZ: languageCode (prop)
U->>SDZ: Drop column
SDZ->>SS: Add mapping for termType+language
SS-->>TS: Updated values
U->>TS: Click remove mapping
TS->>SS: removeLabel/Description/AliasMapping(language, mapping)
SS-->>TS: Updated values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/src/features/wikibase-schema/components/TermSection.vue (4)
18-20: Default language: avoid hardcoding 'en'Prefer initializing selectedLanguage from getAcceptedLanguages() (e.g., first available language) to align with project settings/user locale.
Apply:
-const selectedLanguage = ref('en') +const langs = getAcceptedLanguages() +const selectedLanguage = ref(langs?.[0] ?? 'en')Note: If getAcceptedLanguages is invoked later, move this initialization below its declaration or memoize it in setup.
63-73: Removal actions are consistent; minor note on unused paramThe per-type removal flow is correct. For label/description, mapping arg isn’t used; that’s fine since the button is per-language. Optional: overload removeMapping to avoid the unused param for non-alias types.
-const removeMapping = (languageCode: string, mapping: ColumnMapping) => { +const removeMapping = (languageCode: string, mapping?: ColumnMapping) => { if (props.termType === 'label') { schemaStore.removeLabelMapping(languageCode) } else if (props.termType === 'description') { schemaStore.removeDescriptionMapping(languageCode) } else if (props.termType === 'alias') { schemaStore.removeAliasMapping(languageCode, mapping!) } }
109-113: Use stable keys for mapping itemsIndex-based keys can cause unnecessary re-renders when items are added/removed. Prefer a stable key based on mapping identity.
- :key="`${termType}-${displayItem.languageCode}-${index}`" + :key="`${termType}-${displayItem.languageCode}-${mapping.columnName}-${mapping.dataType}`"
115-123: Accessibility: add aria-label to remove buttonsMake the remove action accessible to screen readers.
<Button :data-testid="`remove-${termType}-mapping-${displayItem.languageCode}-${index}`" icon="pi pi-times" rounded text severity="danger" size="small" + :aria-label="`Remove ${termType} mapping ${mapping.columnName} (${mapping.dataType}) for ${displayItem.languageCode}`" @click="removeMapping(displayItem.languageCode, mapping)" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/.eslintrc-auto-import.json(0 hunks)frontend/auto-imports.d.ts(0 hunks)frontend/src/features/data-processing/components/LanguageDropZone.vue(0 hunks)frontend/src/features/data-processing/composables/__tests__/useLanguageDropZone.test.ts(0 hunks)frontend/src/features/data-processing/composables/useLanguageDropZone.ts(0 hunks)frontend/src/features/wikibase-schema/components/SchemaDropZone.vue(1 hunks)frontend/src/features/wikibase-schema/components/TermSection.vue(3 hunks)frontend/src/features/wikibase-schema/components/TermsEditor.vue(4 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useSchemaDropZone.test.ts(0 hunks)frontend/src/features/wikibase-schema/composables/useSchemaDropZone.ts(0 hunks)frontend/src/features/wikibase-schema/stores/schema.store.ts(2 hunks)
💤 Files with no reviewable changes (7)
- frontend/.eslintrc-auto-import.json
- frontend/src/features/data-processing/components/LanguageDropZone.vue
- frontend/src/features/wikibase-schema/composables/useSchemaDropZone.ts
- frontend/src/features/data-processing/composables/tests/useLanguageDropZone.test.ts
- frontend/src/features/data-processing/composables/useLanguageDropZone.ts
- frontend/src/features/wikibase-schema/composables/tests/useSchemaDropZone.test.ts
- frontend/auto-imports.d.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript or JavaScript files
Do not use dotenv; Bun automatically loads .env files
UseBun.serve()for HTTP servers and WebSockets instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofws
PreferBun.fileovernode:fs's readFile/writeFile
UseBun.$(e.g.,Bun.$ls``) instead of execa for running shell commands
Files:
frontend/src/features/wikibase-schema/stores/schema.store.ts
**/*.{html,ts,tsx,css}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuildfor building HTML, TypeScript, or CSS files
Files:
frontend/src/features/wikibase-schema/stores/schema.store.ts
🧬 Code Graph Analysis (1)
frontend/src/features/wikibase-schema/stores/schema.store.ts (2)
backend/src/api/project/project.wikibase.ts (2)
Alias(117-117)StatementSchemaMapping(109-109)frontend/auto-imports.d.ts (3)
Alias(371-371)Ref(359-359)StatementSchemaMapping(371-371)
🔇 Additional comments (10)
frontend/src/features/wikibase-schema/stores/schema.store.ts (2)
102-112: Nice: early return + markDirty only on actual changeThe early return path and calling markDirty() only when a deletion occurs is cleaner and avoids unnecessary reactivity churn.
27-28: Global types are declared in auto-imports.d.ts – no import changes needed
Alias, Ref and StatementSchemaMapping all appear in frontend/auto-imports.d.ts, so your ambient-type imports will resolve correctly.
(Optional) You can drop the explicitRef<StatementSchemaMapping[]>onstatements–ref([])correctly infersRef<StatementSchemaMapping[]>.frontend/src/features/wikibase-schema/components/TermsEditor.vue (2)
1-3: LGTM: local store initsUsing a local schemaStore via useSchemaStore() keeps TermsEditor lean and aligns with the store-driven approach.
15-16: LGTM: binding store-backed values into TermSectionPassing labels, descriptions, and aliases directly from the store matches the new TermSection API and keeps the view reactive to store updates.
Also applies to: 26-27, 37-38
frontend/src/features/wikibase-schema/components/TermSection.vue (3)
40-61: LGTM: normalized mapping view modelComputing a flat display model and normalizing to an array for aliases keeps the template simple and consistent.
136-141: Verify Select option shape matches v-model typeselectedLanguage is a string. Ensure getAcceptedLanguages() returns string[] or configure Select’s optionLabel/optionValue accordingly.
If getAcceptedLanguages returns objects, consider:
- <Select - v-model="selectedLanguage" - :data-testid="`${termType}-language-selector`" - :options="getAcceptedLanguages()" - placeholder="Select" - class="w-full" - /> + <Select + v-model="selectedLanguage" + :data-testid="`${termType}-language-selector`" + :options="getAcceptedLanguages()" + optionLabel="label" + optionValue="value" + placeholder="Select" + class="w-full" + />
9-10: Ambient types confirmed: Label and Alias are auto-importedVerified that frontend/auto-imports.d.ts exports both Label and Alias from '@backend/api/project/project.wikibase', so SFCs—including TermSection.vue—can reference them without adding explicit imports.
frontend/src/features/wikibase-schema/components/SchemaDropZone.vue (3)
32-38: LGTM: duplicate-aware validation gate before writeisValidForDrop includes existingAliases for alias terms and gates writes in handleDrop—good centralization of constraints.
21-23: WikibaseDataType is already available via auto-imports
TheWikibaseDataTypetype is exported infrontend/auto-imports.d.tsfrom@backend/api/project/project.wikibase, making it ambiently available—no explicit import is needed here.
76-90: Ignore missing ColumnInfo importColumnInfo is declared in
src/shared/types/wikibase-schema.tsand re-exported globally viafrontend/auto-imports.d.ts, so it’s already available without an explicit import. No changes are needed.Likely an incorrect or invalid review comment.
| const currentTarget = computed(() => ({ | ||
| type: props.termType, | ||
| path: `item.terms.${props.termType}s.${props.languageCode}`, | ||
| acceptedTypes, | ||
| language: props.languageCode, | ||
| isRequired: false, | ||
| })) | ||
|
|
There was a problem hiding this comment.
Fix incorrect alias path: 'aliass' → 'aliases'
Using ${props.termType}s produces aliass. This breaks validation/styling for alias drops.
Apply:
-const currentTarget = computed(() => ({
- type: props.termType,
- path: `item.terms.${props.termType}s.${props.languageCode}`,
- acceptedTypes,
- language: props.languageCode,
- isRequired: false,
-}))
+const currentTarget = computed(() => {
+ const segment = props.termType === 'alias' ? 'aliases' : `${props.termType}s`
+ return {
+ type: props.termType,
+ path: `item.terms.${segment}.${props.languageCode}`,
+ acceptedTypes,
+ language: props.languageCode,
+ isRequired: false,
+ }
+})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currentTarget = computed(() => ({ | |
| type: props.termType, | |
| path: `item.terms.${props.termType}s.${props.languageCode}`, | |
| acceptedTypes, | |
| language: props.languageCode, | |
| isRequired: false, | |
| })) | |
| const currentTarget = computed(() => { | |
| const segment = props.termType === 'alias' ? 'aliases' : `${props.termType}s` | |
| return { | |
| type: props.termType, | |
| path: `item.terms.${segment}.${props.languageCode}`, | |
| acceptedTypes, | |
| language: props.languageCode, | |
| isRequired: false, | |
| } | |
| }) |
🤖 Prompt for AI Agents
In frontend/src/features/wikibase-schema/components/SchemaDropZone.vue around
lines 24 to 31, the computed currentTarget builds the path using
`${props.termType}s` which yields "aliass" when termType is "alias"; change the
path generation to use the correct plural form (e.g. if props.termType ===
'alias' use 'aliases', otherwise use `${props.termType}s`) so the path becomes
`item.terms.aliases.${props.languageCode}` for aliases and preserves correct
plurals for other term types.
1 participant
No description provided.