feat: text input value trimming #57

Merged
matous-volf merged 1 commits from feat/text-input-value-trimming into main 2024-09-19 18:47:58 +00:00
matous-volf commented 2024-09-19 18:42:09 +00:00 (Migrated from github.com)

Summary by CodeRabbit

  • New Features
    • Enhanced input validation for project, subtask, and task titles by trimming whitespace.
  • Bug Fixes
    • Improved data integrity by sanitizing titles to prevent errors related to leading or trailing spaces.
  • Refactor
    • Updated the edit_task function to accept mutable references for in-place modifications.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced input validation for project, subtask, and task titles by trimming whitespace. - **Bug Fixes** - Improved data integrity by sanitizing titles to prevent errors related to leading or trailing spaces. - **Refactor** - Updated the `edit_task` function to accept mutable references for in-place modifications. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
coderabbitai[bot] commented 2024-09-19 18:42:24 +00:00 (Migrated from github.com)

Walkthrough

The changes involve modifications to the create_project, edit_project, create_subtask, edit_subtask, create_task, and edit_task functions across three files. Each of these functions now includes a step to sanitize the title field by trimming whitespace, ensuring that leading or trailing spaces are removed before validation. Additionally, the edit_task function's signature has been updated to accept a mutable reference to allow in-place modifications. These updates aim to enhance input handling and data integrity.

Changes

File Change Summary
src/server/projects.rs Updated create_project and edit_project to sanitize the title field by trimming whitespace.
src/server/subtasks.rs Updated create_subtask and edit_subtask to sanitize the title field by trimming whitespace.
src/server/tasks.rs Updated create_task and edit_task to sanitize the title field by trimming whitespace; modified edit_task to accept a mutable reference.
  • #39: The changes in src/server/tasks.rs related to the addition of the delete_task function are relevant because they involve modifications to the same file as the main PR, which also includes changes to the edit_task function in that file.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.
<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- walkthrough_start --> ## Walkthrough The changes involve modifications to the `create_project`, `edit_project`, `create_subtask`, `edit_subtask`, `create_task`, and `edit_task` functions across three files. Each of these functions now includes a step to sanitize the `title` field by trimming whitespace, ensuring that leading or trailing spaces are removed before validation. Additionally, the `edit_task` function's signature has been updated to accept a mutable reference to allow in-place modifications. These updates aim to enhance input handling and data integrity. ## Changes | File | Change Summary | |-----------------------------|------------------------------------------------------------------------------------------------------------------------------| | `src/server/projects.rs` | Updated `create_project` and `edit_project` to sanitize the `title` field by trimming whitespace. | | `src/server/subtasks.rs` | Updated `create_subtask` and `edit_subtask` to sanitize the `title` field by trimming whitespace. | | `src/server/tasks.rs` | Updated `create_task` and `edit_task` to sanitize the `title` field by trimming whitespace; modified `edit_task` to accept a mutable reference. | ## Possibly related PRs - #39: The changes in `src/server/tasks.rs` related to the addition of the `delete_task` function are relevant because they involve modifications to the same file as the main PR, which also includes changes to the `edit_task` function in that file. <!-- walkthrough_end --><!-- This is an auto-generated comment: raw summary by coderabbit.ai --> <!-- src/server/projects.rs: ## AI-generated summary of changes The diff introduces modifications to the `create_project` and `edit_project` functions in the `src/server/projects.rs` file. Specifically, both functions now include a step to sanitize the `title` field of the `new_project` parameter by trimming whitespace and converting it to a `String`. This change is intended to improve data integrity by ensuring that any leading or trailing spaces in the project title are removed before validation occurs. The addition of this sanitization step is marked with a TODO comment, indicating that it is a temporary measure until a more comprehensive model sanitization is implemented, as referenced by an issue link. The overall logic of the functions remains intact, but the input handling is enhanced to prevent potential issues related to whitespace in project titles. ## Alterations to the declarations of exported or public entities - No alterations to the declarations of exported or public entities were made in this diff. --- src/server/subtasks.rs: ## AI-generated summary of changes The changes introduced in the `create_subtask` and `edit_subtask` functions involve the addition of a preliminary step to sanitize the `title` field of the `NewSubtask` struct. Specifically, the `title` is now trimmed of whitespace before further validation occurs. This modification aims to ensure that any leading or trailing spaces in the title do not affect the validation process, which could potentially lead to errors or inconsistencies in the handling of subtasks. The comments indicate that this is a temporary measure, with a note to replace it with a more comprehensive model sanitization in the future. ## Alterations to the declarations of exported or public entities - No alterations to the declarations of exported or public entities were made in this diff. --- src/server/tasks.rs: ## AI-generated summary of changes The diff introduces modifications to the `create_task` and `edit_task` functions in the `src/server/tasks.rs` file. Both functions now include a step to sanitize the `title` field of the `NewTask` struct by trimming whitespace from the beginning and end of the title string. This change is marked with a TODO comment indicating that the sanitization should eventually be replaced with a more comprehensive model sanitization approach, as referenced in an issue on GitHub. The `edit_task` function's signature has been updated to accept a mutable reference to `new_task`, allowing for in-place modifications. Overall, these changes enhance input validation by ensuring that task titles are clean and free of extraneous whitespace before further processing. ## Alterations to the declarations of exported or public entities - Modified: `pub(crate) async fn edit_task(task_id: i32, new_task: NewTask)` in `src/server/tasks.rs` → `pub(crate) async fn edit_task(task_id: i32, mut new_task: NewTask)` in `src/server/tasks.rs` --> <!-- end of auto-generated comment: raw summary by coderabbit.ai --><!-- This is an auto-generated comment: pr objectives by coderabbit.ai --> <!-- Title: feat: text input value trimming Number: 57 User: matous-volf --> <!-- end of auto-generated comment: pr objectives by coderabbit.ai --><!-- commit_ids_reviewed_start --> <!-- 962b68d446a41224f475056d6fd34947333f0b66 --> <!-- commit_ids_reviewed_end --><!-- tips_start --> --- 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? <details> <summary>Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <details> <summary>Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: -- `I pushed a fix in commit <commit_id>, please review it.` -- `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: -- `@coderabbitai generate unit testing code for this file.` -- `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: -- `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` -- `@coderabbitai read src/utils.ts and generate unit testing code.` -- `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` -- `@coderabbitai help me debug CodeRabbit configuration file.` 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) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details> <!-- tips_end -->
coderabbitai[bot] (Migrated from github.com) requested changes 2024-09-19 18:46:21 +00:00
coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 92dc7a7c88 and 962b68d446.

Files selected for processing (3)
  • src/server/projects.rs (2 hunks)
  • src/server/subtasks.rs (2 hunks)
  • src/server/tasks.rs (2 hunks)
Additional comments not posted (4)
src/server/projects.rs (1)

61-64: LGTM! Remember to address the TODO comment.

The changes improve data integrity by ensuring that any leading or trailing spaces in the project title are removed before validation occurs. This is a good temporary measure until a more comprehensive model sanitization is implemented.

Please ensure that the TODO comment is addressed in a timely manner and the issue linked in the comment is resolved.

src/server/subtasks.rs (1)

16-19: LGTM! Remember to address the TODO comment.

The code segment is a good temporary measure to ensure data consistency by trimming the title field before validation. It does not have any negative impact on the functionality.

Please remember to replace this with a more comprehensive model sanitization as indicated by the TODO comment.

src/server/tasks.rs (2)

116-119: Good temporary solution, but address the TODO comment.

Similar to the changes in the create_task function, trimming whitespace from the title field is a good temporary solution to ensure data consistency. However, as mentioned in the TODO comment, it's recommended to implement a more comprehensive model sanitization approach for better validation and security.

Please prioritize the model sanitization task referenced in the GitHub issue to replace this temporary solution.


112-112: Verify that all callers have been updated to match the new signature.

Changing the new_task parameter to be mutable is necessary for the input validation changes made in the function body. However, this is a breaking change for any code that calls this function.

Please ensure that all callers of the edit_task function have been updated to pass a mutable reference to new_task. You can use the following script to search for callers:

Verification successful

To ensure that the new_task variable is properly declared as mutable before being passed to edit_task, we need to examine its declaration in the src/components/task_form.rs file. Let's run a script to check this:


Verification successful: No changes required in callers

The edit_task function's new signature, which accepts a mutable new_task parameter, does not require changes in the calling code. The current implementation in src/components/task_form.rs is correct:

  • new_task is declared without mut as let new_task = NewTask::new(...);
  • It's then passed directly to edit_task(task.id(), new_task)

