-
Notifications
You must be signed in to change notification settings - Fork 132
Return the category from the API result rather than getting it from the DB #11614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Return the category from the API result rather than getting it from the DB #11614
Conversation
val addedCategory = productCategoriesRepository | ||
.getProductCategoryByNameAndParentId(categoryNameTrimmed, parentId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have the new or edited category from the API result, so I think we can safely remove this access to the database, which is the one leading to the crash. The code is now null safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #11614 +/- ##
=========================================
Coverage 40.24% 40.24%
Complexity 5202 5202
=========================================
Files 1089 1089
Lines 63462 63459 -3
Branches 8711 8711
=========================================
Hits 25539 25539
+ Misses 35619 35616 -3
Partials 2304 2304 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val addedCategory = productCategoriesRepository | ||
.getProductCategoryByNameAndParentId(categoryNameTrimmed, parentId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Another attempt to fix this crash: #11210
Description
I wasn't able to reproduce the crash but I've made the code nullably safe and removed a DB access that I'm not sure why was it there in the first place. I think is a leftover from legacy implementation that I missed refactoring when I added the edit and remove functionality.
Testing instructions
Smoke test product category feature: