Skip to content

chore: cleanup #107

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 1 commit into from
Aug 2, 2025
Merged

chore: cleanup #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 12 additions & 12 deletions frontend/src/composables/__tests__/useColumnDataTypeIndicators.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,30 @@ describe('useColumnDataTypeIndicators', () => {

describe('generateDataTypeTooltip', () => {
test('should generate tooltip for basic column', () => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: 'test_column',
dataType: 'VARCHAR',
sampleValues: ['sample1', 'sample2'],
nullable: false,
}

const tooltip = generateDataTypeTooltip(column)
const tooltip = generateDataTypeTooltip(columnInfo)

expect(tooltip).toContain('Data Type: Text (VARCHAR)')
expect(tooltip).toContain('Wikibase Compatible: string, url, external-id, monolingualtext')
expect(tooltip).not.toContain('Nullable: Yes')
})

test('should include nullable information when applicable', () => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: 'nullable_column',
dataType: 'INTEGER',
sampleValues: ['1', '2'],
nullable: true,
uniqueCount: 100,
}

const tooltip = generateDataTypeTooltip(column)
const tooltip = generateDataTypeTooltip(columnInfo)

expect(tooltip).toContain('Data Type: Number (INTEGER)')
expect(tooltip).toContain('Nullable: Yes')
Expand All @@ -73,29 +73,29 @@ describe('useColumnDataTypeIndicators', () => {
})

test('should handle incompatible data types', () => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: 'boolean_column',
dataType: 'BOOLEAN',
sampleValues: ['true', 'false'],
nullable: false,
}

const tooltip = generateDataTypeTooltip(column)
const tooltip = generateDataTypeTooltip(columnInfo)

expect(tooltip).toContain('Data Type: Boolean (BOOLEAN)')
expect(tooltip).toContain('Wikibase Compatible: None (requires transformation)')
})

test('should format unique count with locale formatting', () => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: 'large_column',
dataType: 'VARCHAR',
sampleValues: ['test'],
nullable: false,
uniqueCount: 1234567,
}

const tooltip = generateDataTypeTooltip(column)
const tooltip = generateDataTypeTooltip(columnInfo)

