chore(server): add log to import logic #1386
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request revises the item import logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant I as ItemImportInteractor
participant L as Logger
C->>I: Trigger import operation
alt Item exists in savedItemsMap
I->>I: Lookup item and validate existence
I->>I: Create event for the item
I-->>C: Return success response
else Item not found
I->>L: Log debug message for missing item
I-->>C: Return rerror.ErrNotFound
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for reearth-cms canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
server/internal/usecase/interactor/item_import.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-server / test
| savedItemsMap := savedItems.ToMap() | ||
| for k, changes := range itemsEvent { | ||
| vi := savedItems.Item(k) | ||
| vi := savedItemsMap[k] | ||
| if vi == nil { | ||
| log.Debugf("item %s not found", k) | ||
| return rerror.ErrNotFound | ||
| } |
There was a problem hiding this comment.
Fix error handling to continue processing remaining items.
The current implementation returns rerror.ErrNotFound immediately when an item is not found, which breaks the entire import process. This is likely not the intended behavior.
Consider this alternative implementation:
savedItemsMap := savedItems.ToMap()
for k, changes := range itemsEvent {
vi := savedItemsMap[k]
if vi == nil {
- log.Debugf("item %s not found", k)
- return rerror.ErrNotFound
+ log.Debugf("item %s not found, skipping event creation for this item", k)
+ continue
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| savedItemsMap := savedItems.ToMap() | |
| for k, changes := range itemsEvent { | |
| vi := savedItems.Item(k) | |
| vi := savedItemsMap[k] | |
| if vi == nil { | |
| log.Debugf("item %s not found", k) | |
| return rerror.ErrNotFound | |
| } | |
| savedItemsMap := savedItems.ToMap() | |
| for k, changes := range itemsEvent { | |
| vi := savedItemsMap[k] | |
| if vi == nil { | |
| log.Debugf("item %s not found, skipping event creation for this item", k) | |
| continue | |
| } |
Summary by CodeRabbit