Skip to content

fix(wikibase-schema-editor): bugs saving qualifiers and references #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaxServer merged 2 commits into from
Aug 8, 2025
Merged

fix(wikibase-schema-editor): bugs saving qualifiers and references #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a83e2cc
fix(wikibase-schema-editor): bugs saving qualifiers and references
DaxServer Aug 8, 2025
5f8e717
fix: type
DaxServer Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 16 additions & 17 deletions frontend/src/features/wikibase-schema/components/WikibaseSchemaEditor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const currentStatementWithQualifiers = ref<{
value: ValueMapping
rank: StatementRank
qualifiers?: PropertyValueMap[]
references?: ReferenceSchemaMapping[]
}>({
property: null,
value: {
Expand All @@ -62,6 +63,7 @@ const currentStatementWithQualifiers = ref<{
},
rank: 'normal',
qualifiers: [],
references: [],
})

// Computed properties
Expand Down Expand Up @@ -231,6 +233,7 @@ const handleAddStatement = () => {
},
rank: 'normal',
qualifiers: [],
references: [],
}
editingStatementId.value = null
isAddingStatement.value = true
Expand All @@ -239,12 +242,13 @@ const handleAddStatement = () => {
const handleEditStatement = (statementId: UUID) => {
const statement = schemaStore.statements.find((s) => s.id === statementId)
if (statement) {
// Load existing statement data for editing including qualifiers
// Load existing statement data for editing including qualifiers and references
currentStatementWithQualifiers.value = {
property: statement.property,
value: { ...statement.value },
rank: statement.rank,
qualifiers: [...(statement.qualifiers || [])],
references: [...(statement.references || [])],
}
initializeStatement({
property: statement.property,
Expand All @@ -271,6 +275,7 @@ const handleStatementUpdate = (statement: {
value: ValueMapping
rank: StatementRank
qualifiers?: PropertyValueMap[]
references?: ReferenceSchemaMapping[]
}) => {
currentStatementWithQualifiers.value = statement
// Also update the composable state for backward compatibility
Expand All @@ -286,30 +291,23 @@ const handleStatementSave = () => {
if (!currentStatement?.property) return

if (editingStatementId.value) {
// Update existing statement
const statementIndex = schemaStore.statements.findIndex(
(s) => s.id === editingStatementId.value,
// Update existing statement using the proper store method
schemaStore.updateStatement(
editingStatementId.value as UUID,
currentStatement.property,
currentStatement.value,
currentStatement.rank,
currentStatement.qualifiers || [],
currentStatement.references || [],
)
if (statementIndex !== -1) {
const existingStatement = schemaStore.statements[statementIndex]
if (existingStatement) {
schemaStore.statements[statementIndex] = {
id: existingStatement.id,
property: currentStatement.property,
value: currentStatement.value,
rank: currentStatement.rank,
qualifiers: currentStatement.qualifiers || [],
references: existingStatement.references,
}
}
}
} else {
// Add new statement
schemaStore.addStatement(
currentStatement.property,
currentStatement.value,
currentStatement.rank,
currentStatement.qualifiers || [],
currentStatement.references || [],
)
}

Expand All @@ -332,6 +330,7 @@ const handleCancelStatementEdit = () => {
},
rank: 'normal',
qualifiers: [],
references: [],
}
resetStatement()
}
Expand Down
6 changes: 1 addition & 5 deletions frontend/src/features/wikibase-schema/composables/__tests__/useSchemaApi.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,7 @@ describe('useSchemaApi', () => {
await updateSchema(TEST_PROJECT_ID, TEST_SCHEMA_ID, updatedData)

expect(mockApi.project).toHaveBeenCalledWith({ projectId: TEST_PROJECT_ID })
expect(mockSchemaIdPut).toHaveBeenCalledWith({
name: updatedData.name,
wikibase: updatedData.wikibase,
schema: updatedData.schema,
})
expect(mockSchemaIdPut).toHaveBeenCalledWith(updatedData)
expect(store.schemaId).toBe(mockUpdatedSchema.id as UUID)
expect(store.projectId).toBe(mockUpdatedSchema.project_id as UUID)
expect(store.schemaName).toBe(mockUpdatedSchema.name)
Expand Down
9 changes: 4 additions & 5 deletions frontend/src/features/wikibase-schema/composables/useSchemaApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,10 @@ export const useSchemaApi = () => {

const updateSchema = async (projectId: UUID, schemaId: UUID, schemaData: SchemaRequest) => {
return withLoadingState(async () => {
const { data, error: apiError } = await api.project({ projectId }).schemas({ schemaId }).put({
name: schemaData.name,
wikibase: schemaData.wikibase,
schema: schemaData.schema,
})
const { data, error: apiError } = await api
.project({ projectId })
.schemas({ schemaId })
.put(schemaData)

if (apiError) {
showError(apiError.value as ApiError)
Expand Down
6 changes: 1 addition & 5 deletions frontend/src/features/wikibase-schema/composables/useSchemaPersistence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,16 @@ export const useSchemaPersistence = () => {
if (schemaStore.schemaId) {
// Update existing schema
const schemaData = {
id: schemaStore.schemaId,
projectId: schemaStore.projectId,
name: schemaStore.schemaName,
wikibase: schemaStore.wikibase,
item: {
schema: {
terms: {
labels: schemaStore.labels,
descriptions: schemaStore.descriptions,
aliases: schemaStore.aliases,
},
statements: schemaStore.statements,
},
createdAt: schemaStore.createdAt,
updatedAt: schemaStore.updatedAt,
}

result = await updateSchema(schemaStore.projectId, schemaStore.schemaId, schemaData)
Expand Down
223 changes: 222 additions & 1 deletion frontend/src/features/wikibase-schema/stores/__tests__/schema.store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ const TEST_STATEMENT_ID: UUID = Bun.randomUUIDv7() as UUID

// Mock the useSchemaBuilder composable
const mockBuildStatement = mock(
(property: PropertyReference, valueMapping: ValueMapping, rank = 'normal') => ({
(
property: PropertyReference,
valueMapping: ValueMapping,
rank = 'normal',
qualifiers: PropertyValueMap[] = [],
references: ReferenceSchemaMapping[] = [],
) => ({
id: TEST_STATEMENT_ID,
property,
value: valueMapping,
rank,
qualifiers,
references,
}),
Comment on lines +17 to 30
Copy link

@coderabbitai coderabbitai bot Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Missing type imports for PropertyValueMap and ReferenceSchemaMapping (TS compile fix)

Both PropertyValueMap and ReferenceSchemaMapping are referenced in this new mock signature and return value but are not imported in this file, which will cause TypeScript errors.

Proposed import (outside this range; adjust the source module if different in your repo):

// At the top near other type imports
import type {
  ColumnMapping,
  PropertyReference,
  ValueMapping,
  PropertyValueMap,
  ReferenceSchemaMapping,
} from '@backend/api/project/project.wikibase'

If these types live elsewhere, run this to locate their definitions and correct the import:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where these types are exported
rg -n "export (type|interface) ReferenceSchemaMapping" -A 3
rg -n "export (type|interface) PropertyValueMap" -A 3

# Inspect project.wikibase exports for related types
fd -a "project.wikibase" | xargs -r rg -n "export (type|interface) (ReferenceSchemaMapping|PropertyValueMap|PropertyReference|ValueMapping|ColumnMapping)" -A 2

Length of output: 2410

Add missing type imports for qualifiers and references

The test mock now references PropertyValueMap and ReferenceSchemaMapping but they aren’t imported, causing TS errors. Please update the top of frontend/src/features/wikibase-schema/stores/tests/schema.store.test.ts to include:

import type {
  ColumnMapping,
  PropertyReference,
  ValueMapping,
  PropertyValueMap,
  ReferenceSchemaMapping,
} from '@backend/api/project/project.wikibase'

• Verify this path matches your tsconfig “@backend/api” alias.
• (Optional) Strengthen coverage by:
– Asserting the builder receives references when adding a statement.
– Adding a symmetric test for clearing qualifiers (like you have for references).
– On non-existent updates, asserting that the mockBuildStatement isn’t called.

🤖 Prompt for AI Agents 
In frontend/src/features/wikibase-schema/stores/__tests__/schema.store.test.ts
around lines 17 to 30, the types PropertyValueMap and ReferenceSchemaMapping
used in the test mock are not imported, causing TypeScript errors. Fix this by
adding an import statement at the top of the file importing these types along
with ColumnMapping, PropertyReference, and ValueMapping from
'@backend/api/project/project.wikibase'. Also verify the import path matches the
tsconfig alias. Optionally, improve test coverage by adding assertions for
references handling, clearing qualifiers, and ensuring mockBuildStatement is not
called on non-existent updates.

)

Expand Down Expand Up @@ -681,6 +689,219 @@ describe('useSchemaStore', () => {

expect(store.statements[0]?.rank).toBe('normal')
})

it('should update existing statement with qualifiers', () => {
const mockProperty: PropertyReference = {
id: 'P123',
label: 'Test Property',
dataType: 'string',
}
const mockValue: ValueMapping = {
type: 'column',
source: { columnName: 'test_column', dataType: 'VARCHAR' },
dataType: 'string',
}
const mockQualifiers: PropertyValueMap[] = [
{
property: { id: 'P456', label: 'Qualifier', dataType: 'string' },
value: { type: 'constant', source: 'test value', dataType: 'string' },
},
]

// Add initial statement
const statementId = store.addStatement(mockProperty, mockValue)
expect(store.isDirty).toBe(true)

// Mark as saved to test dirty state
store.markAsSaved()
expect(store.isDirty).toBe(false)

// Update statement with qualifiers
const updatedProperty: PropertyReference = {
id: 'P789',
label: 'Updated Property',
dataType: 'wikibase-item',
}
const updatedValue: ValueMapping = {
type: 'constant',
source: 'updated value',
dataType: 'wikibase-item',
}

store.updateStatement(statementId, updatedProperty, updatedValue, 'preferred', mockQualifiers)

// Verify the statement was updated
const updatedStatement = store.statements.find((s) => s.id === statementId)
expect(updatedStatement).toBeDefined()
expect(updatedStatement?.property.id).toBe('P789')
expect(updatedStatement?.property.label).toBe('Updated Property')
expect(updatedStatement?.value.type).toBe('constant')
expect(updatedStatement?.value.source).toBe('updated value')
expect(updatedStatement?.rank).toBe('preferred')
expect(updatedStatement?.qualifiers).toHaveLength(1)
expect(updatedStatement!.qualifiers[0]!.property.id).toBe('P456')
expect(updatedStatement!.qualifiers[0]!.value.source).toBe('test value')

// Verify store is marked as dirty
expect(store.isDirty).toBe(true)
})

it('should handle updating non-existent statement gracefully', () => {
const mockProperty: PropertyReference = {
id: 'P123',
label: 'Test Property',
dataType: 'string',
}
const mockValue: ValueMapping = {
type: 'column',
source: { columnName: 'test_column', dataType: 'VARCHAR' },
dataType: 'string',
}

// Try to update a non-existent statement
const nonExistentId = 'non-existent-id' as UUID
store.updateStatement(nonExistentId, mockProperty, mockValue, 'normal')

// Should not crash and should not mark as dirty
expect(store.statements).toHaveLength(0)
expect(store.isDirty).toBe(false)
})

it('should update existing statement with references', () => {
const mockProperty: PropertyReference = {
id: 'P123',
label: 'Test Property',
dataType: 'string',
}
const mockValue: ValueMapping = {
type: 'column',
source: { columnName: 'test_column', dataType: 'VARCHAR' },
dataType: 'string',
}
const mockReferences: ReferenceSchemaMapping[] = [
{
id: crypto.randomUUID(),
snaks: [
{
property: { id: 'P248', label: 'stated in', dataType: 'wikibase-item' },
value: { type: 'constant', source: 'Wikipedia', dataType: 'wikibase-item' },
},
],
},
]

// Add initial statement
const statementId = store.addStatement(mockProperty, mockValue)
expect(store.isDirty).toBe(true)

// Mark as saved to test dirty state
store.markAsSaved()
expect(store.isDirty).toBe(false)

// Update statement with references
store.updateStatement(statementId, mockProperty, mockValue, 'normal', [], mockReferences)

// Verify the statement was updated with references
const updatedStatement = store.statements.find((s) => s.id === statementId)
expect(updatedStatement).toBeDefined()
expect(updatedStatement!.references).toHaveLength(1)
expect(updatedStatement!.references[0]!.snaks).toHaveLength(1)
expect(updatedStatement!.references[0]!.snaks[0]!.property.id).toBe('P248')
expect(updatedStatement!.references[0]!.snaks[0]!.value.source).toBe('Wikipedia')

// Verify store is marked as dirty
expect(store.isDirty).toBe(true)
})

it('should add new statement with references', () => {
const mockProperty: PropertyReference = {
id: 'P123',
label: 'Test Property',
dataType: 'string',
}
const mockValue: ValueMapping = {
type: 'column',
source: { columnName: 'test_column', dataType: 'VARCHAR' },
dataType: 'string',
}
const mockReferences: ReferenceSchemaMapping[] = [
{
id: crypto.randomUUID(),
snaks: [
{
property: { id: 'P854', label: 'reference URL', dataType: 'url' },
value: { type: 'constant', source: 'https://example.com', dataType: 'url' },
},
],
},
]

// Add statement with references
const statementId = store.addStatement(mockProperty, mockValue, 'normal', [], mockReferences)

// Verify the statement was created with references
const statement = store.statements.find((s) => s.id === statementId)
expect(statement).toBeDefined()
expect(statement!.references).toHaveLength(1)
expect(statement!.references[0]!.snaks).toHaveLength(1)
expect(statement!.references[0]!.snaks[0]!.property.id).toBe('P854')
expect(statement!.references[0]!.snaks[0]!.value.source).toBe('https://example.com')

// Verify store is marked as dirty
expect(store.isDirty).toBe(true)
})

it('should clear references when updating statement with empty references array', () => {
const mockProperty: PropertyReference = {
id: 'P123',
label: 'Test Property',
dataType: 'string',
}
const mockValue: ValueMapping = {
type: 'column',
source: { columnName: 'test_column', dataType: 'VARCHAR' },
dataType: 'string',
}
const mockReferences: ReferenceSchemaMapping[] = [
{
id: crypto.randomUUID(),
snaks: [
{
property: { id: 'P854', label: 'reference URL', dataType: 'url' },
value: { type: 'constant', source: 'https://example.com', dataType: 'url' },
},
],
},
]

// Add statement with references
const statementId = store.addStatement(mockProperty, mockValue, 'normal', [], mockReferences)

// Verify the statement was created with references
let statement = store.statements.find((s) => s.id === statementId)
expect(statement!.references).toHaveLength(1)

// Mark as saved to test dirty state
store.markAsSaved()
expect(store.isDirty).toBe(false)

// Update statement with empty references array
store.updateStatement(
statementId,
mockProperty,
mockValue,
'normal',
[],
[], // Empty references array
)

// Verify references were cleared
statement = store.statements.find((s) => s.id === statementId)
expect(statement!.references).toHaveLength(0)

// Verify store is marked as dirty
expect(store.isDirty).toBe(true)
})
})

describe('schema persistence state', () => {
Expand Down
Loading