expect(tooltip).toContain('Unique Values: 1,234,567')
})
Expand Down Expand Up @@ -142,15 +142,15 @@ describe('useColumnDataTypeIndicators', () => {

describe('generateColumnTooltip', () => {
test('should combine data type and sample information', () => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: 'test_column',
dataType: 'VARCHAR',
sampleValues: ['sample1', 'sample2'],
nullable: true,
uniqueCount: 50,
}

const tooltip = generateColumnTooltip(column)
const tooltip = generateColumnTooltip(columnInfo)

expect(tooltip).toContain('Data Type: Text (VARCHAR)')
expect(tooltip).toContain('Nullable: Yes')
Expand All @@ -160,14 +160,14 @@ describe('useColumnDataTypeIndicators', () => {
})

test('should handle columns with no sample data', () => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: 'empty_column',
dataType: 'INTEGER',
sampleValues: [],
nullable: false,
}

const tooltip = generateColumnTooltip(column)
const tooltip = generateColumnTooltip(columnInfo)

expect(tooltip).toContain('Data Type: Number (INTEGER)')
expect(tooltip).toContain('Sample Values:')
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/composables/__tests__/useDragDropContext.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,9 @@ describe('createDropZoneConfig', () => {
const config = context.createDropZoneConfig(
'item.terms.labels.en',
['string'],
(column: ColumnInfo, target: DropTarget) => {
(columnInfo: ColumnInfo, target: DropTarget) => {
dropCallbackCalled = true
droppedColumn = column
droppedColumn = columnInfo
},
)

Expand Down
9 changes: 1 addition & 8 deletions frontend/src/composables/__tests__/useStatementConfig.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe('useStatementConfig', () => {
expect(store.statements[0]?.rank).toBe('preferred')
})

it('should not auto-save when statement is invalid', async () => {
it('should not auto-save when statement is invalid', () => {
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 2, 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

Verify that removing async behavior doesn't break auto-save testing.

The test was converted from async to sync, but if the useStatementConfig composable has asynchronous auto-save behavior, this change might cause the test to pass incorrectly by not waiting for the auto-save logic to complete.

🏁 Script executed:

#!/bin/bash
# Description: Check if useStatementConfig has async auto-save behavior
# Expected: Find auto-save implementation with timing/async behavior

# Search for auto-save related code in useStatementConfig
rg -A 10 -B 5 "auto.?save|setTimeout|nextTick|Promise" frontend/src/composables/useStatementConfig.ts

# Also check for watch/watchEffect that might trigger auto-save
ast-grep --pattern 'watch($_, $_)'
ast-grep --pattern 'watchEffect($_)'

Length of output: 556

Add async/await to invalid auto-save test to cover Vue watch scheduling

The useStatementConfig composable triggers auto-save inside a Vue watch callback, which runs asynchronously. By switching this test back to a synchronous function, it never waits for the watcher to fire—risking a false positive. Restore the async signature and insert an explicit wait before your assertion.

• File: frontend/src/composables/tests/useStatementConfig.test.ts
– Line 169: change
diff - it('should not auto-save when statement is invalid', () => { + it('should not auto-save when statement is invalid', async () => {
– After you apply the invalid-statement changes, add:
js import { nextTick } from 'vue' // … await nextTick() expect(saveCurrentStatementMock).not.toHaveBeenCalled()

🤖 Prompt for AI Agents 
In frontend/src/composables/__tests__/useStatementConfig.test.ts at line 169,
the test for invalid auto-save is synchronous but needs to handle Vue's
asynchronous watch behavior. Change the test function to async, import nextTick
from 'vue', and after triggering the invalid statement condition, await
nextTick() before asserting that saveCurrentStatementMock was not called to
ensure the watcher has run.

const store = useSchemaStore()
const { currentStatement, canSaveStatement, resetStatement } = useStatementConfig()

Expand All @@ -177,20 +177,13 @@ describe('useStatementConfig', () => {
expect(canSaveStatement.value).toBe(false)
expect(store.statements).toHaveLength(0)

// Wait to ensure no auto-save happens
await new Promise((resolve) => setTimeout(resolve, 0))
expect(store.statements).toHaveLength(0)

// Set only property (still incomplete without source)
currentStatement.value.property = {
id: 'P123' as PropertyId,
label: 'Test Property',
dataType: 'string',
}
expect(canSaveStatement.value).toBe(false)

// Wait to ensure no auto-save happens
await new Promise((resolve) => setTimeout(resolve, 0))
expect(store.statements).toHaveLength(0)
})

Expand Down
8 changes: 4 additions & 4 deletions frontend/src/composables/__tests__/useStatementDropZone.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ describe('useStatementDropZone', () => {
test('should call column drop callback with valid column data', () => {
const { handleDrop, setOnColumnDrop } = useStatementDropZone()

setOnColumnDrop((column) => {
droppedColumn = column
setOnColumnDrop((columnInfo) => {
droppedColumn = columnInfo
})

const mockEvent = {
Expand All @@ -141,8 +141,8 @@ describe('useStatementDropZone', () => {
test('should not call callback when no column data is provided', () => {
const { handleDrop, setOnColumnDrop } = useStatementDropZone()

setOnColumnDrop((column) => {
droppedColumn = column
setOnColumnDrop((columnInfo) => {
droppedColumn = columnInfo
})

const mockEvent = {
Expand Down
20 changes: 10 additions & 10 deletions frontend/src/composables/useColumnDataTypeIndicators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,18 @@ export const useColumnDataTypeIndicators = () => {
/**
* Generate tooltip content for column data type information
*/
const generateDataTypeTooltip = (_column: ColumnInfo): string => {
const compatibleTypes = getCompatibleWikibaseTypes(_column.dataType)
const displayName = formatDataTypeDisplayName(_column.dataType)
const generateDataTypeTooltip = (columnInfo: ColumnInfo): string => {
const compatibleTypes = getCompatibleWikibaseTypes(columnInfo.dataType)
const displayName = formatDataTypeDisplayName(columnInfo.dataType)

let tooltip = `Data Type: ${displayName} (${_column.dataType})`
let tooltip = `Data Type: ${displayName} (${columnInfo.dataType})`

if (_column.nullable) {
if (columnInfo.nullable) {
tooltip += '\nNullable: Yes'
}

if (_column.uniqueCount !== undefined) {
tooltip += `\nUnique Values: ${_column.uniqueCount.toLocaleString()}`
if (columnInfo.uniqueCount !== undefined) {
tooltip += `\nUnique Values: ${columnInfo.uniqueCount.toLocaleString()}`
}

if (compatibleTypes.length > 0) {
Expand Down Expand Up @@ -157,9 +157,9 @@ export const useColumnDataTypeIndicators = () => {
/**
* Generate comprehensive tooltip content for a column
*/
const generateColumnTooltip = (_column: ColumnInfo): string => {
const dataTypeInfo = generateDataTypeTooltip(_column)
const sampleInfo = formatSampleValuesForTooltip(_column.sampleValues)
const generateColumnTooltip = (columnInfo: ColumnInfo): string => {
const dataTypeInfo = generateDataTypeTooltip(columnInfo)
const sampleInfo = formatSampleValuesForTooltip(columnInfo.sampleValues)

return `${dataTypeInfo}\n\nSample Values:\n${sampleInfo}`
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/composables/useStatementDropZone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const useStatementDropZone = () => {

// Reactive state
const isOverDropZone = ref(false)
const onColumnDropCallback = ref<((column: ColumnInfo) => void) | null>(null)
const onColumnDropCallback = ref<((columnInfo: ColumnInfo) => void) | null>(null)

// Direct reactive references to drag store
const draggedColumn = computed(() => dragDropStore.draggedColumn)
Expand Down Expand Up @@ -71,7 +71,7 @@ export const useStatementDropZone = () => {
)

// Method to set the callback
const setOnColumnDrop = (callback: (column: ColumnInfo) => void) => {
const setOnColumnDrop = (callback: (columnInfo: ColumnInfo) => void) => {
onColumnDropCallback.value = callback
}

Expand Down
8 changes: 4 additions & 4 deletions frontend/src/composables/useStatementEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,18 +198,18 @@ export const useStatementEditor = () => {
localStatement.value.rank = newRank
}

const handleColumnDrop = (_column: ColumnInfo) => {
const handleColumnDrop = (columnInfo: ColumnInfo) => {
// Auto-suggest compatible data type
const compatibleTypes = getCompatibleWikibaseTypes(_column.dataType)
const compatibleTypes = getCompatibleWikibaseTypes(columnInfo.dataType)
const suggestedDataType: WikibaseDataType =
compatibleTypes[0] ?? (localStatement.value.value.dataType || 'string')

// Set to column type and map the column
localStatement.value.value = {
type: 'column',
source: {
columnName: _column.name,
dataType: _column.dataType,
columnName: columnInfo.name,
dataType: columnInfo.dataType,
},
dataType: suggestedDataType,
}
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/composables/useValueMapping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ export const useValueMapping = () => {

// Methods
const createValueMappingFromColumn = (
column: ColumnInfo,
columnInfo: ColumnInfo,
targetDataType?: WikibaseDataType,
): ValueMapping => {
const compatibleTypes = getCompatibleWikibaseTypes(column.dataType)
const compatibleTypes = getCompatibleWikibaseTypes(columnInfo.dataType)

let dataType: WikibaseDataType
if (targetDataType) {
Expand All @@ -103,8 +103,8 @@ export const useValueMapping = () => {
return {
type: 'column',
source: {
columnName: column.name,
dataType: column.dataType,
columnName: columnInfo.name,
dataType: columnInfo.dataType,
},
dataType,
}
Expand Down Expand Up @@ -209,11 +209,11 @@ export const useValueMapping = () => {

const autoSuggestDataType = (
property: PropertyReference,
column?: ColumnInfo,
columnInfo?: ColumnInfo,
): WikibaseDataType => {
if (!column) return property.dataType as WikibaseDataType
if (!columnInfo) return property.dataType as WikibaseDataType

const compatibleTypes = getCompatibleWikibaseTypes(column.dataType)
const compatibleTypes = getCompatibleWikibaseTypes(columnInfo.dataType)

// If property data type is compatible with column, use it
if (compatibleTypes.includes(property.dataType as WikibaseDataType)) {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/stores/__tests__/drag-drop.store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ describe('useDragDropStore', () => {
]

testCases.forEach(({ dataType, expectedTargets }) => {
const column: ColumnInfo = {
const columnInfo: ColumnInfo = {
name: `test_${dataType.toLowerCase()}`,
dataType,
sampleValues: ['test'],
Expand All @@ -329,7 +329,7 @@ describe('useDragDropStore', () => {
}))

store.setAvailableTargets(targets)
store.startDrag(column)
store.startDrag(columnInfo)

expect(store.validDropTargets.length).toBe(expectedTargets.length)
store.endDrag()
Expand Down
20 changes: 10 additions & 10 deletions frontend/src/stores/drag-drop.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ export const useDragDropStore = defineStore('dragDrop', () => {
})

// Actions
const startDrag = (_column: ColumnInfo) => {
draggedColumn.value = _column
const startDrag = (columnInfo: ColumnInfo) => {
draggedColumn.value = columnInfo
dragState.value = 'dragging'
// Calculate valid drop targets based on column data type
validDropTargets.value = calculateValidTargets(_column)
validDropTargets.value = calculateValidTargets(columnInfo)
}

const endDrag = () => {
Expand All @@ -93,10 +93,10 @@ export const useDragDropStore = defineStore('dragDrop', () => {
}

// Helper function to calculate valid drop targets for a column
const calculateValidTargets = (_column: ColumnInfo): string[] => {
if (!_column) return []
const calculateValidTargets = (columnInfo: ColumnInfo): string[] => {
if (!columnInfo) return []

const compatibleTypes = getCompatibleWikibaseTypes(_column.dataType)
const compatibleTypes = getCompatibleWikibaseTypes(columnInfo.dataType)

return availableTargets.value
.filter((target) => {
Expand All @@ -105,15 +105,15 @@ export const useDragDropStore = defineStore('dragDrop', () => {
if (!isCompatible) return false

// Check nullable constraints
if (target.isRequired && _column.nullable) return false
if (target.isRequired && columnInfo.nullable) return false

// Additional validation based on target type
switch (target.type) {
case 'label':
case 'alias': {
// Check for reasonable length constraints
const maxLength = target.type === 'label' ? 250 : 100
const hasLongValues = _column.sampleValues?.some((val) => val.length > maxLength)
const hasLongValues = columnInfo.sampleValues?.some((val) => val.length > maxLength)
if (hasLongValues) return false
break
}
Expand Down Expand Up @@ -142,8 +142,8 @@ export const useDragDropStore = defineStore('dragDrop', () => {
}

// Method to get valid targets for a specific column
const getValidTargetsForColumn = (_column: ColumnInfo): DropTarget[] => {
const validPaths = calculateValidTargets(_column)
const getValidTargetsForColumn = (columnInfo: ColumnInfo): DropTarget[] => {
const validPaths = calculateValidTargets(columnInfo)
return availableTargets.value.filter((target) => validPaths.includes(target.path))
}

Expand Down
Loading