squid/blog
squid/blog
1
2
Fork 0
Code Issues 8 Pull Requests Actions Projects 1 Releases 7 Wiki Activity

BLOG-142 Move label to a new feature #143

Merged
squid merged 4 commits from BLOG-142_move_label_to_new_feature into main 2025-10-15 06:23:58 +08:00
Conversation 3 Commits 4 Files Changed 81 +402 -259
squid commented 2025-10-15 05:34:43 +08:00
Owner
Copy Link

Description

  • As the title

Package Changes

No response

Screenshots

No response

Reference

Resolves #142.

Checklist

  • A milestone is set
  • The related issuse has been linked to this branch
### Description - As the title ### Package Changes _No response_ ### Screenshots _No response_ ### Reference Resolves #142. ### Checklist - [x] A milestone is set - [x] The related issuse has been linked to this branch
squid added this to the 0.4 milestone 2025-10-15 05:34:43 +08:00
squid added 1 commit 2025-10-15 05:34:44 +08:00
refactor: move label to a new feature
Some checks failed
Frontend CI / build (push) Successful in 1m39s
Details
Auto Comment On PR / add_improve_comment (pull_request) Successful in 17s
Details
PR Title Check / pr-title-check (pull_request) Failing after 16s
Details
e177814996
bot commented 2025-10-15 05:34:51 +08:00
Collaborator
Copy Link

/improve

/improve
bot commented 2025-10-15 05:34:57 +08:00
Collaborator
Copy Link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
General
Avoid cloning and return full entity

The label_entity is cloned before being passed to the create_label_use_case, and
then the ID is manually assigned to the original entity. Consider having the
create_label_use_case return the complete Label entity with the generated ID. This
approach avoids unnecessary cloning and simplifies the logic.

backend/feature/label/src/adapter/delivery/label_controller.rs [62-69]

-let mut label_entity = label.into_entity();
-let id = self
+let label_entity = label.into_entity();
+let created_label = self
     .create_label_use_case
-    .execute(label_entity.clone())
+    .execute(label_entity) // Pass by value if use case takes ownership
     .await?;
 
-label_entity.id = id;
-Ok(LabelResponseDto::from(label_entity))
+Ok(LabelResponseDto::from(created_label))
Suggestion importance[1-10]: 8

__

Why: The suggestion proposes a better design by having the create_label_use_case return the complete Label entity, which avoids unnecessary cloning and simplifies the logic in the controller. This would require changes in the CreateLabelUseCase trait and its implementation.

Medium
Return updated entity from use case

The label_entity is cloned before the update_label_use_case is executed. If the use
case modifies the entity (e.g., updates a timestamp), these changes will not be
reflected in the LabelResponseDto as it's built from the pre-modification entity.
The use case should return the updated entity to ensure the response is consistent
with the database state.

backend/feature/label/src/adapter/delivery/label_controller.rs [77-82]

 let label_entity = label.into_entity(id);
-self.update_label_use_case
-    .execute(label_entity.clone())
+let updated_label = self
+    .update_label_use_case
+    .execute(label_entity) // Pass by value if use case takes ownership
     .await?;
 
-Ok(LabelResponseDto::from(label_entity))
+Ok(LabelResponseDto::from(updated_label))
Suggestion importance[1-10]: 8

__

Why: This suggestion improves data consistency by ensuring the update_label_use_case returns the potentially modified Label entity. This prevents the LabelResponseDto from being created with an outdated entity and avoids unnecessary cloning.

Medium
Possible issue
Correct error message for unauthorized access

The error message for LabelError::Unauthorized incorrectly refers to "post". This
message should be updated to reflect the context of a "label" to provide accurate
feedback to users or for logging purposes.

backend/feature/label/src/application/error/label_error.rs [15]

-LabelError::Unauthorized => write!(f, "Unauthorized access to post"),
+LabelError::Unauthorized => write!(f, "Unauthorized access to label"),
Suggestion importance[1-10]: 7

__

Why: The error message Unauthorized access to post is incorrect in the context of a label feature. Correcting it to Unauthorized access to label improves the accuracy and clarity of error reporting.

