-
Notifications
You must be signed in to change notification settings - Fork 0
feat(uppercase-conversion): Implement lowercase conversion service #173
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.
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
Conversation
This commit introduces a new `LowercaseConversionService` that provides functionality to perform lowercase conversion operations on a specified table and column. The key changes are: - Implement the `LowercaseConversionService` class that extends the `ColumnOperationService` base class - Provide the `performOperation` method to execute the lowercase conversion on the specified table and column - Implement the `buildParameterizedUpdateQuery` method to construct a parameterized UPDATE query for the lowercase conversion - Implement the `countAffectedRows` method to count the number of rows that would be affected by the lowercase conversion These changes enable the application to perform efficient and safe lowercase conversion operations on database columns.
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a lowercase conversion feature across backend and frontend: new POST route /api/project/:projectId/lowercase, a LowercaseConversionService, comprehensive API and service tests, and a frontend menu handler to invoke the operation. Minor doc/tag tweak on uppercase route and trivial formatting in existing uppercase code/tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as ColumnHeaderMenu.vue
participant API as POST /api/project/:projectId/lowercase
participant SVC as LowercaseConversionService
participant DB as Database
UI->>API: Request { projectId, column }
API->>API: Validate projectId, column, column existence
alt Column missing
API-->>UI: 400 ApiError { message: "Column not found" }
else Valid
API->>SVC: performOperation({ table, column })
SVC->>DB: UPDATE table SET col=LOWER(col) WHERE col IS NOT NULL AND col != LOWER(col)
SVC->>DB: SELECT COUNT(*) of affected rows (or use update result)
SVC-->>API: affectedRows
API-->>UI: 200 { affectedRows }
end
sequenceDiagram
autonumber
participant User as User
participant UI as ColumnHeaderMenu.vue
participant API as /project/:id/lowercase
User->>UI: Click "To lowercase"
UI->>API: POST { column }
alt Success (affectedRows > 0)
API-->>UI: { affectedRows }
UI->>User: Show success message
UI->>UI: emit("replaceCompleted")
else Success (0 rows)
API-->>UI: { affectedRows: 0 }
UI->>User: Show no-changes warning
end
opt Validation/Error
API-->>UI: 4xx/5xx ApiError
UI->>User: Show error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Tip 🧪 Early access (models): enabledWe are currently testing Sonnet 4.5 code review models, which should lead to better review quality. However, this model may result in higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/src/services/lowercase-conversion.service.ts (1)
4-42: Optional: Consider abstracting case-conversion logic.
LowercaseConversionServiceandUppercaseConversionServiceshare near-identical structure, differing only in the SQL function (LOWERvsUPPER). If additional case transformations are planned (e.g., title case), consider a parameterized base class or factory.This is a nice-to-have refactor—the current approach is clear and maintainable.
backend/src/api/project/index.ts (1)
730-733: Consider extracting the column existence check into a shared helper.The column existence validation is duplicated across multiple endpoints (replace, trim_whitespace, uppercase, and now lowercase). Extracting this into a reusable function or middleware would reduce duplication and centralize the validation logic.
Example approach:
// In a shared utilities file async function validateColumnExists( db: DuckDBConnection, table: string, column: string ): Promise<boolean> { const reader = await db.runAndReadAll( 'SELECT 1 FROM information_schema.columns WHERE table_name = ? AND column_name = ?', [table, column] ) return reader.getRows().length > 0 }Then in each endpoint:
- const columnExistsReader = await db().runAndReadAll( - 'SELECT 1 FROM information_schema.columns WHERE table_name = ? AND column_name = ?', - [table, column], - ) - - if (columnExistsReader.getRows().length === 0) { + if (!(await validateColumnExists(db(), table, column))) { return status( 400, ApiErrorHandler.validationErrorWithData('Column not found', [This refactor can be deferred to a future PR focused on reducing duplication across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/api/project/index.ts(2 hunks)backend/src/services/lowercase-conversion.service.ts(1 hunks)backend/src/services/uppercase-conversion.service.ts(1 hunks)backend/tests/api/project/lowercase.test.ts(1 hunks)backend/tests/api/project/uppercase.test.ts(1 hunks)backend/tests/services/lowercase-conversion.service.test.ts(1 hunks)backend/tests/services/uppercase-conversion.service.test.ts(7 hunks)frontend/src/features/data-processing/components/ColumnHeaderMenu.vue(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
backend/tests/services/lowercase-conversion.service.test.ts (2)
backend/src/services/lowercase-conversion.service.ts (1)
LowercaseConversionService(4-43)backend/src/plugins/database.ts (3)
initializeDb(8-48)getDb(50-55)closeDb(57-63)
backend/tests/api/project/lowercase.test.ts (1)
backend/src/plugins/database.ts (2)
initializeDb(8-48)closeDb(57-63)
backend/src/services/lowercase-conversion.service.ts (1)
backend/src/services/column-operation.service.ts (1)
ColumnOperationParams(3-6)
backend/src/api/project/index.ts (4)
backend/src/types/error-handler.ts (1)
ApiErrorHandler(6-211)backend/src/services/lowercase-conversion.service.ts (1)
LowercaseConversionService(4-43)backend/src/api/project/schemas.ts (2)
ColumnNameSchema(54-59)AffectedRowsSchema(61-63)backend/src/types/error-schemas.ts (2)
ApiErrors(20-26)ApiErrors(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (28)
backend/src/services/uppercase-conversion.service.ts (1)
43-43: LGTM! Trailing newline added.Standard formatting—no functional impact.
backend/tests/services/uppercase-conversion.service.test.ts (2)
1-1: LGTM! Import reordering.Formatting adjustment with no functional impact.
42-47: LGTM! Formatting adjustments.Multi-line formatting of assertions and single-line
runAndReadAllcalls improve consistency. No logic changes.Also applies to: 50-50, 79-84, 87-87, 123-123, 172-177, 202-207, 210-210, 242-247, 250-250
backend/tests/api/project/uppercase.test.ts (1)
114-122: LGTM! Formatting adjustment.Multi-line formatting of the assertion improves readability. No functional change.
frontend/src/features/data-processing/components/ColumnHeaderMenu.vue (2)
67-85: LGTM! Lowercase handler follows established pattern.The
handleLowerCaseimplementation mirrors the existinghandleUpperCaseandhandleTrimWhitespacehandlers: proper error handling, user feedback based onaffectedRows, and emitsreplaceCompletedto refresh the UI.
145-145: LGTM! Menu item wired to handler.The "To lowercase" menu item now invokes
handleLowerCaseinstead of a placeholder, completing the frontend integration.backend/src/services/lowercase-conversion.service.ts (4)
1-3: LGTM! Standard imports.Imports the base class and interface correctly.
4-14: LGTM! performOperation delegates to base class infrastructure.The method correctly destructures params and delegates to
executeColumnOperationwith the query builder and row counter closures. Pattern matchesUppercaseConversionService.
19-28: LGTM! Query logic is correct.The UPDATE query uses
LOWER()and correctly filters out NULL and already-lowercase values to minimize unnecessary writes. Identifier interpolation with double quotes is appropriate for DuckDB.Minor comment nuance: The comment mentions "parameterized" query, but table/column are interpolated (not bound parameters). This is correct for identifiers—params would only apply to data values. The same pattern exists in
UppercaseConversionService.
33-42: LGTM! Row counter matches UPDATE logic.The
countAffectedRowsquery uses the same WHERE clause as the UPDATE, ensuring accurate preview of changes. Consistent withUppercaseConversionService.backend/src/api/project/index.ts (3)
14-14: LGTM!The import is correctly placed and follows the established pattern for service imports.
718-720: LGTM!Good addition for API documentation consistency. The tags attribute aligns this route's OpenAPI metadata with the other endpoints.
724-781: LGTM!The lowercase endpoint implementation is solid and follows the established patterns for column operations (uppercase, trim). Column validation, service instantiation, error handling, and response schemas are all correctly implemented.
backend/tests/services/lowercase-conversion.service.test.ts (9)
1-19: LGTM!The test suite setup is well-structured with proper initialization and cleanup. Using an in-memory database for tests is appropriate and follows the existing test patterns.
21-58: LGTM!The test correctly validates basic lowercase conversion with mixed-case data. The expected affected row count (4 out of 5) is accurate since 'bob johnson' is already lowercase and should not be affected.
60-94: LGTM!The test properly validates that empty strings are handled correctly and remain empty after the operation. The affected row count expectation is accurate.
96-130: LGTM!The test correctly validates NULL value handling. NULLs are properly excluded from the operation, which aligns with the service's WHERE clause filtering.
132-144: LGTM!The test correctly validates error handling for non-existent columns. The expected error message aligns with the service implementation.
146-150: LGTM!The test correctly validates that attempting to operate on a non-existent table results in an error. The less-specific error check is appropriate since the database error message may vary.
152-178: LGTM!The test correctly validates the no-op scenario where all data is already lowercase. The expectation of 0 affected rows is accurate.
180-220: LGTM!Excellent test coverage for special characters, numbers, and various naming conventions. The test validates that the LOWER function correctly handles punctuation, digits, and special characters.
222-258: LGTM!Excellent test for internationalization support. The test validates that DuckDB's LOWER function correctly handles unicode and accented characters, which is crucial for global data processing.
backend/tests/api/project/lowercase.test.ts (6)
1-60: LGTM!The test suite setup is well-structured. The use of treaty for type-safe API testing and proper initialization/cleanup patterns are appropriate. The helper function
importTestDatapromotes code reuse.
61-87: LGTM!The test correctly validates the basic lowercase conversion flow, including both the affected row count and the actual data transformation. The verification approach using
expect.arrayContainingis appropriate.
89-104: LGTM!The test correctly validates error handling for non-existent columns. The error response structure verification is thorough and matches the API's error handling pattern.
106-123: LGTM!The test correctly validates schema-level validation for missing required fields. The 422 status code and error structure expectations are accurate.
125-187: LGTM!The test correctly validates the mixed-case scenario where some data is already lowercase. Creating a new project for this test avoids conflicts, and the expected affected row count (3) is accurate. The data verification confirms the correct transformation behavior.
189-233: LGTM!The test correctly validates the no-op scenario where all data is already lowercase. Creating a new project for this test is appropriate, and the expectation of 0 affected rows is accurate.
This commit introduces a new
LowercaseConversionServicethat providesfunctionality to perform lowercase conversion operations on a specified
table and column. The key changes are:
LowercaseConversionServiceclass that extends theColumnOperationServicebase classperformOperationmethod to execute the lowercaseconversion on the specified table and column
buildParameterizedUpdateQuerymethod to construct aparameterized UPDATE query for the lowercase conversion
countAffectedRowsmethod to count the number of rowsthat would be affected by the lowercase conversion
These changes enable the application to perform efficient and safe
lowercase conversion operations on database columns.