matous-volf/todo-baggins
matous-volf/todo-baggins
1
0
Fork 0
Code Issues 23 Pull Requests Actions Projects 1 Activity

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
Conversation 3 Commits 1 Files Changed 3 +25 -1
3 changed files with 25 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/server/projects.rs
View File

@ -12,6 +12,10 @@ pub(crate) async fn create_project(new_project: NewProject)
-> Result<Project, ServerFnError<ErrorVec<ProjectError>>> {
use crate::schema::projects;
// 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] commented 2024-09-19 18:46:20 +00:00 (Migrated from github.com)
Review
Copy Link

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 -->
new_project.validate()
.map_err::<ErrorVec<ProjectError>, _>(|errors| errors.into())?;
@ -54,6 +58,10 @@ pub(crate) async fn edit_project(project_id: i32, new_project: NewProject)
-> Result<Project, ServerFnError<ErrorVec<ProjectError>>> {
use crate::schema::projects::dsl::*;
// 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();
new_project.validate()
.map_err::<ErrorVec<ProjectError>, _>(|errors| errors.into())?;

8
src/server/subtasks.rs
View File

@ -13,6 +13,10 @@ pub(crate) async fn create_subtask(new_subtask: NewSubtask)
-> Result<Subtask, ServerFnError<ErrorVec<SubtaskError>>> {
use crate::schema::subtasks;
// TODO: replace with model sanitization (https://github.com/matous-volf/todo-baggins/issues/13)
let mut new_subtask = new_subtask;
new_subtask.title = new_subtask.title.trim().to_owned();
new_subtask.validate()
.map_err::<ErrorVec<SubtaskError>, _>(|errors| errors.into())?;
@ -55,6 +59,10 @@ pub(crate) async fn edit_subtask(subtask_id: i32, new_subtask: NewSubtask)
-> Result<Subtask, ServerFnError<ErrorVec<SubtaskError>>> {
use crate::schema::subtasks::dsl::*;
// TODO: replace with model sanitization (https://github.com/matous-volf/todo-baggins/issues/13)
let mut new_subtask = new_subtask;
new_subtask.title = new_subtask.title.trim().to_owned();
new_subtask.validate()
.map_err::<ErrorVec<SubtaskError>, _>(|errors| errors.into())?;

10
src/server/tasks.rs
View File

@ -17,6 +17,10 @@ use crate::server::subtasks::restore_subtasks_of_task;
pub(crate) async fn create_task(new_task: NewTask)
-> Result<Task, ServerFnError<ErrorVec<TaskError>>> {
use crate::schema::tasks;
// 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] commented 2024-09-19 18:46:20 +00:00 (Migrated from github.com)
Review
Copy Link

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 -->
new_task.validate()
.map_err::<ErrorVec<TaskError>, _>(|errors| errors.into())?;
@ -105,10 +109,14 @@ pub(crate) async fn get_tasks_with_subtasks_in_category(filtered_category: Categ
}
#[server]
pub(crate) async fn edit_task(task_id: i32, new_task: NewTask)
pub(crate) async fn edit_task(task_id: i32, mut new_task: NewTask)
-> Result<Task, ServerFnError<ErrorVec<TaskError>>> {
use crate::schema::tasks::dsl::*;
// 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();
new_task.validate()
.map_err::<ErrorVec<TaskError>, _>(|errors| errors.into())?;