Medium
## PR Code Suggestions ✨ <!-- --> <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>General</td> <td> <details><summary>Avoid cloning and return full entity</summary> ___ **The <code>label_entity</code> is cloned before being passed to the <code>create_label_use_case</code>, and <br>then the ID is manually assigned to the original entity. Consider having the <br><code>create_label_use_case</code> return the complete <code>Label</code> entity with the generated ID. This <br>approach avoids unnecessary cloning and simplifies the logic.** [backend/feature/label/src/adapter/delivery/label_controller.rs [62-69]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-142_move_label_to_new_feature/backend/feature/label/src/adapter/delivery/label_controller.rs#L62-L69) ```diff -let mut label_entity = label.into_entity(); -let id = self +let label_entity = label.into_entity(); +let created_label = self .create_label_use_case - .execute(label_entity.clone()) + .execute(label_entity) // Pass by value if use case takes ownership .await?; -label_entity.id = id; -Ok(LabelResponseDto::from(label_entity)) +Ok(LabelResponseDto::from(created_label)) ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion proposes a better design by having the `create_label_use_case` return the complete `Label` entity, which avoids unnecessary cloning and simplifies the logic in the controller. This would require changes in the `CreateLabelUseCase` trait and its implementation. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Return updated entity from use case</summary> ___ **The <code>label_entity</code> is cloned before the <code>update_label_use_case</code> is executed. If the use <br>case modifies the entity (e.g., updates a timestamp), these changes will not be <br>reflected in the <code>LabelResponseDto</code> as it's built from the pre-modification entity. <br>The use case should return the updated entity to ensure the response is consistent <br>with the database state.** [backend/feature/label/src/adapter/delivery/label_controller.rs [77-82]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-142_move_label_to_new_feature/backend/feature/label/src/adapter/delivery/label_controller.rs#L77-L82) ```diff let label_entity = label.into_entity(id); -self.update_label_use_case - .execute(label_entity.clone()) +let updated_label = self + .update_label_use_case + .execute(label_entity) // Pass by value if use case takes ownership .await?; -Ok(LabelResponseDto::from(label_entity)) +Ok(LabelResponseDto::from(updated_label)) ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This suggestion improves data consistency by ensuring the `update_label_use_case` returns the potentially modified `Label` entity. This prevents the `LabelResponseDto` from being created with an outdated entity and avoids unnecessary cloning. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Correct error message for unauthorized access</summary> ___ **The error message for <code>LabelError::Unauthorized</code> incorrectly refers to "post". This <br>message should be updated to reflect the context of a "label" to provide accurate <br>feedback to users or for logging purposes.** [backend/feature/label/src/application/error/label_error.rs [15]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-142_move_label_to_new_feature/backend/feature/label/src/application/error/label_error.rs#L15-L15) ```diff -LabelError::Unauthorized => write!(f, "Unauthorized access to post"), +LabelError::Unauthorized => write!(f, "Unauthorized access to label"), ``` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The error message `Unauthorized access to post` is incorrect in the context of a `label` feature. Correcting it to `Unauthorized access to label` improves the accuracy and clarity of error reporting. </details></details></td><td align=center>Medium </td></tr></tr></tbody></table>
squid added 1 commit 2025-10-15 05:40:11 +08:00
fix: correct error message for unauthorized label access
All checks were successful
Frontend CI / build (push) Successful in 1m38s
Details
PR Title Check / pr-title-check (pull_request) Successful in 16s
Details
72e944af7b
squid commented 2025-10-15 05:41:27 +08:00
Author
Owner
Copy Link

The error message for LabelError::Unauthorized incorrectly refers to "post". This
message should be updated to reflect the context of a "label" to provide accurate
feedback to users or for logging purposes.

backend/feature/label/src/application/error/label_error.rs [15]

-LabelError::Unauthorized => write!(f, "Unauthorized access to post"),
+LabelError::Unauthorized => write!(f, "Unauthorized access to label"),

Addressed in 72e944af7b.

> **The error message for <code>LabelError::Unauthorized</code> incorrectly refers to "post". This <br>message should be updated to reflect the context of a "label" to provide accurate <br>feedback to users or for logging purposes.** > > [backend/feature/label/src/application/error/label_error.rs [15]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-142_move_label_to_new_feature/backend/feature/label/src/application/error/label_error.rs#L15-L15) > > ```diff > -LabelError::Unauthorized => write!(f, "Unauthorized access to post"), > +LabelError::Unauthorized => write!(f, "Unauthorized access to label"), > ``` Addressed in 72e944af7b617854b08e5029a54003f93bbbf9b5.
squid changed title from BLOG-142 Move `label` to a new feature to BLOG-142 Move label to a new feature 2025-10-15 05:41:46 +08:00
squid added 1 commit 2025-10-15 05:43:49 +08:00
fix: rename post route configuration to label route configuration
All checks were successful
Frontend CI / build (push) Successful in 1m37s
Details
PR Title Check / pr-title-check (pull_request) Successful in 17s
Details
6213ab7a95
squid added 1 commit 2025-10-15 06:12:04 +08:00
feat: enhance error handling and API documentation for image and label features
All checks were successful
Frontend CI / build (push) Successful in 1m36s
Details
PR Title Check / pr-title-check (pull_request) Successful in 16s
Details
3b16b165bb
squid merged commit a577f94acd into main 2025-10-15 06:23:58 +08:00
squid deleted branch BLOG-142_move_label_to_new_feature 2025-10-15 06:23:58 +08:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
design
dev
automation
dev
backend
dev
frontend
dev
improvement
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
question
More information is needed
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone 0.4
Projects
Clear projects
No project
Assignees
Clear assignees
bot zoe squid
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: squid/blog#143
No description provided.
Delete Branch "BLOG-142_move_label_to_new_feature"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?