This usage is valid because:

  1. The edit_task function can internally make the parameter mutable.
  2. The new_task variable is not used after the edit_task call, so it doesn't need to be mutable in the caller's context.

No further changes are necessary in the calling code.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for callers of the `edit_task` function.
rg --type rust -A 5 $'edit_task'

Length of output: 2134


Script:

#!/bin/bash
# Search for the declaration of new_task in src/components/task_form.rs
rg --type rust -A 5 -B 5 'let.*new_task' src/components/task_form.rs

Length of output: 2410

**Actionable comments posted: 3** <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: ASSERTIVE** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 92dc7a7c887ecd939a10e1c75072f8027f4fa735 and 962b68d446a41224f475056d6fd34947333f0b66. </details> <details> <summary>Files selected for processing (3)</summary> * src/server/projects.rs (2 hunks) * src/server/subtasks.rs (2 hunks) * src/server/tasks.rs (2 hunks) </details> <details> <summary>Additional comments not posted (4)</summary><blockquote> <details> <summary>src/server/projects.rs (1)</summary><blockquote> `61-64`: **LGTM! Remember to address the TODO comment.** The changes improve data integrity by ensuring that any leading or trailing spaces in the project title are removed before validation occurs. This is a good temporary measure until a more comprehensive model sanitization is implemented. Please ensure that the TODO comment is addressed in a timely manner and the issue linked in the comment is resolved. </blockquote></details> <details> <summary>src/server/subtasks.rs (1)</summary><blockquote> `16-19`: **LGTM! Remember to address the TODO comment.** The code segment is a good temporary measure to ensure data consistency by trimming the `title` field before validation. It does not have any negative impact on the functionality. Please remember to replace this with a more comprehensive model sanitization as indicated by the TODO comment. </blockquote></details> <details> <summary>src/server/tasks.rs (2)</summary><blockquote> `116-119`: **Good temporary solution, but address the TODO comment.** Similar to the changes in the `create_task` function, trimming whitespace from the `title` field is a good temporary solution to ensure data consistency. However, as mentioned in the TODO comment, it's recommended to implement a more comprehensive model sanitization approach for better validation and security. Please prioritize the model sanitization task referenced in the GitHub issue to replace this temporary solution. --- `112-112`: **Verify that all callers have been updated to match the new signature.** Changing the `new_task` parameter to be mutable is necessary for the input validation changes made in the function body. However, this is a breaking change for any code that calls this function. Please ensure that all callers of the `edit_task` function have been updated to pass a mutable reference to `new_task`. You can use the following script to search for callers: <details> <summary>Verification successful</summary> To ensure that the `new_task` variable is properly declared as mutable before being passed to `edit_task`, we need to examine its declaration in the src/components/task_form.rs file. Let's run a script to check this: --- <!-- [monologue] [task verify_new_task_declaration 1/1] Analyze the search results for the declaration of `new_task` in src/components/task_form.rs and verify if it's properly declared as mutable. [observations] 1. There are two occurrences of `new_task` declaration: a. First occurrence: `let new_task = NewTask::new(...);` b. Second occurrence: `let new_task = NewTask::new(...);` 2. Both declarations use `let` without the `mut` keyword. 3. The `new_task` variable is used immediately after declaration in both cases: a. `let _ = edit_task(task.id(), new_task).await;` b. `let _ = edit_task(task.id(), new_task).await;` [analysis] 1. The `new_task` variable is not declared as mutable in either case. 2. The `edit_task` function is called with `new_task` as an argument immediately after its declaration. 3. Since `new_task` is not used after the `edit_task` call, it doesn't need to be mutable in this context. 4. The change in the `edit_task` function signature to accept a mutable reference doesn't require the caller to pass a mutable variable. The function can still accept an immutable reference and make it mutable internally. [actions] 1. Conclude that the current usage of `new_task` in src/components/task_form.rs is correct and doesn't need to be changed. 2. Generate a final response indicating that the verification is successful and no issues are found. --> **Verification successful: No changes required in callers** The `edit_task` function's new signature, which accepts a mutable `new_task` parameter, does not require changes in the calling code. The current implementation in `src/components/task_form.rs` is correct: - `new_task` is declared without `mut` as `let new_task = NewTask::new(...);` - It's then passed directly to `edit_task(task.id(), new_task)` This usage is valid because: 1. The `edit_task` function can internally make the parameter mutable. 2. The `new_task` variable is not used after the `edit_task` call, so it doesn't need to be mutable in the caller's context. No further changes are necessary in the calling code. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Search for callers of the `edit_task` function. rg --type rust -A 5 $'edit_task' ``` Length of output: 2134 --- Script: ```shell #!/bin/bash # Search for the declaration of new_task in src/components/task_form.rs rg --type rust -A 5 -B 5 'let.*new_task' src/components/task_form.rs ``` Length of output: 2410 </details> </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
@ -15,0 +15,4 @@
// TODO: replace with model sanitization (https://github.com/matous-volf/todo-baggins/issues/13)
let mut new_project = new_project;
new_project.title = new_project.title.trim().to_owned();
coderabbitai[bot] (Migrated from github.com) commented 2024-09-19 18:46:20 +00:00

