Change slug usage to encoded text #261
Conversation
WalkthroughRecent changes in the project enhance name handling and conflict detection in backend services, significantly improving support for non-English characters. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateModal
participant UsersService
participant WorkspacesService
User->>CreateModal: Submit Workspace
CreateModal->>UsersService: Validate Nickname & Title
UsersService-->>CreateModal: Validation Result
CreateModal->>WorkspacesService: Create Workspace
WorkspacesService-->>CreateModal: Workspace Created
CreateModal->>User: Display Success Message
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (6)
- backend/package.json (1 hunks)
- backend/src/check/check.service.ts (2 hunks)
- backend/src/users/users.service.ts (3 hunks)
- backend/src/workspaces/workspaces.controller.ts (3 hunks)
- backend/src/workspaces/workspaces.service.ts (2 hunks)
- frontend/src/components/modals/CreateModal.tsx (5 hunks)
Files skipped from review due to trivial changes (1)
- backend/package.json
Additional comments not posted (9)
backend/src/users/users.service.ts (2)
97-97: LGTM! Verify the usage ofencodedTextin the codebase.The change from
slugifytoencodeURIComponentensures proper encoding of special characters.However, ensure that all occurrences of
encodedTextare correctly used in database operations.Verification successful
LGTM!
The change from
slugifytoencodeURIComponentensures proper encoding of special characters. TheencodedTextvariable is correctly used in various database operations.
- Lines involving
encodedText:
slug: encodedTextOR: [{ nickname: name }, { nickname: encodedText }]OR: [{ title: name }, { title: encodedText }]Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all occurrences of `encodedText` in the codebase. # Test: Search for the variable usage. Expect: Proper usage in database operations. rg --type typescript -A 5 $'encodedText'Length of output: 74
Script:
#!/bin/bash # Description: Verify all occurrences of `encodedText` in the codebase. # Find all TypeScript files and search for the variable usage. Expect: Proper usage in database operations. fd --type f --extension ts --exec rg 'encodedText' {}Length of output: 287
106-106: LGTM! Verify the workspace creation logic.The change ensures that workspace slugs are correctly encoded using
encodedText.However, ensure that the workspace creation logic handles the encoded text properly.
Verification successful
Verified: The
slugis correctly assignedencodedTextin thebackend/src/users/users.service.tsfile, ensuring proper handling of the encoded text in the workspace creation logic.
backend/src/users/users.service.ts:slug: encodedTextScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the workspace creation logic for handling `encodedText`. # Test: Search for the workspace creation logic. Expect: Proper handling of `encodedText`. rg --type typescript -A 5 $'this.prismaService.workspace.create'Length of output: 98
Script:
#!/bin/bash # Description: Verify the workspace creation logic for handling `encodedText`. # Test: Search for the workspace creation logic. Expect: Proper handling of `encodedText`. rg --type ts -A 5 $'this.prismaService.workspace.create'Length of output: 818
Script:
#!/bin/bash # Description: Verify the `slug` assignment in `backend/src/users/users.service.ts`. # Test: Search for `slug` assignment in `backend/src/users/users.service.ts`. Expect: Proper handling of `encodedText`. rg --type ts -A 5 'slug' backend/src/users/users.service.tsLength of output: 447
backend/src/check/check.service.ts (1)
18-18: LGTM! Verify the usage ofencodedTextin the conflict checking logic.The change from
slugifytoencodeURIComponentensures proper encoding of special characters.However, ensure that all occurrences of
encodedTextare correctly used in the conflict checking logic.Verification successful
LGTM! Verify the usage of
encodedTextin the conflict checking logic.The change from
slugifytoencodeURIComponentensures proper encoding of special characters.
backend/src/check/check.service.ts:
- Line 18:
const encodedText = encodeURIComponent(name);- Line 21:
OR: [{ nickname: name }, { nickname: encodedText }];- Line 26:
OR: [{ title: name }, { title: encodedText }];The occurrences of
encodedTextare correctly used in the conflict checking logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all occurrences of `encodedText` in the conflict checking logic. # Test: Search for the variable usage. Expect: Proper usage in conflict checking logic. rg --type typescript -A 5 $'encodedText'Length of output: 74
Script:
#!/bin/bash # Description: Verify all occurrences of `encodedText` in the conflict checking logic. # Test: Search for the variable usage. Expect: Proper usage in conflict checking logic. rg --type ts -A 5 $'encodedText'Length of output: 1681
frontend/src/components/modals/CreateModal.tsx (4)
111-111: LGTM! Verify the button'sdisabledproperty logic.The change ensures that the button is disabled if either
errorMessageis present or the nickname length is less than 2.However, ensure that the button's
disabledproperty logic is consistent across the component.Verification successful
The button's
disabledproperty logic is consistent.The
disabledproperty is correctly set based on the presence oferrorMessageor if thenicknamelength is less than 2. This ensures that the button is disabled under the intended conditions.
- Verified that the logic is centralized and consistent within the
CreateModal.tsxfile.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the button's `disabled` property logic in the component. # Test: Search for button's `disabled` property logic. Expect: Consistent handling of `disabled` property. rg --type tsx -A 5 $'disabled'Length of output: 57
Script:
#!/bin/bash # Description: Verify the button's `disabled` property logic in the CreateModal.tsx component. # Test: Search for button's `disabled` property logic in the CreateModal.tsx file. Expect: Consistent handling of `disabled` property. rg 'disabled' frontend/src/components/modals/CreateModal.tsx -A 5Length of output: 209
56-59: LGTM! Verify the title length logic.The change ensures that the title must have at least 2 characters before proceeding.
However, ensure that the title length logic is consistent across the component.
Verification successful
Verification Successful: Title length logic is consistent.
The title length check is only present in the
CreateModal.tsxfile, ensuring that the logic is not duplicated or inconsistent elsewhere in the codebase.
frontend/src/components/modals/CreateModal.tsx:if (data.title.length >= 2) { ... }Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the title length logic in the component. # Test: Search for title length logic. Expect: Consistent title length handling. rg --type tsx -A 5 $'title.length'Length of output: 61
Script:
#!/bin/bash # Description: Verify the title length logic in the component. # Test: Search for title length logic. Expect: Consistent title length handling. rg --type ts -A 5 $'title.length'Length of output: 404
Line range hint
61-63: LGTM! Verify the nickname state logic.The change ensures that the nickname state is updated correctly.
However, ensure that the nickname state logic is consistent across the component.
Verification successful
Let's generate a new shell script to search for the
setNicknamefunction in the TypeScript component files without specifying the file type.
The nickname state logic is consistent.
The
setNicknamefunction is used appropriately to update thenicknamestate across various components, includingCreateModal.tsx.
frontend/src/components/modals/CreateModal.tsxfrontend/src/components/modals/ChangeNicknameModal.tsxfrontend/src/pages/settings/profile/Index.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the nickname state logic in the component. # Test: Search for nickname state logic. Expect: Consistent nickname state handling. rg --type tsx -A 5 $'setNickname'Length of output: 60
Script:
#!/bin/bash # Description: Verify the nickname state logic in the component. # Test: Search for nickname state logic. Expect: Consistent nickname state handling. rg 'setNickname' -A 5Length of output: 3642
34-36: LGTM! Verify the error message logic.The change ensures that the nickname must have at least 2 characters before proceeding.
However, ensure that the error message logic is consistent across the component.
Verification successful
Let's re-run the verification script by specifying the file extension directly to ensure we capture the error message logic.
To fully understand and verify the error message logic, we need to extract the complete
useMemohooks whereerrorMessageis being computed.
Verification Successful: Error Message Logic Consistent
The error message logic in
CreateModal.tsxis consistent and correctly checks for both the nickname length and conflict conditions.
frontend/src/components/modals/CreateModal.tsx: TheuseMemohook forerrorMessageensures that the nickname must be at least 2 characters long and checks for any conflicts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error message logic in the component. # Test: Search for error message logic. Expect: Consistent error message handling. rg --type tsx -A 5 $'errorMessage'Length of output: 61
Script:
#!/bin/bash # Description: Verify the error message logic in the component. # Find all .tsx files and search for errorMessage logic within those files. fd -e tsx -x rg 'errorMessage' {}Length of output: 628
Script:
#!/bin/bash # Description: Extract the complete useMemo hooks for errorMessage computation. # Search for useMemo hooks and include surrounding lines for context. rg 'useMemo' -A 10 -B 2Length of output: 31877
backend/src/workspaces/workspaces.service.ts (1)
33-33: LGTM! Verify the impact on URL routing and workspace identification.The change to use
encodeURIComponentfor generating slugs enhances compatibility with special characters. Ensure that this change does not introduce any issues with URL routing or workspace identification.backend/src/workspaces/workspaces.controller.ts (1)
71-71: LGTM! Verify the impact on URL routing and workspace identification.The change to use
encodeURIComponentfor theworkspaceSlugparameter enhances compatibility with special characters. Ensure that this change does not introduce any issues with URL routing or workspace identification.
There was a problem hiding this comment.
And we have to add validation check in CreateWorkspaceDto and ChangeNicknameDto.
Validation Docs: https://docs.nestjs.com/techniques/validation
| @@ -102,7 +108,7 @@ function CreateModal(props: CreateModalProps) { | |||
| type="submit" | |||
| variant="contained" | |||
| size="large" | |||
| disabled={Boolean(errorMessage)} | |||
| disabled={Boolean(errorMessage) || nickname.length < 2} | |||
There was a problem hiding this comment.
This is redundant.
When nickname.length < 2, errorMessage already exists.
There was a problem hiding this comment.
Thanks. I think it's right to check that the error message exists.
Could you let me know which parts need validation? |
For workspace title and user nickname should be checked both frontend in backend. For example, in @ApiProperty({ description: "Title of project to create", type: String, maxLength: 2 })
@MinLength(2)
title: string;(It also should be applied to swagger.) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- backend/src/users/dto/change-nickname.dto.ts (1 hunks)
- backend/src/workspaces/dto/create-workspace.dto.ts (1 hunks)
- frontend/src/components/modals/CreateModal.tsx (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/modals/CreateModal.tsx
Additional comments not posted (4)
backend/src/workspaces/dto/create-workspace.dto.ts (2)
2-2: LGTM! Import statement is necessary.The
MinLengthdecorator from theclass-validatorlibrary is correctly imported to enforce the minimum length constraint.
5-6: LGTM! Enhanced validation and documentation for thetitleproperty.The
@ApiPropertydecorator now includes theminLengthattribute, and the@MinLength(2)decorator ensures the title has a minimum length of 2 characters.backend/src/users/dto/change-nickname.dto.ts (2)
2-2: LGTM! Import statement is necessary.The
MinLengthdecorator from theclass-validatorlibrary is correctly imported to enforce the minimum length constraint.
5-6: LGTM! Enhanced validation and documentation for thenicknameproperty.The
@ApiPropertydecorator now includes theminLengthattribute, and the@MinLength(2)decorator ensures the nickname has a minimum length of 2 characters.
|
Nice improvement @minai621! 👍👍 |
* Chore remove slugify * Feat change slug to encoding text * Feat decoded workspace_slug to encoding back * Feat Add validate title length in CreateModal * Refactor remove duplicated error check * Feat update validation for nickname and title length
What this PR does / why we need it:
This PR changes the method of handling text from using slugs to using encoded text. The previous slug generation method did not support all languages and returned empty strings when special characters like emojis were included. By switching to encoded text, we ensure compatibility with any type of text input.
Which issue(s) this PR fixes:
Fixes #209
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor