feat: support "is" and "contains" condition in issue search #1400
Conversation
WalkthroughThis pull request refactors the structure of query objects across various DTOs and service files. The dynamic index signatures and shorthand property syntax have been replaced with explicit Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🔭 Outside diff range comments (1)
apps/api/src/domains/admin/feedback/feedback.service.ts (1)
94-168: 💡 Verification agent🧩 Analysis chain
Consider deprecating the old validation method.
The old
validateQuerymethod still exists but appears to be replaced byvalidateQueryV2in two locations. Consider adding a deprecation notice or removing it if it's no longer used elsewhere in the codebase.- private validateQuery( + /** + * @deprecated Use validateQueryV2 instead + */ + private validateQuery(
🏁 Script executed:
#!/bin/bash # Check if validateQuery is still used elsewhere in the codebase echo "Searching for validateQuery usage in the codebase..." rg -A 1 -B 1 "validateQuery\(" --glob "!feedback.service.ts"Length of output: 177
Below is a proposed rewritten review comment. However, before finalizing, please run the following additional shell script to double-check that there are no external references to
validateQueryoutside offeedback.service.ts:If this complete repository search confirms no other usages, then you can safely add a deprecation notice.
Deprecate the legacy validateQuery method
It appears that
validateQueryis no longer used outside of its own file—replaced in two locations byvalidateQueryV2. Please add a deprecation notice to the method (or remove it entirely if it’s not needed any longer). For example:- private validateQuery( + /** + * @deprecated Use validateQueryV2 instead + */ + private validateQuery(
- Action required: Manually verify with the provided shell script that there are no external references to
validateQuery.
🏁 Script executed:
#!/bin/bash echo "Searching for validateQuery usage in the entire repository..." rg -A 1 -B 1 "validateQuery\("Length of output: 630
Deprecate legacy validateQuery method in feedback.service.ts
It appears that the
validateQuerymethod is used exclusively within this file—its only reference being the call atthis.validateQuery(dto.query ?? {}, fields);. Since you've introducedvalidateQueryV2in its place in other parts of the codebase, please add a deprecation notice to help steer future usage away from the old method. Also, review the call site to ensure it’s updated to usevalidateQueryV2where appropriate.- private validateQuery( + /** + * @deprecated Use validateQueryV2 instead. + */ + private validateQuery(
- Action required:
- Add the deprecation annotation to the
validateQuerymethod.- Verify and, if applicable, update the call site(s) within
feedback.service.tsto usevalidateQueryV2.
🧹 Nitpick comments (3)
apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts (1)
39-40: Consider removing redundant propertiesWhile the explicit
key/valuestructure is good, the class still maintains separatecreatedAtandupdatedAtproperties (lines 29 and 37) which are now redundant with this new structure. These properties could be removed since they can be represented using the generic key/value pair structure.apps/api/src/domains/admin/feedback/dtos/requests/find-feedbacks-by-channel-id-request-v2.dto.ts (1)
32-39: Update API property description to reflect the new query structure.The current API description still references
createdAtandupdatedAtas examples, but these fields are no longer part of the query structure. Consider updating the description to reflect the new approach with explicitkeyandvalueproperties.@ApiProperty({ required: false, description: - "You can query by key-value with this object. (createdAt, updatedAt are kind of examples) If you want to search by text, you can use 'searchText' key.", + "You can query using the key and value properties. The key represents the field to query, and value contains the search criteria. For text searches, use 'searchText' as the key.", type: [QueryV2], })apps/api/src/domains/admin/feedback/feedback.os.service.ts (1)
507-507: Avoid using thedeleteoperator for performance reasons.Static analysis suggests assigning
undefinedornullto the property instead of using thedeleteoperator. For instance:- delete queries[i].value; + queries[i].value = undefined;This approach tends to be more optimized in JavaScript engines and avoids hidden class deoptimizations.
Also applies to: 596-596
🧰 Tools
🪛 Biome (1.9.4)
[error] 507-507: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/api/integration-test/test-specs/feedback.integration-spec.ts(2 hunks)apps/api/src/domains/admin/feedback/dtos/find-feedbacks-by-channel-id-v2.dto.ts(1 hunks)apps/api/src/domains/admin/feedback/dtos/generate-excel.dto.ts(1 hunks)apps/api/src/domains/admin/feedback/dtos/requests/find-feedbacks-by-channel-id-request-v2.dto.ts(1 hunks)apps/api/src/domains/admin/feedback/dtos/scroll-feedbacks-v2.dto.ts(1 hunks)apps/api/src/domains/admin/feedback/feedback.mysql.service.ts(2 hunks)apps/api/src/domains/admin/feedback/feedback.os.service.ts(6 hunks)apps/api/src/domains/admin/feedback/feedback.service.ts(3 hunks)apps/api/src/domains/admin/project/issue/dtos/find-issues-by-project-id-v2.dto.ts(1 hunks)apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/api/src/domains/admin/feedback/feedback.service.ts (1)
apps/api/src/domains/admin/feedback/dtos/find-feedbacks-by-channel-id-v2.dto.ts (1) (1)
FindFeedbacksByChannelIdDtoV2(21-32)
🪛 Biome (1.9.4)
apps/api/src/domains/admin/feedback/feedback.os.service.ts
[error] 507-507: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 596-596: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (15)
apps/api/src/domains/admin/project/issue/dtos/find-issues-by-project-id-v2.dto.ts (1)
29-30: Structure alignment for query conditionsThe explicit
keyandvalueproperties create a clearer structure for query conditions, which aligns well with the PR objective of supporting theIScondition for exact match multiselect options. This approach makes the query structure more explicit and easier to validate.apps/api/src/domains/admin/feedback/dtos/generate-excel.dto.ts (1)
24-25: Standardized query structure improves code consistencyThe change to use explicit
keyandvalueproperties instead of an index signature provides better type safety and makes the structure more consistent with other DTOs in the system. This is a good improvement.apps/api/integration-test/test-specs/feedback.integration-spec.ts (2)
209-211: Test correctly implements the new query structureThe test has been properly updated to use the new explicit key/value structure along with the
IScondition as specified in the PR objectives. This helps validate that the new condition works as expected for exact matching.
386-392:❓ Verification inconclusive
Verify date range handling with IS condition
The test is using the
IScondition with a date range, which is interesting sinceIStypically implies exact matching. Make sure this is the intended behavior for date ranges and that the backend properly handles this combination.
🏁 Script executed:
#!/bin/bash # Check how IS condition is used with date ranges in the codebase rg -A 3 "IS.*date|TimeRange.*IS" --type tsLength of output: 680
Revisit Date Range Matching Logic
The test at
apps/api/integration-test/test-specs/feedback.integration-spec.ts(lines 386–392) uses theQueryV2ConditionsEnum.IScondition with a date range for thecreatedAtfield. Since theIScondition usually implies an exact match, please confirm that the backend is intended to interpret this combination (withgteandltboundaries) correctly. There wasn’t additional evidence in the codebase (e.g., from migration or other query handling logic) that clarifies an alternate interpretation for date ranges withIS.
- Action: Verify that the query logic in the API handles the
IScondition for date ranges as intended. If this behavior is deliberate, consider adding documentation or inline comments to clarify the design choice.apps/api/src/domains/admin/feedback/dtos/find-feedbacks-by-channel-id-v2.dto.ts (1)
24-26: Improved type safety with explicit key-value structure.The change from dynamic index signatures to explicit
keyandvalueproperties enhances type safety and code clarity. This restructuring aligns with the PR objective of supporting theIScondition for exact match multiselect options in the feedback issue search functionality.apps/api/src/domains/admin/feedback/feedback.service.ts (3)
170-241: Well-implemented validation method for the new query structure.The
validateQueryV2method is appropriately designed to validate the new query structure with explicitkeyandvalueproperties. The method follows the same pattern as the existingvalidateQuerymethod, making it easy to understand while supporting the new format needed for theIScondition.
622-622: Correct implementation of the new validation method.The validation call has been properly updated to use the new
validateQueryV2method, ensuring consistency with the updated DTO structure.
789-789: Consistent implementation of the new validation method.The validation call in the
generateFilemethod has been correctly updated to use the newvalidateQueryV2method, maintaining consistency across the service.apps/api/src/domains/admin/feedback/dtos/scroll-feedbacks-v2.dto.ts (1)
23-26: Consistent implementation of the explicit key-value structure.The changes to the
queriesproperty maintain consistency with other DTOs in the codebase, enhancing type safety and maintainability. The structure now clearly defines what constitutes a valid query object.apps/api/src/domains/admin/feedback/dtos/requests/find-feedbacks-by-channel-id-request-v2.dto.ts (1)
23-26: Enhanced query structure with added IDs support.The transition to explicit
keyandvalueproperties improves type safety, and the addition of theidsproperty provides a way to query by ID directly, which enhances the API's functionality.apps/api/src/domains/admin/feedback/feedback.mysql.service.ts (2)
312-316: Confirm the date boundary logic forcreatedAt.You're currently applying
>= :gteand< :lt. If you intended to include records exactly at the boundary time, consider whether< :ltshould be<= :ltinstead. Otherwise, carefully verify that the current exclusivity aligns with your product requirements.
345-355: Verify performance implications of JSON aggregation in the HAVING clause.Using
JSON_ARRAYAGGwithinHAVINGcan be costly on large datasets. Consider indexing or alternative approaches (e.g., precomputed columns or join relationships) if performance becomes an issue.apps/api/src/domains/admin/feedback/feedback.os.service.ts (3)
39-39: No issues found with the import change.
60-63: Good addition of condition parameter.Allowing a default
QueryV2ConditionsEnum.CONTAINSprovides flexibility for different matching behaviors.
499-505: Logic for mergingissueIdsintoidslooks appropriate.Ensuring that we remove the original
issueIdsproperty and unify it intoqueries[i].idsis a clean approach. No concerns noted with the new condition handling.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/integration-test/global.setup.ts(1 hunks)apps/api/integration-test/test-specs/user.integration-spec.ts(3 hunks)apps/api/src/domains/admin/feedback/feedback.os.service.ts(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/api/src/domains/admin/feedback/feedback.os.service.ts
[error] 506-506: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 595-595: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (5)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (3)
39-39: No issues with the new import.
276-279: Date range logic looks good.
No concerns with this condition block; it appears consistent with the new query structure.
318-372: Multi-selectIScondition logic is consistent.
The advanced approach for exact matching, including the script check for matching array lengths, looks correct and aligns with your requirements.apps/api/integration-test/global.setup.ts (1)
25-27: Environment variable setup looks correct for integration tests.
Defining secondary URLs for MySQL in a test environment is reasonable. No concerns here.apps/api/integration-test/test-specs/user.integration-spec.ts (1)
30-39: Tenant setup logic is properly integrated.
Adding the tenant repository and service, clearing repositories, and creating a tenant in the test setup all appear coherent and maintainable.Also applies to: 50-76
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/domains/admin/feedback/feedback.os.service.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (5)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (5)
60-76: Multiple database queries can be combined for better performance.You're executing two consecutive database queries for the same feedback IDs, which is unnecessary and impacts performance. Consider consolidating these into a single query.
78-90: Well-implementedIScondition filtering.The condition-based filtering works correctly by checking both the length equality and ensuring every issue ID exists in the feedback. This implements the exact match functionality as required for the IS condition.
319-356: Good implementation of exact match filtering for multiSelect fields.Your implementation properly handles the
IScondition for multiSelect fields using bothterms_setwithminimum_should_match_scriptand an additional script check to ensure exact length matching. The empty array case is also handled correctly.
276-279: Improved query structure with explicit key-value pairs.The updated approach of checking for conditions using
query.key === 'createdAt'instead of direct property access aligns well with the refactored query structure. This change supports the PR objective of making field keys and values more explicit.
39-39: Updated import statement.Changed from type import to regular import, indicating that
Feedbackis now used as a value rather than just a type.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/api/src/domains/admin/user/dtos/requests/get-all-users-request.dto.ts (1)
19-24:⚠️ Potential issueRemove unused import
The pipeline flagged an unused import.
import { IsEnum, - IsNumber, IsOptional, IsString, ValidateNested, } from 'class-validator';🧰 Tools
🪛 GitHub Check: lint
[failure] 20-20:
'IsNumber' is defined but never used. Allowed unused vars must match /^_/u🪛 GitHub Actions: CI
[error] 20-20: 'IsNumber' is defined but never used. Allowed unused vars must match /^_/u @typescript-eslint/no-unused-vars
🧹 Nitpick comments (1)
apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts (1)
54-62: Update example to match the new QueryV2 structure.The example in the
@ApiPropertydecorator for thequeriesproperty doesn't reflect the new structure with explicitkey,value, andconditionproperties.@ApiProperty({ required: false, description: 'You can query by key-value with this object.', - example: { name: 'issue name' }, + example: [{ key: 'name', value: 'issue name', condition: 'EQ' }], type: [QueryV2], })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/api/src/domains/admin/feedback/dtos/requests/find-feedbacks-by-channel-id-request-v2.dto.ts(1 hunks)apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts(2 hunks)apps/api/src/domains/admin/project/member/dtos/find-all-members.dto.ts(1 hunks)apps/api/src/domains/admin/project/member/dtos/requests/get-all-member-request.dto.ts(1 hunks)apps/api/src/domains/admin/project/member/member.service.ts(1 hunks)apps/api/src/domains/admin/user/dtos/find-all-users.dto.ts(1 hunks)apps/api/src/domains/admin/user/dtos/requests/get-all-users-request.dto.ts(2 hunks)apps/api/src/domains/admin/user/user.service.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
apps/api/src/domains/admin/user/dtos/requests/get-all-users-request.dto.ts
[error] 20-20: 'IsNumber' is defined but never used. Allowed unused vars must match /^_/u @typescript-eslint/no-unused-vars
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (20)
apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts (4)
17-17: LGTM: Necessary import addition.The import statement is correctly updated to include
IsEnumandIsStringfromclass-validator, which are needed for the new validation decorators used in theQueryV2class.
20-20: LGTM: Added required enum import.The import for
QueryV2ConditionsEnumis necessary for validating theconditionproperty and documenting it in the API schema.
39-44: Good restructuring of the QueryV2 class.The change from using an index signature to explicit
keyandvalueproperties improves type safety and makes the API contract more clear. The validation decorator@IsString()for the key property ensures proper data validation.
46-51: Good addition of API documentation and validation for condition.Adding the
@ApiPropertydecorator with enum information and the@IsEnumvalidation improves both API documentation and ensures that only valid enum values are accepted.apps/api/src/domains/admin/feedback/dtos/requests/find-feedbacks-by-channel-id-request-v2.dto.ts (4)
17-21: Good enhancement of import statements to support the new query structure.The additional imports properly support the new
QueryV2structure with enum types and validation decorators. TheIsNullabledecorator is appropriately imported from the user domain to handle nullable values.
24-27: Well-definedidsarray property.Adding the
idsarray property with proper validation via@IsArray()provides a clear way to filter by specific feedback IDs, enhancing the search capabilities.
29-35: Good implementation of explicit key-value structure.The replacement of dynamic index signatures with explicit
keyandvalueproperties improves type safety and makes the API contract more clear. Thevaluetype supports multiple data types to accommodate different search scenarios.
37-42: Excellent addition of theconditionproperty with IS support.The introduction of the
conditionproperty usingQueryV2ConditionsEnumproperly implements the requirement to support theIScondition for exact matching, as specified in the PR objectives.apps/api/src/domains/admin/project/member/dtos/find-all-members.dto.ts (1)
24-27: Well-structured query interface with explicit key-value pairs.The refactored query structure with explicit
keyandvalueproperties aligns with the project-wide standardization of query objects. This approach provides better type safety and makes the API contract more clear.apps/api/src/domains/admin/user/dtos/find-all-users.dto.ts (1)
24-26: Improved query structure with comprehensive value type support.The refactored query structure now properly supports multiple value types, including string arrays and number arrays. This enhancement allows for more flexible and powerful queries when searching for users.
apps/api/src/domains/admin/project/member/member.service.ts (3)
115-119: Good refactoring to support the new key-value query structure.The refactored code properly destructures the new query structure, extracting
key,value, andconditionproperties for use in the query builder.
119-159: Well-implemented IS condition support for multiple fields.The code now properly handles both the
IScondition (exact match) and the default LIKE condition (partial match) for the member fields including role, name, email, and department. The ternary operators effectively handle the different query conditions.
159-171: Good improvement in date range handling within the main query loop.The date range handling for
createdAthas been nicely integrated into the main query loop, making the code more maintainable and consistent. The check for the value being an object and the extraction ofgteandltproperties are handled properly.apps/api/src/domains/admin/project/member/dtos/requests/get-all-member-request.dto.ts (2)
17-18: Import refactoring looks goodThe updated imports properly support the new DTO structure with the addition of
IsNullablefrom the user decorators and explicit validation imports.Also applies to: 21-21
24-37: New query structure enhances API flexibilityThe refactoring from specific field properties to a generic key-value structure with explicit condition support aligns with the PR objectives to support the
IScondition for exact matching. The implementation properly enforces type validation using appropriate decorators.apps/api/src/domains/admin/user/user.service.ts (3)
23-23: New import for TimeRange looks goodThe TimeRange import is correctly added to support the refactored query structure.
63-89: Query processing logic improvedThe refactored query processing logic properly handles the new key-value structure and adds support for the
IScondition. The implementation correctly:
- Extracts key, value, and condition from each query
- Processes different field types appropriately
- Uses exact matching (
=) for IS condition and partial matching (LIKE) for other conditions- Handles TimeRange objects for date filtering
This implementation fully supports the PR objectives for enhancing search with the IS condition.
93-93: Empty line formatting looks goodThe added empty line improves code readability by separating the query building logic from the ordering logic.
apps/api/src/domains/admin/user/dtos/requests/get-all-users-request.dto.ts (2)
28-28: Import path change looks goodThe relative import path for
IsNullableis correctly updated.
39-52: Enhanced query structure with proper validationThe refactored
UserSearchQueryclass now properly supports the new key-value structure with appropriate validation:
- Required
keyproperty with string validation- Required
valueproperty that supports multiple data types (string, arrays, TimeRange, null)- Updated
conditionproperty to use theQueryV2ConditionsEnumThis implementation aligns with the PR objectives and enhances type safety.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/api/src/common/enums/query-v2-conditions.enum.ts (1)
16-20: Consider adding JSDoc comments to improve enum documentation.While the enum values are self-explanatory, adding JSDoc comments explaining the purpose of each condition would improve code maintainability and assist developers who are new to the codebase.
export enum QueryV2ConditionsEnum { + /** + * Checks if a field contains the specified value + */ CONTAINS = 'CONTAINS', + /** + * Checks for exact match with the specified value(s) + */ IS = 'IS', + /** + * Checks if a value falls within a specified range + */ BETWEEN = 'BETWEEN', }apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts (3)
34-41: Consider adding error handling for the async request.
ThetoQueryfunction’sissueproperty awaits a network call without retry or error handling. If the request fails, this code might throw unhandled exceptions.issue: async (value) => { try { const { data } = await client.post({ path: '/api/admin/projects/{projectId}/issues/search', pathParams: { projectId }, body: { - queries: [{ key: 'name', value, condition: 'IS' }] as SearchQuery[], + queries: [ + { key: 'name', value, condition: 'IS' } + ] as SearchQuery[], }, }); const result = data?.items.find((v) => v.name === value); return result ? [result.id] : []; } catch (err) { // Handle error or return a default value return []; } },
61-65: WrapdecodeandJSON.parsein a try-catch.
A malformed query string or base64 data could cause runtime errors. Handling parse failures more gracefully would improve robustness.parse: (value) => { try { - return JSON.parse(decode(value)) as SearchQuery[]; + const decoded = decode(value); + return JSON.parse(decoded) as SearchQuery[]; } catch (error) { console.error('Failed to parse query:', error); return []; } },
139-141: Consider error handling around dynamic filter conversions.
When callingtoQuery(projectId)[format](value), an unexpected format or a failure in the async logic (like forissue) could raise exceptions.tableFilters.map(async ({ condition, format, key, value }) => { try { return { key, value: await toQuery(projectId)[format](value), condition, }; } catch (error) { // Handle gracefully return { key, value: null, condition }; } });apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (2)
70-74: Wrap decoding logic in a try/catch for robustness.
Similar to the other file, unhandled decoding or parsing errors could break the UI.parse: (value) => { try { - return JSON.parse(decode(value)) as SearchQuery[]; + const decoded = decode(value); + return JSON.parse(decoded) as SearchQuery[]; } catch (error) { console.error('Failed to parse query:', error); return []; } },
140-141: Validate async conversions for the feedback converter.
Similar to the issue converter, consider try/catch intoQuery(projectId)[format](value)calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
apps/api/src/common/enums/query-v2-conditions.enum.ts(1 hunks)apps/web/src/entities/dashboard/ui/issue-bar-chart.ui.tsx(1 hunks)apps/web/src/entities/dashboard/ui/issue-feedback-line-chart.ui.tsx(1 hunks)apps/web/src/entities/dashboard/ui/issue-rank.ui.tsx(2 hunks)apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts(7 hunks)apps/web/src/entities/feedback/lib/use-feedback-search.ts(2 hunks)apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx(1 hunks)apps/web/src/entities/feedback/ui/feedback-table-download.ui.tsx(1 hunks)apps/web/src/entities/feedback/ui/issue-cell.tsx(2 hunks)apps/web/src/entities/issue/lib/use-issue-search-infinite.ts(2 hunks)apps/web/src/entities/issue/lib/use-issue-search.ts(2 hunks)apps/web/src/entities/issue/ui/issue-select-box.ui.tsx(1 hunks)apps/web/src/entities/member/ui/member-form-dialog.ui.tsx(1 hunks)apps/web/src/entities/member/ui/member-name-cell.ui.tsx(1 hunks)apps/web/src/entities/user/ui/user-management-table.ui.tsx(2 hunks)apps/web/src/features/create-project/ui/input-members-step.ui.tsx(1 hunks)apps/web/src/shared/types/api.type.ts(3 hunks)apps/web/src/shared/types/index.ts(1 hunks)apps/web/src/shared/types/search-query.type.ts(1 hunks)apps/web/src/shared/ui/category-table-row.ui.tsx(3 hunks)apps/web/src/shared/ui/category-table.ui.tsx(1 hunks)apps/web/src/shared/ui/table-filter-popover/table-filter-popover.type.ts(1 hunks)apps/web/src/shared/ui/table-filter-popover/table-filter-popover.ui.tsx(2 hunks)apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts(8 hunks)apps/web/src/widgets/issue-table/ui/issue-detail-sheet.ui.tsx(4 hunks)apps/web/src/widgets/issue-table/ui/issue-kanban-column.ui.tsx(3 hunks)apps/web/src/widgets/issue-table/ui/issue-kanban.ui.tsx(2 hunks)apps/web/src/widgets/project-settings/ui/member-setting.ui.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (13)
apps/web/src/entities/user/ui/user-management-table.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/widgets/issue-table/ui/issue-detail-sheet.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/widgets/issue-table/ui/issue-kanban-column.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/shared/ui/category-table-row.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/entities/feedback/lib/use-feedback-search.ts (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/entities/issue/lib/use-issue-search.ts (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/widgets/project-settings/ui/member-setting.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/entities/issue/lib/use-issue-search-infinite.ts (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/entities/feedback/ui/feedback-table-download.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts (2)
apps/web/src/shared/ui/table-filter-popover/table-filter-popover.type.ts (1)
TableFilterFieldFotmat(16-16)apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/shared/ui/category-table.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/widgets/issue-table/ui/issue-kanban.ui.tsx (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
🔇 Additional comments (63)
apps/api/src/common/enums/query-v2-conditions.enum.ts (1)
19-19: AddedBETWEENcondition to support range queries.The addition of the
BETWEENcondition in theQueryV2ConditionsEnumwill help support range-based searches, enhancing the query capabilities. This aligns with the PR's objective to improve the feedback issue search functionality.apps/web/src/shared/types/search-query.type.ts (1)
16-20: Well-structured type definition for SearchQueryThe new
SearchQuerytype is clearly defined with appropriate property types:
key: stringfor field identifiersvalue: unknownprovides flexibility while maintaining type-safety (better thanany)conditionas a union type with 'IS', 'CONTAINS', and 'BETWEEN' optionsThis structure aligns well with the PR objective to support the
IScondition for exact match multiselect options.apps/web/src/shared/types/index.ts (1)
24-24: Export addition maintains consistent patternProperly exporting the new
SearchQuerytype from the index file makes it easily accessible throughout the application, following the established pattern of the codebase.apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx (1)
150-152: URL query structure updated to match new SearchQuery typeThe URL construction now correctly uses the
key,value, andconditionstructure for the query parameters instead of the previous dynamic property approach. This ensures consistency with the new query format throughout the application.apps/web/src/entities/issue/ui/issue-select-box.ui.tsx (1)
39-39: Query structure updated to match new SearchQuery typeThe search query now correctly uses the
key,value, andconditionstructure for consistent query formatting throughout the application. This aligns with the new query format and enhances code clarity by explicitly defining what field is being searched.apps/web/src/shared/ui/table-filter-popover/table-filter-popover.type.ts (1)
70-71: Required property enhances type safetyThe change from
value?: unknowntovalue: unknownmakes thevalueproperty required in theTableFilterinterface, which aligns with the PR objective of enforcing explicit field key and value in search parameters. This ensures more consistent data structure across filter operations.apps/web/src/features/create-project/ui/input-members-step.ui.tsx (1)
51-54: Search query structure updated to support IS conditionThe query structure has been updated to use explicit
key,value, andconditionproperties. This enhances the search functionality by clearly specifying that you want an exact match with the IS condition for the GENERAL type.apps/web/src/entities/member/ui/member-name-cell.ui.tsx (1)
29-31: Query format updated to use explicit key-value structureThe query structure has been updated to use explicit
key,value, andconditionproperties. This makes the search criteria more clear and consistent with the new search payload format used throughout the application.apps/web/src/entities/dashboard/ui/issue-feedback-line-chart.ui.tsx (1)
103-108: Issue search query updated to use structured format with CONTAINS conditionThe query structure has been updated to use explicit
key,value, andconditionproperties with a CONTAINS condition for the name field. This provides more flexibility in how searches are performed and maintains consistency with the new search payload format.apps/web/src/entities/dashboard/ui/issue-bar-chart.ui.tsx (1)
56-60: Improved query structure formatThe query structure has been updated to use explicit
key,value, andconditionproperties instead of the previous shorthand format. This change aligns with the PR objective to improve search functionality with explicit query structures.apps/web/src/shared/ui/table-filter-popover/table-filter-popover.ui.tsx (2)
84-84: Initialized value property in filter objectAdded the
value: undefinedproperty to ensure consistent filter object structure. This aligns with the standardized query format using key-value pairs.
99-101: Consistent filter object structure in resetFiltersAdded the
value: undefinedproperty to maintain consistency with the updated filter object format throughout the codebase.apps/web/src/widgets/issue-table/ui/issue-detail-sheet.ui.tsx (4)
38-38: Added SearchQuery type importImported the SearchQuery type to ensure type safety for the query structure changes.
112-117: Updated search query structure with type safetyModified the query structure to use explicit
key,value, andconditionproperties along with type assertion toSearchQuery[]. This improves code clarity and ensures type safety.
227-233: Standardized query format in URL parametersUpdated the URL query parameter structure to use the standardized format with explicit
key,value, andconditionproperties. This ensures consistency in how queries are formatted across the application.
191-191: Updated URL query encoding formatModified the clipboard copy URL to use the new query format with explicit
key,value, andconditionproperties.apps/web/src/entities/member/ui/member-form-dialog.ui.tsx (1)
64-66: Standardized user search query formatUpdated the query structure in the
useUserSearchhook to use explicitkeyandvalueproperties, aligning with the standardized query format across the application.apps/web/src/entities/feedback/ui/feedback-table-download.ui.tsx (2)
32-32: Improved type import for better query structure.This change properly imports the
SearchQuerytype, which defines the structure for query objects withkey,value, andconditionproperties.
40-40: Enhanced type safety with explicit query structure.The type for the
queriesproperty has been updated from a genericRecord<string, unknown>[]to the more specificSearchQuery[]. This aligns with the PR objective of introducing support for theIScondition and ensuring the query structure is clear and type-safe.apps/web/src/widgets/issue-table/ui/issue-kanban.ui.tsx (2)
28-28: Improved type import for better query structure.Importing the
SearchQuerytype provides a well-defined structure for search queries, improving type safety.
39-39: Enhanced type safety with explicit query structure.Updating the type from a generic
Record<string, unknown>[]to the specificSearchQuery[]ensures that all query objects follow the expected structure withkey,value, andconditionproperties. This supports the PR's objective of adding theIScondition for exact match multiselect options.apps/web/src/shared/ui/category-table-row.ui.tsx (3)
34-34: Added import for SearchQuery type.This import supports the improved query structure implementation.
49-49: Enhanced type safety with explicit query structure.The type for the
queriesproperty has been updated from a genericRecord<string, unknown>[]to the more specificSearchQuery[], ensuring consistency in query object structure.
86-88: Implemented IS condition for exact category matching.This is a practical example of using the new
SearchQuerystructure with theIScondition for exact matching. The code correctly adds a category filter to the existing queries using the explicit key-value-condition format.apps/web/src/entities/feedback/lib/use-feedback-search.ts (2)
23-23: Added SearchQuery type import.This import supports the improved query structure implementation.
40-40: Enhanced type safety with explicit query structure.Updating the
queriesproperty type in theIBodyinterface ensures that all search queries conform to the expected structure withkey,value, andconditionproperties. This change maintains consistency with the API expectations and aligns with the PR objective of supporting theIScondition.apps/web/src/entities/user/ui/user-management-table.ui.tsx (2)
29-29: Updated import to include SearchQuery typeThe import statement now includes the
SearchQuerytype, which is used to provide better type safety for the query structure.
61-71: Improved type safety with explicit key-value structureThe code now uses the
SearchQuerytype with explicitkey,value, andconditionproperties, replacing the previous dynamic key approach. This provides better type safety and aligns with theIScondition support for exact match multiselect options.apps/web/src/shared/ui/category-table.ui.tsx (2)
23-23: Added SearchQuery type importAdded import for the
SearchQuerytype from the types module, which is used to provide better type safety for the query structure.
30-30: Enhanced type safety with SearchQuery typeUpdated the
queriesprop type from the genericRecord<string, unknown>[]to the more specificSearchQuery[]type, improving type safety and making the expected data structure more explicit.apps/web/src/widgets/issue-table/ui/issue-kanban-column.ui.tsx (3)
27-27: Updated import to include SearchQuery typeThe import statement now includes the
SearchQuerytype, which is used to provide better type safety for the query structure.
50-50: Enhanced type safety with SearchQuery typeUpdated the
queriesprop type from the genericRecord<string, unknown>[]to the more specificSearchQuery[]type, improving type safety and making the expected data structure more explicit.
74-76: Updated query concatenation to use structured formatModified the query concatenation to use the new
SearchQueryformat with explicitkey,value, andconditionproperties. This change supports theIScondition and provides a more consistent query structure throughout the application.apps/web/src/entities/issue/lib/use-issue-search.ts (2)
19-23: Updated imports to include SearchQuery typeAdded the
SearchQuerytype to the imports, which is used to provide better type safety for the query structure in theIBodyinterface.
36-36: Enhanced type safety with SearchQuery typeUpdated the
queriesproperty in theIBodyinterface from the genericRecord<string, unknown>[]to the more specificSearchQuery[]type. This improves type safety and aligns with the application-wide shift to a more structured query format that supports the newIScondition.apps/web/src/entities/feedback/ui/issue-cell.tsx (2)
71-74: Well-structured query format for feedback search.The change to use explicit
key,value, andconditionproperties in the query object improves code clarity and consistency with the new search API format. TheIScondition correctly enables exact matching for feedback ID.
98-98: Consistent query format for issue search.This modification follows the same pattern of explicit
key,value, andconditionproperties, providing a consistent interface across all search queries in the application.apps/web/src/entities/issue/lib/use-issue-search-infinite.ts (2)
18-22: Good addition of SearchQuery type import.Adding the SearchQuery type to the imports helps enforce type safety for the query structure used in this component.
35-35: Enhanced type safety with SearchQuery array.Changing from a generic
Record<string, unknown>[]to the more specificSearchQuery[]improves type safety and ensures consistent query structure throughout the application. This change aligns with the PR objective of making field keys and values more explicit.apps/web/src/widgets/project-settings/ui/member-setting.ui.tsx (2)
31-31: Appropriate SearchQuery type import.The addition of the SearchQuery type import prepares for its usage in the queries implementation.
72-78: Well-structured query object transformation.The modified implementation now explicitly constructs objects with
key,value, andconditionproperties instead of using dynamic property names. This enhances code readability and ensures consistent query structure throughout the application.apps/web/src/entities/dashboard/ui/issue-rank.ui.tsx (2)
73-73: Consistent query format in issue link.The query structure has been updated to use explicit
key,value, andconditionproperties, which provides consistency with the rest of the application and supports the new search functionality.
134-136: Standardized query structure in issue status filter.The query format has been properly updated to use the explicit
key,value, andconditionproperties. This change contributes to a consistent query structure across the application and supports theIScondition for exact matching.apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts (6)
24-24: Good introduction of theSearchQuerytype.
Using a single, well-defined interface for query objects promotes type-safety and consistency across the codebase.
92-94: Appropriate date range clearing.
Removing thecreatedAtquery if the user clears the date range is straightforward and correct.
99-102: Check appropriateness of'IS'for the new date query.
As above, confirm whether'BETWEEN'is preferable if you intend to capture a baseline date range.
116-120: Effective filtration ofcreatedAtfrom table filters.
Excluding date-based filters in the table UI is consistent with your date handling logic. This approach prevents duplication or conflicting filters.
149-152: Repeated'IS'usage for date range.
Same verification needed for the date filter’s condition when reapplyingcreatedAt.
74-76:❓ Verification inconclusive
Refactor usage of
'IS'for date ranges if needed.
Here, you store{ gte, lt }under the'createdAt'key while usingcondition: 'IS'. Confirm whether'BETWEEN'better represents the intended range semantics.
🌐 Web query:
When using a date range for queries with fields like "createdAt", is it more appropriate to use a "BETWEEN" condition instead of "IS" in your system?💡 Result:
When working with date ranges in SQL queries, the choice between using
BETWEENand comparison operators (>=and<) depends on how date-time values are stored and the specific requirements of your query. Here's a breakdown:Key Considerations for Date Range Queries
Inclusivity of
BETWEEN:
- The
BETWEENoperator is inclusive, meaning it includes both the start and end values in the range. For example:This will include all rows whereWHERE createdAt BETWEEN '2025-03-01' AND '2025-03-25'createdAtis between March 1, 00:00:00 and March 25, 00:00:00. If yourcreatedAtfield includes time components (e.g.,2025-03-25 14:30:00), this query excludes any times after midnight on March 25[1][2][5].Time Component Handling:
- If your date field includes time data (e.g.,
DATETIMEtype),BETWEENcan unintentionally exclude records. For example, entries on March 25 after 00:00:00 will not be included in the above query. Instead, use:This captures all times up to (but not including) March 26, ensuring full coverage of March 25[2][5].WHERE createdAt >= '2025-03-01' AND createdAt < '2025-03-26'Database and Regional Settings:
- Date formats (e.g.,
MM/DD/YYYYvs.YYYY-MM-DD) can affect query accuracy. For consistency, use unambiguous formats likeYYYY-MM-DDand platform-specific delimiters (e.g.,#in Access)[5].- In Microsoft Access, explicit timestamp handling is critical. For example:
ensures the entire day of March 25 is included[4][5].WHERE createdAt BETWEEN #2025-03-01# AND #2025-03-25 23:59:59#When to Use
BETWEENvs. Comparison Operators
Scenario Recommended Approach Dates without time components Use BETWEENfor simplicity[1][3].Dates with time components Use >=and<to avoid cutoff[2][5].Dynamic ranges (e.g., "last week") Combine >=and<with date functions (e.g.,DateAdd()in Access)[4].Cross-database consistency Prefer >=and<to avoid dialect-specific edge cases[2][6].Example Queries
-- Includes March 25 up to 00:00:00 (excludes times): SELECT * FROM orders WHERE order_date BETWEEN '2025-03-01' AND '2025-03-25'; -- Includes all times on March 25: SELECT * FROM orders WHERE order_date >= '2025-03-01' AND order_date < '2025-03-26'; -- Access-specific example with explicit time: SELECT * FROM orders WHERE order_date BETWEEN #2025-03-01# AND #2025-03-25 23:59:59#;Summary
- Use
BETWEENonly if your dates lack time components or if you explicitly set the end value to include the maximum time (e.g.,23:59:59).- Prefer
>=and<for datetime fields to avoid cutoff errors and ensure compatibility across SQL dialects. This approach is more robust when dealing with time-sensitive data[2][5][6].Always validate the date format and time-storage logic of your database system to choose the optimal method.
Citations:
- 1: https://docs.getdbt.com/sql-reference/between
- 2: https://nolongerset.com/gotchas-of-the-sql-between/
- 3: https://www.w3schools.com/sql/sql_between.asp
- 4: https://support.microsoft.com/en-us/office/examples-of-using-dates-as-criteria-in-access-queries-aea83b3b-46eb-43dd-8689-5fc961f21762
- 5: https://forums.ni.com/t5/LabVIEW/SQL-query-between-date-range/td-p/4307250
- 6: https://asktom.oracle.com/pls/apex/asktom.search%3Ftag=date-queries-using-between
- 7: https://forums.mysql.com/read.php?24%2C229613
Action: Verify and adjust the date range condition for 'createdAt'.
It appears that you’re currently storing a range (i.e.,{ gte, lt }) under the'createdAt'key while applying thecondition: 'IS', which is typically used for single-value equality. Based on common SQL practices:
- If the
createdAtfield contains only date values, aBETWEENcondition can clearly express an inclusive date range.- If
createdAtincludes time components, using separated operators (i.e.,>=and<) is safer to avoid cutoff errors.Please review the underlying database schema and the intended query semantics, then adjust the condition accordingly.
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (10)
24-24: Good introduction ofSearchQueryhere as well.
Ensures consistency with the new structured query type used across the codebase.
43-43: Validate'IS'usage for the'name'condition.
While'IS'might be correct for exact name matches, confirm consistency when other fields might require'CONTAINS'or'BETWEEN'.
80-80: Thoughtful defaulting withcreatedAt.
Providing a default date range query can improve user experience if searching by date is a common scenario.
85-87: Consistent approach to extracting date range queries.
Using.find((v) => v.key === 'createdAt')aligns with the newSearchQuerystruct’skeyproperty.
100-100: Correctly removingcreatedAtfrom queries.
The filter approach matches the code in the other converter file. This maintains parity and clarity.
105-106: Re-adding the date query: verify'IS'usage for range.
As previously stated, ensure that'IS'accurately reflects your date range semantics.
118-120: Offsetting date-based queries from other table filters.
SkippingcreatedAtensures it does not conflict with the standard filter logic.
146-148: Graceful fallback to default date query.
When all queries are removed, automatically re-adding'createdAt'ensures coverage. Nicely done.
156-159: Same'IS'vs'BETWEEN'question for date ranges.
Watch out for the range semantics.
176-176: Good defensive filtering of extraneous queries.
Ensuring that only recognizedfilterFieldsandcreatedAtremain fosters a cleaner query state.apps/web/src/shared/types/api.type.ts (4)
1048-1048: Useful enumerated conditions.
Defining'CONTAINS' | 'IS' | 'BETWEEN'clarifies query usage across the codebase.
1050-1052: StreamlinedUserSearchQuerystructure.
Switching to key/value/condition fosters consistency and standardization in user search queries.
1234-1237: RefinedQueryV2with optionalids.
Makingidsoptional while enforcingkey,value, andconditionis a solid approach for flexible queries.
1348-1350: Alignment ofMemberSearchQuerywith the new pattern.
All data now follows a uniform shape, making the code more maintainable.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (1)
60-91: 🛠️ Refactor suggestionImprove the issueIdsToFeedbackIds method by eliminating duplicate database queries.
The method performs two consecutive database queries to fetch the same data - once to get feedbacks by issue IDs and then again to get feedbacks by the previously fetched feedback IDs. This is inefficient and could potentially impact performance.
Consider simplifying this to a single query:
private async issueIdsToFeedbackIds( issueIds: number[], condition: QueryV2ConditionsEnum = QueryV2ConditionsEnum.CONTAINS, ) { - let feedbacks = await this.feedbackRepository.find({ - relations: ['issues'], - where: { - issues: { id: In(issueIds) }, - }, - }); - - feedbacks = await this.feedbackRepository.find({ - relations: ['issues'], - where: { - id: In(feedbacks.map((feedback) => feedback.id)), - }, - }); + const feedbacks = await this.feedbackRepository.find({ + relations: ['issues'], + where: { + issues: { id: In(issueIds) }, + }, + }); if (condition === QueryV2ConditionsEnum.IS) { return feedbacks .filter((feedback) => { const feedbackIssueIds = feedback.issues.map((issue) => issue.id); return ( feedbackIssueIds.length === issueIds.length && issueIds.every((id) => feedbackIssueIds.includes(id)) ); }) .map((feedback) => feedback.id); } else { return feedbacks.map((feedback) => feedback.id); } }
🧹 Nitpick comments (1)
apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts (1)
22-36: Good update to the QueryV2 class with proper validation.The changes to the
QueryV2class properly implement the new query structure with explicitkeyandvalueproperties, along with appropriate validation decorators. This enhances both type safety and API documentation.One thing to note: The example in
@ApiPropertydecorator forqueries(line 42) still shows the old format{ name: 'issue name' }. Consider updating it to reflect the new structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/src/domains/admin/feedback/feedback.os.service.ts(7 hunks)apps/api/src/domains/admin/project/issue/dtos/find-issues-by-project-id-v2.dto.ts(1 hunks)apps/api/src/domains/admin/project/issue/dtos/requests/find-issues-by-project-id-request-v2.dto.ts(1 hunks)apps/api/src/domains/admin/project/issue/issue.service.ts(2 hunks)
🔇 Additional comments (8)
apps/api/src/domains/admin/project/issue/dtos/find-issues-by-project-id-v2.dto.ts (1)
25-28: Great job standardizing the query structure with explicit key-value pairs!The refactoring improves type safety by explicitly defining the query structure with
keyandvalueproperties, which aligns with the PR objective of enhancing the search functionality. This change will make the code more maintainable and prevent potential runtime errors from dynamic property access.apps/api/src/domains/admin/project/issue/issue.service.ts (3)
185-192: Clean implementation of the refactored query structure.The change properly adapts to the new query structure by checking for
query.key === 'createdAt'instead of directly accessing properties, maintaining consistency with the DTO changes.
194-205: Good handling of the categoryId condition.The implementation includes a proper type check and special handling for the categoryId value of 0. This is a robust approach that prevents potential runtime errors.
213-252: Well-implemented support for the IS condition.The code now properly handles the
IScondition forstatusandexternalIssueIdfields, as well as a more flexible handling forname,description, andcategoryfields based on the condition type. This directly implements the PR objective of supporting exact match queries.apps/api/src/domains/admin/feedback/feedback.os.service.ts (4)
276-286: Good handling of the createdAt condition with the new query structure.The code properly adapts to the new query structure by checking for
query.key === 'createdAt'and accessing the value correctly.
328-380: Excellent implementation of the IS condition for multiSelect fields.The implementation correctly handles the exact match requirement for the
IScondition using a combination ofterms_setand script queries in OpenSearch. This ensures that the field contains exactly the specified values (no more, no less), directly addressing the PR objective of supporting exact match multiselect options.
508-513: Good update to pass condition to issueIdsToFeedbackIds in findByChannelIdV2.The code now properly passes the condition to the
issueIdsToFeedbackIdsmethod, ensuring consistent behavior based on whether the condition isISorCONTAINS.
599-604: Good update to pass condition to issueIdsToFeedbackIds in scrollV2.Similar to the update in
findByChannelIdV2, the code now properly passes the condition to theissueIdsToFeedbackIdsmethod in thescrollV2method as well, ensuring consistent behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/src/shared/ui/tables/draggable-row.ui.tsx (1)
57-65: Great improvement to the TableCell className logic.This change enhances the component's flexibility by allowing column-specific control over text truncation. The implementation correctly uses the
cnutility for conditional class name composition and ensuresoverflow-hiddenis always applied, while making thetruncateclass configurable through column metadata.Consider simplifying the conditional logic using optional chaining:
className={cn('overflow-hidden', { truncate: - ( - cell.column.columnDef.meta && - 'truncate' in cell.column.columnDef.meta - ) ? - cell.column.columnDef.meta.truncate !== false - : true, + cell.column.columnDef.meta?.truncate !== false, })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/package.json(1 hunks)apps/web/src/entities/feedback/feedback-table-columns.tsx(1 hunks)apps/web/src/entities/field/field-columns.tsx(4 hunks)apps/web/src/entities/member/member-columns.tsx(2 hunks)apps/web/src/entities/user/user-columns.tsx(5 hunks)apps/web/src/shared/ui/tables/draggable-row.ui.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (14)
apps/web/package.json (1)
9-9: Inclusion of Turbopack in the Dev CommandThe updated dev script
"next dev --turbopack"is clear and should improve development build performance. Please verify that your Next.js version (^15.0.0) fully supports Turbopack and that no additional configuration changes (e.g., in next.config.js) are needed to avoid unexpected issues.apps/web/src/entities/feedback/feedback-table-columns.tsx (1)
115-121: Improve table column display with metadata configurationThe added
metaproperty with conditional truncation setting improves the display behavior of different field formats. By explicitly specifying that select, multiSelect, and images formats shouldn't be truncated, their content will be displayed fully, enhancing readability for these specific field types.apps/web/src/entities/field/field-columns.tsx (4)
41-41: Consistent metadata for drag handle column displayAdding
meta: { truncate: false }ensures that the drag handle UI element isn't truncated in the table, improving usability.
73-73: Improved options column displaySetting
truncate: falsefor the options column ensures that the option list popover displays properly without being cut off, enhancing the user experience.
86-86: Better property tag displayAdding
meta: { truncate: false }to the property column ensures that the Tag component displays completely without truncation.
99-99: Enhanced status badge visibilitySetting
truncate: falsefor the status column ensures that status badges remain fully visible in the table.apps/web/src/entities/user/user-columns.tsx (5)
22-22: Removed unused importThe
displayStringfunction has been removed from imports as it's no longer used in this file.
48-48: Improved checkbox column displayAdding
meta: { truncate: false }ensures that the checkbox UI element displays properly.
72-73: Enhanced department display with badge componentThe department cell rendering has been improved to use a Badge component, and
truncate: falseensures it displays properly. This change creates visual consistency with other badge-based columns.Also applies to: 76-76
88-88: Better type badge visibilityAdding
meta: { truncate: false }to the type column ensures that type badges display properly without truncation.
109-109: Enhanced project badges displaySetting
truncate: falsefor the members column ensures that project badges display completely, improving readability.apps/web/src/entities/member/member-columns.tsx (3)
48-48: Improved checkbox column displayAdding
meta: { truncate: false }ensures that the checkbox UI element displays properly in the table.
73-73: Better department badge displaySetting
truncate: falsefor the department column ensures that department badges display completely without truncation.
78-78: Enhanced role badge visibilityAdding
meta: { truncate: false }to the role column ensures that role badges remain fully visible in the table.
… multi-select functionality
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (4)
33-74: Typo in function parameter nameThere's a typo in the
toQueryfunction parameter:valeushould bevalue.-const toQuery = ( - projectId: number, -): Record<TableFilterFieldFotmat, (valeu: unknown) => unknown> => ({ +const toQuery = ( + projectId: number, +): Record<TableFilterFieldFotmat, (value: unknown) => unknown> => ({
21-28: Typo in type nameThere's a typo in the imported type name:
TableFilterFieldFotmatshould beTableFilterFieldFormat.
152-186: Typo in function nameThere's a typo in the function name:
onChageTableFiltersshould beonChangeTableFilters.
189-201: Typo in function name in returned objectIn the returned object,
updateDateRageshould beupdateDateRange.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
apps/web/package.json(1 hunks)apps/web/src/entities/dashboard/ui/issue-bar-chart.ui.tsx(1 hunks)apps/web/src/entities/dashboard/ui/issue-rank.ui.tsx(2 hunks)apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts(7 hunks)apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx(1 hunks)apps/web/src/entities/issue/ui/issue-select-box.ui.tsx(1 hunks)apps/web/src/shared/ui/inputs/index.ts(1 hunks)apps/web/src/shared/ui/inputs/multi-select-search-input.ui.tsx(1 hunks)apps/web/src/shared/ui/table-filter-popover/table-filter-popover-input.ui.tsx(1 hunks)apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts(7 hunks)apps/web/src/widgets/issue-table/ui/issue-detail-sheet.ui.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/web/package.json
- apps/web/src/widgets/issue-table/ui/issue-detail-sheet.ui.tsx
- apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx
- apps/web/src/entities/dashboard/ui/issue-bar-chart.ui.tsx
- apps/web/src/entities/dashboard/ui/issue-rank.ui.tsx
- apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
🪛 Biome (1.9.4)
apps/web/src/shared/ui/inputs/multi-select-search-input.ui.tsx
[error] 81-81: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (19)
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (8)
21-28: Type import enhances code clarity and type safetyThe addition of the
SearchQuerytype import is a positive change that provides better type safety for the query objects throughout the file.
38-67: Successfully implemented IS condition for issue searchThe implementation correctly uses the new
SearchQuerystructure with explicitkey,value, andconditionproperties. The 'IS' condition allows for exact matching in issue searches, which aligns with the PR objectives.
90-98: Type safety improved for query state managementThe
useQueryStatehook now correctly uses theSearchQuery[]type and provides a properly structured default value. This ensures type consistency throughout the component.
101-103: Query structure update for date range handlingThe date range logic has been appropriately updated to work with the new query structure by using
keyproperty for identification instead of direct property access.
115-129: Date range update logic maintains structure consistencyThe function correctly filters out existing date queries using the
keyproperty and adds a new one with the proper structure, maintaining consistency with the newSearchQueryformat.
131-150: Table filters logic correctly adapted to new query structureThe table filters logic has been properly updated to extract properties using the new structure. The code is more explicit and easier to understand with the key-value approach.
152-187: Consistent implementation of table filter changesThe function for changing table filters has been consistently updated to work with the new query structure, ensuring that all query manipulations follow the same pattern throughout the component.
189-193: Return filter logic properly updatedThe query filtering in the return statement has been updated to use the
keyproperty, maintaining consistency with the rest of the changes.apps/web/src/shared/ui/inputs/multi-select-search-input.ui.tsx (3)
37-49: Good implementation of Props interface with optional properties.The interface provides a well-structured set of props with appropriate types, making the component flexible and reusable. The use of optional properties (marked with
?) allows for more versatile usage patterns.
97-116: Well-implemented option selection logic.The component correctly handles the addition and removal of selected items by checking if they already exist in the value array. The rendering of the component is optimized with appropriate checks for selected state.
118-121: Good implementation of infinite scrolling.The integration with the
InfiniteScrollAreacomponent allows for efficient loading of large datasets, enhancing the user experience when dealing with many options.apps/web/src/shared/ui/inputs/index.ts (1)
21-21: Good modular approach for exporting the new component.Exporting the
MultiSelectSearchInputfrom the barrel file makes it easily accessible alongside other input components, following the project's established pattern.apps/web/src/shared/ui/table-filter-popover/table-filter-popover-input.ui.tsx (1)
194-194: Improved handling of issue selection values.The code now properly handles multiple issue selections by ensuring the value is always treated as a string array. The nullish coalescing operator provides an empty array as a fallback when value is null or undefined.
apps/web/src/entities/issue/ui/issue-select-box.ui.tsx (6)
21-21: Good use of imports for the new MultiSelectSearchInput component.The import statement correctly brings in the new component that supports multiple selections.
25-28: Props interface updated to support multiple selections.The interface has been correctly modified to handle arrays of strings instead of single string values, which aligns with the new multi-select functionality.
33-33: State variable renamed to reflect plural nature.The state variable was appropriately renamed from
issueNametoissueNamesto better represent its new purpose as an array of names.
39-39: Query structure updated to use explicit key-value format.The search query now follows the structured format with
key,value, andconditionproperties, which aligns with the PR objective to enhance clarity in search parameters.
48-59: Improved issue name fetching with Promise.all.The code now efficiently fetches multiple issue names in parallel using Promise.all, properly handling the asynchronous nature of the API calls.
62-74: Updated to use MultiSelectSearchInput component.The component now correctly uses the new MultiSelectSearchInput instead of the previous SelectSearchInput, passing the appropriate props for multiple selection functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/web/src/shared/ui/inputs/multi-select-search-input.ui.tsx (5)
37-49: Props interface looks good, but consider default value handlingThe Props interface is well-defined with appropriate types. The
valueprop is defined asstring[] | null, which requires careful handling in the component logic.Consider providing a default empty array for
valueto simplify the component logic:interface Props { label?: string; - value: string[] | null; + value: string[]; onChange: (value: string[]) => void; options: { label: string; value: string }[]; required?: boolean; disabled?: boolean; error?: string; fetchNextPage?: () => void; hasNextPage?: boolean; inputValue?: string; setInputValue?: (value: string) => void; }
95-95: Translate hardcoded error messageThe "No results found" message is hardcoded in English instead of using the translation function.
Use the translation function for consistency:
- <ComboboxEmpty>No results found.</ComboboxEmpty> + <ComboboxEmpty>{t('v2.common.noResultsFound', 'No results found.')}</ComboboxEmpty>
98-109: Improve type safety and simplify the selection logicThe current implementation has some unnecessary optional chaining and could be simplified.
Simplify the selection logic:
{options.map((option) => { - const isChecked = value?.some((v) => v === option.value); + const isChecked = value ? value.includes(option.value) : false; return ( <ComboboxSelectItem key={option.value} value={option.value} checked={isChecked} onSelect={() => { onChange( - isChecked ? - (value?.filter((v) => v !== option.value) ?? []) - : [...(value ?? []), option.value], + isChecked + ? (value || []).filter((v) => v !== option.value) + : [...(value || []), option.value], ); setOpen(false); }} >
118-121: Check if fetchNextPage is defined before passing to InfiniteScrollAreaThe
fetchNextPageprop is optional but passed directly to theInfiniteScrollAreacomponent, which might cause issues if it's undefined.Add a conditional check:
<InfiniteScrollArea hasNextPage={hasNextPage} - fetchNextPage={fetchNextPage} + fetchNextPage={fetchNextPage ?? (() => {})} />
70-127: Consider adding ARIA attributes for better accessibilityThe component could benefit from additional accessibility attributes to improve usability for screen readers.
For example, you could add
aria-label,aria-required, andaria-invalidattributes to the appropriate elements to improve accessibility.<Combobox open={open} onOpenChange={setOpen}> - <ComboboxTrigger disabled={disabled} className="font-normal"> + <ComboboxTrigger + disabled={disabled} + className="font-normal" + aria-required={required || false} + aria-invalid={!!error} + aria-label={label || t('v2.common.select')} + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/package.json(0 hunks)apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts(7 hunks)apps/web/src/entities/feedback/ui/feedback-cell.tsx(1 hunks)apps/web/src/entities/feedback/ui/feedback-detail-cell.ui.tsx(1 hunks)apps/web/src/shared/ui/inputs/multi-select-search-input.ui.tsx(1 hunks)apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts(8 hunks)tooling/eslint/react.js(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/widgets/issue-table/lib/use-issue-query-converter.ts
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (1)
apps/web/src/shared/types/search-query.type.ts (1)
SearchQuery(16-20)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (11)
tooling/eslint/react.js (1)
18-18: Great addition of thereact/jsx-keyrule!Setting the
react/jsx-keyrule to 'error' enforces a critical React best practice by requiring unique key props on JSX elements in arrays and iterators. This improves rendering performance and helps React efficiently identify changes in list items, which aligns well with the PR's goal of enhancing query parameter structure with explicit key-value pairs.apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (8)
22-24: Type import enhancement for query structureGood addition of the
SearchQuerytype import. This enhances type safety by providing a clear structure for query objects.
38-67: Refactored issue search to use explicit key-value structureThe refactoring of the issue search functionality now properly implements the
IScondition with explicitkeyandvalueproperties in the query structure. This aligns with the PR objective of supporting exact match multiselect options.
90-97: Updated query state with type-safe structureGood update to the query state definition. The typed
SearchQuery[]provides better type safety than the previousRecord<string, unknown>[]and the default query now uses the explicit key-value structure.
100-113: Updated dateRange calculation with key-based accessThe dateRange calculation has been properly updated to use the key-based query structure. This ensures consistency with the new query format.
115-129: Refactored date range handling to use key-value structureThe date range handling now correctly implements the key-value structure in the query objects, maintaining the same functionality while using the new format.
132-150: Updated tableFilters logic to use key-value structureThe tableFilters calculation has been properly modified to extract key and value from the new query structure, maintaining consistency throughout the codebase.
152-187: Refactored table filter update logicThe table filter update logic has been properly updated to use the key-value structure, ensuring that all parts of the code work together consistently with the new query format.
189-204: Updated return structure with consistent filteringThe return object's filtering logic has been updated to use key-based property access, ensuring consistent behavior with the new query structure throughout the component.
apps/web/src/entities/feedback/ui/feedback-cell.tsx (1)
58-59: Added key prop to Badge componentGood addition of the
keyprop to the Badge component. This follows React's best practices for list rendering and helps React efficiently update the DOM when the list changes.apps/web/src/entities/feedback/ui/feedback-detail-cell.ui.tsx (1)
50-51: Added key prop to Badge componentGood addition of the
keyprop to the Badge component. This follows React's best practices for list rendering and helps React efficiently update the DOM when the list changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (1)
528-543: 🛠️ Refactor suggestionInconsistent query properties usage
While most of the codebase has migrated to a structured
key/valueapproach, this section still uses theidsproperty to store feedback IDs after processing. This creates inconsistency and potential confusion.if (queries?.[i].key === 'issueIds') { const condition = queries[i].condition; const feedbackIds = await this.issueIdsToFeedbackIds( queries[i].value as number[], condition, ); - if (queries[i].ids) { - queries[i].ids = [...(queries[i].ids ?? []), ...feedbackIds]; - } else { - queries[i].ids = feedbackIds; - } + queries[i].value = feedbackIds; + queries[i].key = 'ids'; }Apply the same pattern in the
scrollV2method (lines 619-634).
♻️ Duplicate comments (1)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (1)
293-310:⚠️ Potential issueAction Required: Update
issueIdsFilter Handling in OS Query BuilderIn this block, you're using
query.idsinstead ofquery.value—inconsistent with the refactored query structure which useskeyandvalueproperties.-if (fieldKey === 'issueIds' && query.ids) { +if (fieldKey === 'issueIds' && query.value) { osQuery.bool.must.push({ ids: { - values: query.ids.map((id) => id.toString()), + values: query.value.map((id) => id.toString()), }, }); // ...
🧹 Nitpick comments (1)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (1)
78-90: The IS condition implementation needs test coverage.The new filter logic for the
IScondition looks good, but it introduces complex operations that deserve comprehensive testing. Edge cases to verify include:
- Empty arrays
- Cases where all issue IDs match but feedback has additional issue IDs
- Feedbacks with no issue IDs
- Performance with large datasets
Consider implementing unit tests to verify this implementation works correctly for all possible scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/domains/admin/feedback/feedback.mysql.service.ts(3 hunks)apps/api/src/domains/admin/feedback/feedback.os.service.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/domains/admin/feedback/feedback.mysql.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-test
🔇 Additional comments (3)
apps/api/src/domains/admin/feedback/feedback.os.service.ts (3)
64-76: Avoid multiple queries for the same data.Currently, the code performs two consecutive database lookups on the same feedback IDs. This doubles the overhead. Consider combining these queries into one or refactoring to reduce redundant calls.
328-402: Well-implemented query conditions for multi-select fieldsThe implementation for both
ISandCONTAINSconditions is comprehensive and handles all cases properly, including empty arrays. The script-based approach ensures accurate matching of exact sets for theIScondition and proper containment for theCONTAINScondition.
60-91: The flexibility added for query conditions is a good enhancementAdding support for the
IScondition in theissueIdsToFeedbackIdsmethod improves the query capabilities by allowing for exact matches on feedback issues. This aligns well with the PR objectives of enhancing search functionality.
chiol
changed the title
IScondition in feedback issue search (exact match in multiselect field)Summary by CodeRabbit
Summary by CodeRabbit
New Features
MultiSelectSearchInputcomponent for improved multi-selection capabilities.Refactor
createdAtcondition in query processing logic for users and feedback.SearchQuerytype for better type safety in query handling.BETWEENto the query conditions enumeration.