LGTM! Remember to address the TODO comment.

The changes improve data integrity by ensuring that any leading or trailing spaces in the project title are removed before validation occurs. This is a good temporary measure until a more comprehensive model sanitization is implemented.

Please ensure that the TODO comment is addressed in a timely manner and the issue linked in the comment is resolved.

Do you want me to open a GitHub issue to track this task?

**LGTM! Remember to address the TODO comment.** The changes improve data integrity by ensuring that any leading or trailing spaces in the project title are removed before validation occurs. This is a good temporary measure until a more comprehensive model sanitization is implemented. Please ensure that the TODO comment is addressed in a timely manner and the issue linked in the comment is resolved. Do you want me to open a GitHub issue to track this task? <!-- This is an auto-generated comment by CodeRabbit -->
@ -20,0 +20,4 @@
// TODO: replace with model sanitization (https://github.com/matous-volf/todo-baggins/issues/13)
let mut new_task = new_task;
new_task.title = new_task.title.trim().to_owned();
coderabbitai[bot] (Migrated from github.com) commented 2024-09-19 18:46:20 +00:00

Good temporary solution, but address the TODO comment.

Trimming whitespace from the title field is a good temporary solution to ensure data consistency. However, as mentioned in the TODO comment, it's recommended to implement a more comprehensive model sanitization approach for better validation and security.

Please prioritize the model sanitization task referenced in the GitHub issue to replace this temporary solution.


Consider extracting the input validation logic into a separate function.

The input validation logic that trims whitespace from the title field is duplicated in the create_task and edit_task functions. This violates the DRY principle and can lead to maintenance issues.

Consider extracting the input validation logic into a separate function and calling it from both create_task and edit_task functions. This will centralize the validation logic and make it easier to update in the future.

For example:

fn sanitize_new_task(mut new_task: NewTask) -> NewTask {
    new_task.title = new_task.title.trim().to_owned();
    new_task
}

Then, in create_task and edit_task functions:

let new_task = sanitize_new_task(new_task);

Also applies to: 116-119

**Good temporary solution, but address the TODO comment.** Trimming whitespace from the `title` field is a good temporary solution to ensure data consistency. However, as mentioned in the TODO comment, it's recommended to implement a more comprehensive model sanitization approach for better validation and security. Please prioritize the model sanitization task referenced in the GitHub issue to replace this temporary solution. --- **Consider extracting the input validation logic into a separate function.** The input validation logic that trims whitespace from the `title` field is duplicated in the `create_task` and `edit_task` functions. This violates the DRY principle and can lead to maintenance issues. Consider extracting the input validation logic into a separate function and calling it from both `create_task` and `edit_task` functions. This will centralize the validation logic and make it easier to update in the future. For example: ```rust fn sanitize_new_task(mut new_task: NewTask) -> NewTask { new_task.title = new_task.title.trim().to_owned(); new_task } ``` Then, in `create_task` and `edit_task` functions: ```rust let new_task = sanitize_new_task(new_task); ``` Also applies to: 116-119 <!-- This is an auto-generated comment by CodeRabbit -->
coderabbitai[bot] (Migrated from github.com) approved these changes 2024-09-19 18:47:54 +00:00
Sign in to join this conversation.
No Milestone
No project
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: matous-volf/todo-baggins#57
No description provided.