refactor(wikibase-schema-editor): extract common components around drop zones #105
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis set of changes introduces a modularized and centralized validation and styling system for drag-and-drop schema editing in the frontend. New composables ( Changes
Sequence Diagram(s)Centralized Validation Flow for Drag-and-DropsequenceDiagram
participant User
participant SchemaDropZone
participant useSchemaDropZone
participant useValidationCore
participant useDropZoneStyling
User->>SchemaDropZone: Drag column over drop zone
SchemaDropZone->>useSchemaDropZone: handleDragOver()
useSchemaDropZone->>useValidationCore: validateForDrop(column, target)
useSchemaDropZone->>useDropZoneStyling: getDropZoneClasses(target)
useSchemaDropZone-->>SchemaDropZone: Update drop zone classes and validation state
SchemaDropZone-->>User: Show visual feedback (valid/invalid)
User->>SchemaDropZone: Drop column
SchemaDropZone->>useSchemaDropZone: handleDrop()
useSchemaDropZone->>useValidationCore: validateForDrop(column, target)
alt Valid
useSchemaDropZone-->>SchemaDropZone: Accept drop, add mapping
else Invalid
useSchemaDropZone-->>SchemaDropZone: Reject drop, show error
end
Validation Feedback DisplaysequenceDiagram
participant ValidationDisplay
participant useValidationErrors
participant useSchemaValidationUI
ValidationDisplay->>useValidationErrors: Get errors, warnings, suggestions
ValidationDisplay->>useSchemaValidationUI: Get current drag feedback
ValidationDisplay-->>User: Render errors, warnings, suggestions, and feedback UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Nitpick comments (7)
frontend/src/stores/schema.store.ts (1)
57-67: LGTM! Consider returning a boolean for better API design.The duplicate prevention logic is well-implemented and consistent with the removal logic. The function correctly checks for existing aliases and only modifies state when necessary.
Consider returning a boolean to indicate whether the alias was actually added:
- const addAliasMapping = (languageCode: string, columnMapping: ColumnMapping) => { + const addAliasMapping = (languageCode: string, columnMapping: ColumnMapping): boolean => { if (!aliases.value[languageCode]) { aliases.value[languageCode] = [] } // Check for duplicates before adding const existingAliases = aliases.value[languageCode] const isDuplicate = existingAliases.some( (alias) => alias.columnName === columnMapping.columnName && alias.dataType === columnMapping.dataType, ) if (!isDuplicate) { aliases.value[languageCode].push(columnMapping) markDirty() + return true } + return false }This would allow calling code to provide user feedback when duplicates are attempted.
frontend/src/composables/useSchemaValidationUI.ts (2)
101-107: Consider improving error handling for invalid target pathsWhen a target is not found, the method returns default classes which might mask configuration errors. Consider either throwing an error or returning an empty array to make invalid paths more apparent during development.
const getDropZoneClassesForPath = (targetPath: string) => { const target = dragDropStore.availableTargets.find((t) => t.path === targetPath) if (!target) { - return ['drop-zone', 'transition-colors', 'border-surface-200'] + console.warn(`[useSchemaValidationUI] Target not found for path: ${targetPath}`) + return [] } return getDropZoneClasses(target) }
154-154: Consider consistent naming for the drop zone classes methodThe exported method is named
getDropZoneClassesin the return object but actually referencesgetDropZoneClassesForPath. This inconsistency might confuse consumers of the API.Either rename the method to match:
- getDropZoneClasses: getDropZoneClassesForPath, + getDropZoneClassesForPath,Or keep the original name and update the implementation:
-const getDropZoneClassesForPath = (targetPath: string) => { +const getDropZoneClasses = (targetPath: string) => {frontend/src/composables/useRealTimeValidation.ts (1)
92-103: Consider simplifying the return type in future iterationsThe method now only returns at most one error but still uses arrays for backward compatibility. While maintaining the API is good, consider documenting this behavior or planning a future migration to a simpler return type that doesn't use arrays when only single errors are possible.
frontend/src/components/ValidationDisplay.vue (1)
190-201: Remove redundant clear methodsThere are two methods that essentially do the same thing. The
clearAllmethod (lines 190-197) has more logic to handle path filtering, whilehandleClearAll(lines 199-201) just callsclearAllValidation. Consider using only one method.-const handleClearAll = () => { - clearAllValidation() -}And update the template to use
clearAllinstead:- @click="handleClearAll" + @click="clearAll"frontend/src/composables/useDropZoneStyling.ts (2)
16-40: Review the styling logic and CSS class consistency.The function provides comprehensive styling based on drag state and validation, but there are a few considerations:
- Potential CSS class inconsistency: Line 31 uses
bg-green-25which may not exist in standard Tailwind CSS (typically uses increments of 50: 50, 100, 150, etc.)- Logic flow is clear: The function correctly handles different states (no drag, valid/invalid targets, hovered states)
- Tailwind usage: Properly uses Tailwind utility classes as required by coding guidelines
Verify that
bg-green-25is available in your Tailwind configuration:- return [...baseClasses, 'border-green-300', 'bg-green-25', 'border-dashed'] + return [...baseClasses, 'border-green-300', 'bg-green-50', 'border-dashed']If
bg-green-25is intentionally configured in your Tailwind setup, this change is not needed.
45-61: Class object function logic is sound but has redundancy.The function correctly provides Vue class binding objects, but there's a minor optimization opportunity and potential consistency issue:
- Good separation of concerns: Different styling approach for when no column is dragged vs when dragging
- Redundancy: Lines 49-50 always set these classes to
falsewhen not dragging, which is unnecessary- Consistent validation: Properly uses
validateForStylingfor consistencySimplify the class object when not dragging:
const getDropZoneClassObject = (target: DropTarget, isOverDropZone = false) => { if (!dragDropStore.draggedColumn) { return { 'border-primary-400 bg-primary-50': isOverDropZone, - 'border-green-400 bg-green-50': false, - 'border-red-400 bg-red-50': false, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
frontend/.eslintrc-auto-import.json(2 hunks)frontend/auto-imports.d.ts(4 hunks)frontend/components.d.ts(1 hunks)frontend/src/components/SchemaDropZone.vue(3 hunks)frontend/src/components/StatementEditor.vue(3 hunks)frontend/src/components/TermSection.vue(1 hunks)frontend/src/components/TermsEditor.vue(1 hunks)frontend/src/components/ValidationDisplay.vue(1 hunks)frontend/src/components/ValidationErrorDisplay.vue(0 hunks)frontend/src/components/WikibaseSchemaEditor.vue(5 hunks)frontend/src/composables/__tests__/useDataTypeCompatibility.test.ts(1 hunks)frontend/src/composables/__tests__/useReferenceValueMapping.test.ts(1 hunks)frontend/src/composables/__tests__/useSchemaDropZone.test.ts(4 hunks)frontend/src/composables/__tests__/useSchemaValidationUI.test.ts(3 hunks)frontend/src/composables/__tests__/useValidationCore.test.ts(1 hunks)frontend/src/composables/useDropZoneStyling.ts(1 hunks)frontend/src/composables/useRealTimeValidation.ts(3 hunks)frontend/src/composables/useSchemaDropZone.ts(4 hunks)frontend/src/composables/useSchemaValidationUI.ts(5 hunks)frontend/src/composables/useValidationCore.ts(1 hunks)frontend/src/stores/schema.store.ts(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/components/ValidationErrorDisplay.vue
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useReferenceValueMapping.test.tsfrontend/src/composables/__tests__/useSchemaValidationUI.test.tsfrontend/src/stores/schema.store.tsfrontend/src/composables/useValidationCore.tsfrontend/components.d.tsfrontend/src/composables/__tests__/useDataTypeCompatibility.test.tsfrontend/src/composables/useSchemaDropZone.tsfrontend/src/composables/__tests__/useValidationCore.test.tsfrontend/auto-imports.d.tsfrontend/src/composables/useSchemaValidationUI.tsfrontend/src/composables/useDropZoneStyling.tsfrontend/src/composables/useRealTimeValidation.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun testinstead ofjestfor running tests
Files:
frontend/src/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useReferenceValueMapping.test.tsfrontend/src/composables/__tests__/useSchemaValidationUI.test.tsfrontend/src/composables/__tests__/useDataTypeCompatibility.test.tsfrontend/src/composables/__tests__/useValidationCore.test.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/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useReferenceValueMapping.test.tsfrontend/src/composables/__tests__/useSchemaValidationUI.test.tsfrontend/src/stores/schema.store.tsfrontend/src/composables/useValidationCore.tsfrontend/components.d.tsfrontend/src/composables/__tests__/useDataTypeCompatibility.test.tsfrontend/src/composables/useSchemaDropZone.tsfrontend/src/composables/__tests__/useValidationCore.test.tsfrontend/auto-imports.d.tsfrontend/src/composables/useSchemaValidationUI.tsfrontend/src/composables/useDropZoneStyling.tsfrontend/src/composables/useRealTimeValidation.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Use composables for logic that is not global state
📚 Learning: applies to src/composables/**/*.ts : use composables for logic that is not global state...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Use composables for logic that is not global state
Applied to files:
frontend/src/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/useValidationCore.tsfrontend/components.d.tsfrontend/src/components/WikibaseSchemaEditor.vuefrontend/src/composables/useSchemaDropZone.tsfrontend/src/composables/__tests__/useValidationCore.test.tsfrontend/src/components/ValidationDisplay.vuefrontend/auto-imports.d.tsfrontend/src/composables/useSchemaValidationUI.tsfrontend/src/composables/useDropZoneStyling.tsfrontend/src/composables/useRealTimeValidation.ts
📚 Learning: applies to src/**/*.vue : use auto-imports for vue, pinia, composables, and utilities...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use auto-imports for Vue, Pinia, composables, and utilities
Applied to files:
frontend/.eslintrc-auto-import.jsonfrontend/components.d.tsfrontend/auto-imports.d.ts
📚 Learning: applies to **/*.{tsx,jsx,js} : import .css files directly in .tsx, .jsx, or .js files and bun will h...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{tsx,jsx,js} : Import .css files directly in .tsx, .jsx, or .js files and Bun will handle them
Applied to files:
frontend/.eslintrc-auto-import.jsonfrontend/auto-imports.d.ts
📚 Learning: applies to **/*.{html,tsx,jsx,js,css} : use html imports with `bun.serve()` instead of `vite` for fr...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{html,tsx,jsx,js,css} : Use HTML imports with `Bun.serve()` instead of `vite` for frontend development
Applied to files:
frontend/.eslintrc-auto-import.json
📚 Learning: applies to src/**/*.vue : use reactive objects for form state and errors...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use reactive objects for form state and errors
Applied to files:
frontend/.eslintrc-auto-import.jsonfrontend/src/components/WikibaseSchemaEditor.vuefrontend/src/components/ValidationDisplay.vuefrontend/auto-imports.d.tsfrontend/src/components/SchemaDropZone.vuefrontend/src/composables/useRealTimeValidation.ts
📚 Learning: applies to src/**/*.{vue,ts} : use useapi composable (elysia eden) for all api calls...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Use useApi composable (Elysia Eden) for all API calls
Applied to files:
frontend/src/composables/useValidationCore.tsfrontend/components.d.tsfrontend/src/composables/useSchemaDropZone.tsfrontend/src/composables/__tests__/useValidationCore.test.tsfrontend/auto-imports.d.tsfrontend/src/composables/useSchemaValidationUI.tsfrontend/src/composables/useDropZoneStyling.ts
📚 Learning: applies to src/**/*.vue : build reusable, well-structured components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Build reusable, well-structured components
Applied to files:
frontend/src/components/TermsEditor.vuefrontend/components.d.tsfrontend/src/components/TermSection.vue
📚 Learning: applies to src/**/*.vue : use <script setup lang="ts"> at the top, template second, style last (rare...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use <script setup lang="ts"> at the top, template second, style last (rarely used) in Vue components
Applied to files:
frontend/src/components/TermsEditor.vuefrontend/components.d.tsfrontend/src/components/TermSection.vue
📚 Learning: applies to src/**/*.vue : use v-memo, shallowref, markraw, and suspense for performance optimization...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use v-memo, shallowRef, markRaw, and Suspense for performance optimization
Applied to files:
frontend/src/components/TermsEditor.vuefrontend/components.d.tsfrontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.vue : prefer composables over methods in vue components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Prefer composables over methods in Vue components
Applied to files:
frontend/src/components/TermsEditor.vuefrontend/components.d.tsfrontend/src/components/WikibaseSchemaEditor.vuefrontend/src/components/ValidationDisplay.vuefrontend/auto-imports.d.tsfrontend/src/composables/useSchemaValidationUI.tsfrontend/src/composables/useDropZoneStyling.ts
📚 Learning: applies to src/**/*.vue : use vue 3 with composition api and <script setup lang="ts"> in all vue com...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use Vue 3 with Composition API and <script setup lang="ts"> in all Vue components
Applied to files:
frontend/src/components/TermsEditor.vuefrontend/components.d.tsfrontend/src/components/TermSection.vuefrontend/src/composables/useSchemaDropZone.tsfrontend/src/composables/__tests__/useValidationCore.test.tsfrontend/src/components/ValidationDisplay.vuefrontend/auto-imports.d.tsfrontend/src/composables/useDropZoneStyling.ts
📚 Learning: applies to src/**/*.{ts,vue} : type safety everywhere...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{ts,vue} : Type safety everywhere
Applied to files:
frontend/components.d.tsfrontend/src/components/TermSection.vuefrontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.vue : props and emits must use explicit typescript interfaces...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Props and emits must use explicit TypeScript interfaces
Applied to files:
frontend/components.d.tsfrontend/src/components/TermSection.vuefrontend/src/components/ValidationDisplay.vue
📚 Learning: applies to src/**/*.{vue,ts} : always use backend-inferred types for api data...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Always use backend-inferred types for API data
Applied to files:
frontend/components.d.tsfrontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.vue : use primevue as the ui library in all vue components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use PrimeVue as the UI library in all Vue components
Applied to files:
frontend/components.d.tsfrontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.{vue,ts} : use pinia stores for global state...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Use Pinia stores for global state
Applied to files:
frontend/components.d.tsfrontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.{vue,ts} : use readonly and shallowreactive for large or expensive data...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Use readonly and shallowReactive for large or expensive data
Applied to files:
frontend/components.d.tsfrontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.vue : validate on client and handle server errors for forms...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Validate on client and handle server errors for forms
Applied to files:
frontend/src/components/WikibaseSchemaEditor.vuefrontend/src/components/ValidationDisplay.vuefrontend/src/composables/useRealTimeValidation.ts
📚 Learning: applies to src/composables/**/*.ts : place composables in the composables/ directory...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Place composables in the composables/ directory
Applied to files:
frontend/src/composables/useSchemaDropZone.tsfrontend/src/composables/__tests__/useValidationCore.test.tsfrontend/auto-imports.d.tsfrontend/src/composables/useDropZoneStyling.ts
📚 Learning: applies to src/composables/**/*.ts : do not export store state from composables...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Do not export store state from composables
Applied to files:
frontend/src/composables/useSchemaDropZone.tsfrontend/auto-imports.d.tsfrontend/src/composables/useDropZoneStyling.tsfrontend/src/composables/useRealTimeValidation.ts
📚 Learning: applies to **/*.test.{ts,tsx,js,jsx} : use `bun test` instead of `jest` for running tests...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `bun test` instead of `jest` for running tests
Applied to files:
frontend/src/composables/__tests__/useValidationCore.test.ts
📚 Learning: for more information, read the official bun documentation at https://bun.sh/docs or check `node_modu...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: For more information, read the official Bun documentation at https://bun.sh/docs or check `node_modules/bun-types/docs/**.md` if available locally
Applied to files:
frontend/src/composables/__tests__/useValidationCore.test.ts
📚 Learning: applies to src/**/*.{vue,ts} : handle errors and loading states reactively when making api calls...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Handle errors and loading states reactively when making API calls
Applied to files:
frontend/src/components/ValidationDisplay.vuefrontend/src/composables/useRealTimeValidation.ts
📚 Learning: applies to src/**/*.vue : use storetorefs for state in components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use storeToRefs for state in components
Applied to files:
frontend/auto-imports.d.ts
📚 Learning: applies to src/**/*.vue : use tailwind css utility classes for all styling; tailwind is mandatory fo...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use Tailwind CSS utility classes for all styling; Tailwind is mandatory for all styling
Applied to files:
frontend/auto-imports.d.tsfrontend/src/composables/useDropZoneStyling.ts
📚 Learning: applies to src/composables/**/*.ts : do not proxy or export pinia store state/actions from composabl...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Do not proxy or export Pinia store state/actions from composables
Applied to files:
frontend/auto-imports.d.ts
🧬 Code Graph Analysis (8)
frontend/src/composables/__tests__/useSchemaDropZone.test.ts (1)
frontend/src/stores/drag-drop.store.ts (1)
useDragDropStore(10-174)
frontend/src/composables/__tests__/useReferenceValueMapping.test.ts (1)
frontend/src/types/wikibase-schema.ts (1)
ColumnInfo(169-175)
frontend/src/composables/useValidationCore.ts (2)
frontend/src/composables/useDataTypeCompatibility.ts (1)
useDataTypeCompatibility(8-44)frontend/auto-imports.d.ts (2)
ColumnInfo(384-384)DropTarget(378-378)
frontend/src/composables/__tests__/useDataTypeCompatibility.test.ts (1)
frontend/auto-imports.d.ts (1)
ColumnInfo(384-384)
frontend/src/composables/__tests__/useValidationCore.test.ts (2)
frontend/src/composables/useValidationCore.ts (1)
useValidationCore(14-121)frontend/auto-imports.d.ts (2)
ColumnInfo(384-384)DropTarget(378-378)
frontend/auto-imports.d.ts (2)
frontend/src/composables/useDropZoneStyling.ts (1)
useDropZoneStyling(9-85)frontend/src/composables/useValidationCore.ts (1)
useValidationCore(14-121)
frontend/src/composables/useDropZoneStyling.ts (3)
frontend/src/stores/drag-drop.store.ts (1)
useDragDropStore(10-174)frontend/src/composables/useValidationCore.ts (1)
useValidationCore(14-121)frontend/auto-imports.d.ts (1)
DropTarget(378-378)
frontend/src/composables/useRealTimeValidation.ts (2)
frontend/src/composables/useValidationErrors.ts (1)
useValidationErrors(11-87)frontend/src/composables/useValidationCore.ts (1)
useValidationCore(14-121)
🪛 GitHub Actions: Typecheck code
frontend/src/components/ValidationDisplay.vue
[error] 217-217: TypeScript error TS2551: Property 'hasErrors' does not exist on the component instance. Did you mean 'errors'?
[error] 221-221: TypeScript error TS2551: Property 'hasWarnings' does not exist on the component instance. Did you mean 'warnings'?
[error] 226-226: TypeScript error TS2551: Property 'hasWarnings' does not exist on the component instance. Did you mean 'warnings'?
🔇 Additional comments (34)
frontend/src/composables/__tests__/useReferenceValueMapping.test.ts (1)
43-50: LGTM! Consistent variable naming.The renaming from
columntocolumnInfoimproves code clarity and aligns with naming conventions used across other test files in this PR.frontend/src/composables/__tests__/useDataTypeCompatibility.test.ts (1)
15-132: LGTM! Systematic improvement in variable naming.The consistent renaming from
columntocolumnInfoacross all test cases enhances code readability and better reflects theColumnInfotype being used. The test logic and functionality remain unchanged.frontend/src/composables/__tests__/useSchemaDropZone.test.ts (2)
277-293: LGTM! Proper test setup for drag-drop flow.Adding explicit
dragDropStore.startDrag(columnData)calls before testing drop events ensures the drag state is properly initialized, making the tests more realistic and aligned with actual user interactions.
317-331: LGTM! Consistent test setup pattern.The same proper drag state initialization is applied to the invalid column test case, maintaining consistency in test setup patterns.
frontend/src/composables/__tests__/useSchemaValidationUI.test.ts (1)
212-275: LGTM! API simplification reflects good refactoring.The updated
getDropZoneClassescalls now only require the path parameter, reflecting the simplified API after centralizing styling logic into the newuseDropZoneStylingcomposable. This reduces coupling and makes the interface cleaner.frontend/.eslintrc-auto-import.json (2)
250-250: LGTM! Auto-import configuration for new composable.Adding
useDropZoneStylingto the auto-import globals enables seamless usage of the new drop zone styling composable throughout the frontend without explicit imports.
382-382: LGTM! Auto-import configuration for validation composable.Adding
useValidationCoreto the auto-import globals enables seamless usage of the new validation core composable throughout the frontend, supporting the centralized validation logic refactoring.frontend/src/components/TermsEditor.vue (1)
6-33: Excellent refactoring to eliminate duplication!The replacement of individual section blocks with the reusable
TermSectioncomponent significantly improves maintainability and consistency. The validation-path props are correctly structured to enable nested validation feedback.frontend/components.d.ts (1)
52-59: LGTM! Proper TypeScript support for new components.The addition of global component declarations for the new validation and term section components ensures proper TypeScript support and auto-completion throughout the application.
frontend/src/components/SchemaDropZone.vue (2)
89-100: LGTM! Clean validation feedback implementation.The conditional validation feedback with appropriate styling based on feedback type provides good user experience during drag operations.
41-52: Fix class binding approach for proper Vue compatibility.The validation integration is well-designed, but the class combination approach may not work correctly with Vue's class binding system.
Vue's class binding expects either an object, array of strings, or array of objects. The current approach mixes formats incorrectly:
- // Computed drop zone classes that combine both systems - const enhancedDropZoneClasses = computed(() => { - const targetPath = `item.terms.${props.termType}s.${props.languageCode}` - const validationClasses = getDropZoneClasses(targetPath) - - // dropZoneClasses returns an object for Vue class binding - // validationClasses returns an array of class names - // We need to combine them properly - return [ - dropZoneClasses.value, // Object for Vue class binding - ...validationClasses, // Array of class strings - ] - }) + // Computed drop zone classes that combine both systems + const enhancedDropZoneClasses = computed(() => { + const targetPath = `item.terms.${props.termType}s.${props.languageCode}` + const validationClasses = getDropZoneClasses(targetPath) + + // Combine object classes with validation class strings + return { + ...dropZoneClasses.value, // Spread object classes + [validationClasses.join(' ')]: validationClasses.length > 0 // Add validation classes as a single key + } + })Or use array format consistently:
+ const enhancedDropZoneClasses = computed(() => { + const targetPath = `item.terms.${props.termType}s.${props.languageCode}` + const validationClasses = getDropZoneClasses(targetPath) + + return [dropZoneClasses.value, ...validationClasses] + })⛔ Skipped due to learnings
Learnt from: CR PR: DaxServer/dataforge#0 File: .cursor/rules/frontend-setup.rule.md:0-0 Timestamp: 2025-07-20T14:13:24.706Z Learning: Applies to src/**/*.vue : Use reactive objects for form state and errorsfrontend/src/components/StatementEditor.vue (2)
398-414: LGTM! Well-integrated validation feedback.The vertical layout adjustment and conditional validation feedback enhance the user experience during drag operations. The styling is consistent with other components in the refactor.
358-358: No changes needed for class binding.Vue’s
:classbinding fully supports arrays containing both objects and nested arrays. Here,dropZoneClasses(a computed ref returning an object) andgetDropZoneClasses()(returning a string array) will be correctly normalized and flattened at runtime.frontend/src/components/TermSection.vue (2)
1-34: LGTM! Well-structured component setup.The component follows Vue 3 Composition API best practices with explicit TypeScript interfaces for props, proper composable usage, and clean reactive state management. The validation logic integration is well-implemented.
36-55: LGTM! Clean template implementation.The template structure is well-organized with proper conditional rendering, dynamic class binding, and correct prop passing to the child component. The use of Tailwind CSS classes aligns with the styling guidelines.
frontend/auto-imports.d.ts (1)
191-191: LGTM! Necessary auto-import declarations added.The additions of
useDropZoneStylinganduseValidationCoreto the auto-import declarations are essential for the refactoring effort to centralize validation and styling logic. The declarations follow the established pattern and maintain consistency with existing entries.Also applies to: 323-323, 574-574, 706-706
frontend/src/components/WikibaseSchemaEditor.vue (4)
23-32: LGTM! Clean composable integration.The integration of
useSchemaValidationUIfollows proper destructuring patterns and provides clear access to validation state and control functions.
96-98: LGTM! Proper lifecycle management.The real-time validation is appropriately enabled on mount and cleaned up on unmount, following good Vue lifecycle practices and preventing potential memory leaks.
Also applies to: 338-341
365-385: LGTM! Excellent validation status indicator.The validation status indicator provides clear visual feedback with appropriate color coding, icons, and conditional rendering. The implementation follows Tailwind CSS patterns and provides good user experience.
427-442: LGTM! Comprehensive validation display integration.The integration of
ValidationDisplaycomponents in both status and full modes provides users with appropriate levels of validation feedback. The conditional rendering ensures the UI remains clean when no issues exist.frontend/src/composables/__tests__/useValidationCore.test.ts (4)
1-9: LGTM! Proper test setup following guidelines.The test setup correctly uses
bun:testas required by the coding guidelines and properly imports the necessary composable and types.
10-149: LGTM! Comprehensive core validation test coverage.The test suite thoroughly covers all validation scenarios including data type compatibility, nullable constraints, length limits, and property requirements. Each test properly validates both success and failure cases with appropriate assertions.
151-202: LGTM! Thorough alias deduplication testing.The alias deduplication tests properly verify duplicate detection based on both column name and data type matching, while ensuring no false positives occur. This provides confidence in the alias validation logic.
204-288: LGTM! Critical distinction between styling and drop validation.These tests properly verify the important behavioral difference between
validateForStyling(which ignores duplicates for UI feedback) andvalidateForDrop(which enforces duplicate checking). This distinction is crucial for the correct operation of the drag-and-drop UI.frontend/src/composables/useSchemaDropZone.ts (4)
6-24: LGTM! Clean integration of shared composables.The integration of
useValidationCoreanduseDropZoneStylingsuccessfully centralizes validation and styling logic, reducing code duplication and improving maintainability.
32-46: LGTM! Simplified validation logic using shared composable.The validation logic has been successfully refactored to use the shared
validateForDropfunction while maintaining proper duplicate checking for aliases. The reactive target computation is clean and efficient.
60-89: LGTM! Consistent validation in event handlers.The event handlers now consistently use the centralized
isValidForDropvalidation state, ensuring proper drop behavior and user feedback throughout the drag-and-drop operation.
108-122: LGTM! Clean API additions for configuration.The addition of
setTermTypeandsetLanguageCodesetter methods provides a clean, explicit API for configuring the drop zone behavior, improving the composable's usability and maintainability.frontend/src/composables/useSchemaValidationUI.ts (1)
97-99: Good refactoring to use centralized validation logicThe delegation to
validateColumnForTargetfrom the core validation composable improves code maintainability and ensures consistent validation across the application.frontend/src/composables/useValidationCore.ts (1)
82-113: Excellent separation of validation concernsThe distinction between
validateForStyling(which allows duplicate aliases to show as valid targets) andvalidateForDrop(which prevents actual duplicate drops) provides good UX. Users can see that a column is compatible with the alias target type even if it would be a duplicate, but are prevented from creating the duplicate when dropping.frontend/src/composables/useDropZoneStyling.ts (4)
1-5: LGTM! Clean imports and proper typing.The imports are well-organized and follow the established patterns. The type import for
DropTargetensures type safety throughout the composable.
6-11: Good composable structure following established patterns.The composable correctly uses other composables (
useDragDropStoreanduseValidationCore) and follows the learning that composables should be used for logic that is not global state.
66-78: Computed property logic is correct and efficient.The computed property efficiently determines drag validity for styling purposes:
- Proper reactive dependencies: Uses store reactive properties that will trigger updates
- Safe navigation: Checks for existence of required properties before proceeding
- Consistent validation: Uses the same
validateForStylingfunction as other methods
80-85: Clean composable return structure.The return object properly exposes the three main functions with clear, descriptive names that indicate their purposes.
| const getErrorCodeFromReason = (reason: string) => { | ||
| switch (reason) { | ||
| case 'incompatible_data_type': | ||
| return 'INCOMPATIBLE_DATA_TYPE' | ||
| case 'nullable_required_field': | ||
| return 'MISSING_REQUIRED_MAPPING' | ||
| case 'length_constraint': | ||
| return 'INCOMPATIBLE_DATA_TYPE' | ||
| case 'missing_property_id': | ||
| return 'INVALID_PROPERTY_ID' | ||
| default: | ||
| return 'INCOMPATIBLE_DATA_TYPE' | ||
| } | ||
|
|
||
| return errors | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review error code mappings for semantic accuracy
Some mappings may be misleading:
length_constraint→INCOMPATIBLE_DATA_TYPE: Length constraints are not about data type compatibilitynullable_required_field→MISSING_REQUIRED_MAPPING: These have different semantic meanings
Consider adding new error codes or adjusting the mappings to better represent the actual validation failures.
const getErrorCodeFromReason = (reason: string) => {
switch (reason) {
case 'incompatible_data_type':
return 'INCOMPATIBLE_DATA_TYPE'
case 'nullable_required_field':
- return 'MISSING_REQUIRED_MAPPING'
+ return 'NULLABLE_REQUIRED_FIELD'
case 'length_constraint':
- return 'INCOMPATIBLE_DATA_TYPE'
+ return 'LENGTH_CONSTRAINT_VIOLATION'
case 'missing_property_id':
return 'INVALID_PROPERTY_ID'
default:
return 'INCOMPATIBLE_DATA_TYPE'
}
}Note: This would require adding the new error codes to the validation error types.
📝 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 getErrorCodeFromReason = (reason: string) => { | |
| switch (reason) { | |
| case 'incompatible_data_type': | |
| return 'INCOMPATIBLE_DATA_TYPE' | |
| case 'nullable_required_field': | |
| return 'MISSING_REQUIRED_MAPPING' | |
| case 'length_constraint': | |
| return 'INCOMPATIBLE_DATA_TYPE' | |
| case 'missing_property_id': | |
| return 'INVALID_PROPERTY_ID' | |
| default: | |
| return 'INCOMPATIBLE_DATA_TYPE' | |
| } | |
| return errors | |
| } | |
| const getErrorCodeFromReason = (reason: string) => { | |
| switch (reason) { | |
| case 'incompatible_data_type': | |
| return 'INCOMPATIBLE_DATA_TYPE' | |
| case 'nullable_required_field': | |
| return 'NULLABLE_REQUIRED_FIELD' | |
| case 'length_constraint': | |
| return 'LENGTH_CONSTRAINT_VIOLATION' | |
| case 'missing_property_id': | |
| return 'INVALID_PROPERTY_ID' | |
| default: | |
| return 'INCOMPATIBLE_DATA_TYPE' | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/src/composables/useRealTimeValidation.ts around lines 109 to 122,
the error code mappings for 'length_constraint' and 'nullable_required_field'
are semantically inaccurate. Update the mapping to assign 'length_constraint' a
new or more appropriate error code reflecting length validation issues, and
assign 'nullable_required_field' a distinct error code that clearly indicates a
missing required field rather than a missing mapping. Add these new error codes
to the validation error types to ensure consistency and clarity in error
handling.
1 participant
No description provided.