Skip to content

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

Merged

Conversation

JorgeMucientes
Copy link
Contributor

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:

  • Adding new product categories
  • Editing categories (changing name or parent)
  • Deleting categories with or without child categories

Comment on lines -152 to -153
val addedCategory = productCategoriesRepository
.getProductCategoryByNameAndParentId(categoryNameTrimmed, parentId)
Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@JorgeMucientes JorgeMucientes added the type: crash The worst kind of bug. label May 29, 2024
@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commit29ae9fc
Direct Downloadwoocommerce-prototype-build-pr11614-29ae9fc.apk

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 40.24%. Comparing base (11da5dd) to head (29ae9fc).
Report is 89 commits behind head on trunk.

Files Patch % Lines
...products/categories/AddProductCategoryViewModel.kt 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Comment on lines -152 to -153
val addedCategory = productCategoriesRepository
.getProductCategoryByNameAndParentId(categoryNameTrimmed, parentId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@0nko 0nko merged commit c0833a1 into trunk May 29, 2024
13 checks passed
@0nko 0nko deleted the fix/avoid-nullpointer-exception-when-adding-edit-categories branch May 29, 2024 19:55
@JorgeMucientes JorgeMucientes added this to the 18.9 ❄️ milestone Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: crash The worst kind of bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants