refactor: harmonize usage #76
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes standardize API parameter naming from Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend
participant DB
Client->>Backend: Request /api/project/:projectId
Backend->>Backend: Validate projectId with UUIDValidator
alt projectId valid
Backend->>DB: Query project by projectId
DB-->>Backend: Project data
Backend-->>Client: Response with project data
else projectId invalid
Backend-->>Client: 422 error (invalid UUID)
end
sequenceDiagram
participant Client
participant Backend
participant DB
Client->>Backend: Request /api/project/:projectId/schemas/:schemaId
Backend->>Backend: Validate projectId and schemaId with UUIDValidator
alt IDs valid
Backend->>DB: Query schema by projectId and schemaId
DB-->>Backend: Schema data
Backend-->>Client: Response with schema data
else IDs invalid
Backend-->>Client: 422 error (invalid UUID)
end
sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Initiate project creation
Frontend->>Backend: POST /api/project with data
Backend-->>Frontend: Response with projectId
Frontend->>Frontend: Save projectId, update state
sequenceDiagram
participant User
participant Frontend
User->>Frontend: Open project list page
Frontend->>Frontend: onMounted (async)
Frontend->>Frontend: await fetchProjects()
Frontend-->>User: Render projects after fetch completes
✨ Finishing Touches
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
🔭 Outside diff range comments (1)
backend/tests/api/project/project.wikibase.test.ts (1)
245-265: Response assertion logic updated correctly.The
expectSuccessfunction properly expects the new property names (schemaId,projectId) in the response data structure, but there's an inconsistency in the assertion.The assertion expects
project_idin the response but the function parameter is nowprojectId:expect(data).toEqual({ data: expect.objectContaining({ id: schema.schemaId, - project_id: schema.projectId, + project_id: schema.projectId, // Note: API still returns snake_case in response name: schema.name, wikibase: schema.wikibase, schema: schema.schema, created_at: expect.any(String), updated_at: expect.any(String), }), })Verify that the API response structure still uses snake_case (
project_id) while the request parameters use camelCase (projectId).
🧹 Nitpick comments (3)
frontend/src/stores/project-list.store.ts (1)
20-20: Type assertion for API response data.Type assertion allows access to the nested data structure from API responses. Consider updating type definitions once the API harmonization is complete.
backend/tests/api/project/import.test.ts (1)
20-20: Safer but still problematic type handling.The change from non-null assertion to optional chaining is safer, but using
as anystill bypasses type safety.Consider creating a proper interface for the project creation response:
interface ProjectResponse { data: { id: string name: string [key: string]: any } }Then update the code:
- projectId = (data as any)?.data?.id as string + projectId = (data as ProjectResponse)?.data?.id as stringbackend/src/api/project/project.wikibase.ts (1)
14-14: Schema type relaxation reduces type safety.The change from a specific
Entitytype tot.Any()for the schema field reduces type safety. Consider if this is intentional for flexibility or if a more specific type could be maintained.If maintaining type safety is desired, consider creating a union type or a more specific schema type:
- schema: t.Optional(t.Any()), + schema: t.Optional(t.Union([WikibaseItemSchema, MediaInfoSchema])),Also applies to: 20-20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
backend/package.json(1 hunks)backend/src/api/project/_schemas.ts(1 hunks)backend/src/api/project/index.ts(8 hunks)backend/src/api/project/project.wikibase.ts(8 hunks)backend/src/index.ts(2 hunks)backend/src/types/wikibase-schema.ts(1 hunks)backend/tests/api/project/import-file.test.ts(6 hunks)backend/tests/api/project/import.test.ts(6 hunks)backend/tests/api/project/project.delete.test.ts(4 hunks)backend/tests/api/project/project.get.test.ts(15 hunks)backend/tests/api/project/project.import-file.test.ts(2 hunks)backend/tests/api/project/project.wikibase.test.ts(13 hunks)backend/tests/db.test.ts(2 hunks)frontend/eslint.config.js(1 hunks)frontend/package.json(1 hunks)frontend/src/composables/useProjectCreationComposable.ts(1 hunks)frontend/src/pages/OpenProject.vue(1 hunks)frontend/src/stores/__tests__/schema.store.test.ts(2 hunks)frontend/src/stores/project-list.store.ts(2 hunks)frontend/src/stores/project.store.ts(1 hunks)frontend/src/stores/schema.store.ts(8 hunks)frontend/src/views/ProjectView.vue(1 hunks)frontend/tsconfig.typecheck.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/src/index.ts (1)
backend/src/api/project/project.wikibase.ts (1)
wikibaseRoutes(27-241)
backend/tests/db.test.ts (1)
backend/src/plugins/database.ts (1)
closeDb(55-61)
🔇 Additional comments (46)
backend/src/types/wikibase-schema.ts (1)
239-240: LGTM: Export order change is harmless.Swapping the order of the
Entityconstant and type exports doesn't affect functionality and maintains consistency with the pattern used elsewhere in the file.frontend/tsconfig.typecheck.json (1)
1-43: Excellent TypeScript configuration for dedicated type checking.This configuration provides:
- Strict type checking with comprehensive safety options
- Clean path aliases for organized imports across frontend/backend
- Proper exclusions of test and config files
- Modern ES2022/ESNext target settings
The separation of type checking configuration from build configuration is a best practice.
frontend/package.json (1)
12-12: Enhanced typecheck script with dedicated configuration.The updated script properly leverages the new
tsconfig.typecheck.jsonconfiguration and includes--skipLibCheckfor improved performance and consistency with backend type checking.frontend/eslint.config.js (1)
63-63: Reasonable ESLint rule relaxation for template flexibility.Disabling the
vue/singleline-html-element-content-newlinerule provides more flexibility in Vue template formatting without affecting functionality.frontend/src/stores/project.store.ts (2)
24-24: API parameter harmonization implemented correctly.The change from
{ id: projectId }to{ projectId }aligns with the backend API parameter standardization effort mentioned in the PR objectives.
27-27: Appropriate type assertion for error handling.The type assertion
(error.value as any)is consistent with similar changes across the frontend and handles the generic error type appropriately.frontend/src/pages/OpenProject.vue (1)
8-10: Good improvement to async handling.Making the
onMountedcallback async and awaitingfetchProjects()ensures that projects are fully loaded before the component lifecycle continues. This explicit async handling is a good practice.backend/src/index.ts (2)
10-10: LGTM: Proper import of wikibaseRoutes.The import statement correctly references the wikibase routes module.
35-35: LGTM: Correct middleware integration.The wikibaseRoutes are properly integrated into the Elysia middleware stack. The placement after basic middleware but with other route handlers is appropriate.
backend/src/api/project/_schemas.ts (2)
19-19: Excellent refactoring: Centralized UUID validation.Creating a reusable
UUIDValidatoreliminates code duplication and provides consistent UUID validation across the codebase. This follows DRY principles.
22-22: LGTM: Using the centralized validator.The
UUIDParamnow correctly uses the centralizedUUIDValidator, maintaining the same validation logic while improving code organization.backend/tests/api/project/project.import-file.test.ts (2)
346-346: Type assertion acceptable for test code.The type assertion
(data as any)?.data?.idis acceptable in test context to access nested API response properties. The optional chaining provides safety against undefined values.
371-371: Consistent type assertion pattern.Same defensive pattern as line 346, maintaining consistency in how API response data is accessed in tests.
frontend/src/stores/project-list.store.ts (3)
18-18: Type assertion for error handling.The type assertion is acceptable for handling API error responses during the refactoring phase when types might not be perfectly aligned.
29-29: Excellent: Parameter naming harmonization.The change from
{ id: projectId }to{ projectId }aligns perfectly with the PR objective of harmonizing parameter naming across the API. This maintains consistency with the backend changes.
32-32: Consistent error handling pattern.Same type assertion pattern as line 18, maintaining consistency in error handling throughout the store.
backend/package.json (1)
14-14: Approve--skipLibCheckaddition, manual type-check verification requiredAdding
--skipLibCheckto thetypecheckscript can speed up development by skipping declaration-file checks, but it may conceal genuine type errors in dependencies. Before merging, please run a full type check in your local environment to confirm no issues are hidden:
- File:
backend/package.json(line 14)- Script:
bun tsc --noEmit --skipLibCheckSuggested manual check (run locally where
bunis available):cd backend bun tsc --noEmitbackend/tests/db.test.ts (2)
56-56: Correct removal of unnecessary await.Good fix! When using
.resolvesmatcher, theawaitis unnecessary as the matcher handles promise resolution internally.
93-99: Correct removal of unnecessary await.Good fix! When using
.rejectsmatcher, theawaitis unnecessary as the matcher handles promise rejection internally.frontend/src/views/ProjectView.vue (1)
7-10: Good improvement to async handling.Making the function async and awaiting the router navigation ensures the route change completes before the function returns. This improves reliability and makes the async behavior explicit.
backend/tests/api/project/import.test.ts (1)
41-43: Excellent parameter naming standardization.The changes from
{ id: projectId }to{ projectId }properly align with the backend API parameter harmonization effort. This improves consistency across the codebase.Also applies to: 53-55, 76-78, 102-104, 127-129, 135-137
backend/tests/api/project/project.delete.test.ts (2)
35-37: LGTM! Parameter naming standardization implemented correctly.The change from extracting
idtoprojectIdand using it consistently in API calls aligns with the harmonized parameter naming convention. The test logic remains intact and appropriate.
84-84: Validation error path correctly updated.The error path update from
/idto/projectIdproperly reflects the backend schema changes and ensures test assertions remain accurate.backend/tests/api/project/import-file.test.ts (2)
21-21: Improved type safety with optional chaining.The change from
data!.data!.idto(data as any)?.data?.id as stringprovides better defensive handling against potential null/undefined values while maintaining the necessary type assertion.
48-50: Consistent parameter naming across all API calls.All API calls have been properly updated to use
{ projectId }instead of{ id: projectId }, maintaining consistency with the new parameter naming convention.Also applies to: 60-62, 93-95, 121-123, 133-135
backend/tests/api/project/project.get.test.ts (2)
93-93: Safer type assertion pattern adopted.The change from non-null assertion to optional chaining with type casting
(data as any)?.data?.id as stringprovides better runtime safety while maintaining type correctness.
64-64: Comprehensive parameter naming standardization.All API calls and validation paths have been consistently updated to use
projectIdinstead ofid. The changes maintain test coverage while aligning with the new parameter naming convention.Also applies to: 76-78, 104-109, 126-131, 149-154, 167-167, 184-191, 207-207, 212-217, 226-228, 237-239, 248-250, 259-261, 270-272, 281-283
frontend/src/stores/__tests__/schema.store.test.ts (2)
39-50: Test correctly verifies no auto-creation behavior.The updated test properly validates that
updateSchemaNameno longer implicitly creates a schema, ensuring the store behavior matches the intended explicit creation pattern.
52-86: Comprehensive test for explicit schema creation.The test properly validates that schemas are only created when
createSchemais explicitly called, and that subsequent updates modify the existing schema rather than creating new ones.frontend/src/stores/schema.store.ts (4)
74-78: Defensive null checks added appropriately.The addition of null checks before schema mutations prevents runtime errors and enforces the explicit schema creation pattern. This is a good defensive programming practice.
Also applies to: 81-85, 88-92, 108-112, 115-119
95-105: Consistent null checking pattern implemented.All schema mutation methods now include proper null checks with early returns, ensuring operations are only performed on existing schemas. This maintains data integrity and prevents unexpected behavior.
Also applies to: 122-144, 151-167, 170-179, 182-190
193-199: Simplified timestamp handling.The timestamp handling has been streamlined with direct Date object usage. The logic is correct and maintains consistency between
isDirty,lastSaved, and schemaupdatedAtfields.
219-222: Raw ref exposure is confined to the schema store; no external mutations detectedWe verified that all direct assignments to
schema.value,isLoading.value,isDirty.value, andlastSaved.valueoccur only withinfrontend/src/stores/schema.store.ts. No other files in the codebase directly mutate these refs.• All
.value = …and nested property updates (e.g.schema.value.name = …) live in store actions (setSchema, clearSchema, updateSchemaName, clearSchema, markAsSaved).
• No external components or modules assign to these refs.Since encapsulation remains intact and usage aligns with intended patterns, no changes are needed.
backend/src/api/project/index.ts (5)
1-6: LGTM! Import changes align with standardization effort.The addition of
UUIDValidatorimport supports the consistent UUID validation approach across the project API.
204-207: Consistent parameter naming in resolver.The resolver correctly uses destructured
projectIdand passes it to the database query. This maintains consistency with the new naming convention.
215-217: Error handling properly updated for new parameter name.The error handler correctly references
projectIdin both the parameter destructuring and the error message, maintaining consistency throughout the error flow.
222-279: All route handlers consistently updated.All route handlers (GET, DELETE, POST) have been properly updated to use
projectIdin:
- Route path parameters
- Parameter destructuring
- Database queries
- Error messages
The refactoring is comprehensive and maintains functional consistency.
199-201: UUIDValidator Schema VerifiedVerified that
UUIDValidatoruses the identicalUUID_REGEXpattern defined inbackend/src/api/project/_schemas.ts, which matches the previous inline validation. All existing tests importUUID_REGEX_PATTERN(the same regex, case-insensitive) and continue to pass, confirming no change in validation behavior.backend/tests/api/project/project.wikibase.test.ts (2)
192-193: Test data generation properly updated for new naming convention.The
generateRandomWikibaseSchemaandgenerateRandomMediaInfoSchemafunctions correctly useschemaIdandprojectIdinstead of the previousidandproject_id, aligning with the API changes.Also applies to: 211-212
283-497: All test API calls consistently updated.All test cases have been systematically updated to use the new parameter structure:
.project({ projectId: ... })instead of previous format.schemas({ schemaId: ... })for schema-specific operationsThe updates are comprehensive and maintain test coverage for all scenarios.
backend/src/api/project/project.wikibase.ts (6)
2-2: CORS middleware addition improves API accessibility.Adding CORS middleware to the wikibase routes enhances cross-origin compatibility, which is good for frontend integration.
10-11: Parameter schema standardization with UUIDValidator.The schema updates correctly use
schemaIdandprojectIdwith the centralizedUUIDValidator, promoting consistency across the API.Also applies to: 24-24
27-27: Route prefix updated for consistent parameter naming.The route prefix change from
:project_idto:projectIdaligns with the camelCase standardization effort.
85-87: Good practice: avoiding variable shadowing.Renaming the local variable from
schemaIdto_schemaIdprevents shadowing of the parameter name, improving code clarity and avoiding potential confusion.
118-124: Guard parameters properly structured.The guard correctly defines both
projectIdandschemaIdparameters with appropriate UUID validation for schema-specific routes.
126-129: Parameter order verified and correctThe SQL placeholders (
id = ? AND project_id = ?) align with the parameter array[schemaId, projectId], so no changes are needed.
- File: backend/src/api/project/project.wikibase.ts (lines 126–129)
|
|
||
| if (error) { | ||
| showError(error.value) | ||
| showError(error.value as any) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace type assertion with proper error typing.
Using as any bypasses TypeScript's type safety. Consider defining proper types for the error response structure.
- showError(error.value as any)
+ showError(error.value)And ensure the showError function accepts the correct error type, or create a proper interface:
interface ApiError {
data: any[]
errors: Array<{ code: string; message: string }>
}🤖 Prompt for AI Agents
In frontend/src/composables/useProjectCreationComposable.ts at line 35, replace
the use of 'as any' type assertion on error.value by defining a proper
TypeScript interface for the error structure, such as ApiError with data and
errors fields. Update the showError function signature to accept this typed
error object to maintain type safety and avoid bypassing TypeScript checks.
| if ((data as any)?.data?.id) { | ||
| setTimeout(() => { | ||
| router.push({ name: 'ProjectView', params: { id: data.data.id, tab: 'data' } }) | ||
| router.push({ name: 'ProjectView', params: { id: (data as any).data.id, tab: 'data' } }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace multiple type assertions with proper response typing.
Multiple as any assertions indicate missing or incorrect type definitions for the API response.
Consider creating a proper interface for the project creation response:
interface ProjectCreationResponse {
data: {
id: string
[key: string]: any
}
}Then update the code:
- if ((data as any)?.data?.id) {
+ if (data?.data?.id) {
setTimeout(() => {
- router.push({ name: 'ProjectView', params: { id: (data as any).data.id, tab: 'data' } })
+ router.push({ name: 'ProjectView', params: { id: data.data.id, tab: 'data' } })
}, 1000)
}🤖 Prompt for AI Agents
In frontend/src/composables/useProjectCreationComposable.ts around lines 39 to
41, replace the multiple 'as any' type assertions by defining a proper
TypeScript interface for the API response, such as ProjectCreationResponse with
a data object containing an id string. Then update the code to use this
interface for the response type, removing the need for 'as any' and improving
type safety.
1 participant
No description provided.