Skip to content

Add a check to avoid app crash when the products list is empty #11490

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
merged 2 commits into from
May 13, 2024

Conversation

hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented May 13, 2024

Closes: #11489

Description

During the code cleanup after enabling Dynamic Dashboard feature, I removed the status Hidden from the Blaze card as it wasn't relevant anymore (the card visibility is handled by the parent ViewModel), but this has caused a crash in some cases, the testing instructions will explain those cases.

Testing instructions

  1. Using the branch release/18.6.
  2. Use a site with a single product.
  3. Open the app.
  4. Make sure the Blaze card is enabled.
  5. Go to the Products list, and trash the published product.
  6. Go back to the MyStore screen.
  7. Pull the screen to refresh it.
  8. Notice the crash.
  9. Now switch to the branch issue/11489-fix-blaze-card-crash
  10. Repeat 2 - 7
  11. Confirm the app doesn't crash, and the Blaze card is hidden as expected.

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@hichamboushaba hichamboushaba added type: crash The worst kind of bug. feature: dashboard Related to home screen project feature: blaze Related to the Blaze project labels May 13, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented May 13, 2024

1 Warning
⚠️ This PR is assigned to the milestone 18.6 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@hichamboushaba hichamboushaba added this to the 18.6 ❄️ milestone May 13, 2024
@hichamboushaba hichamboushaba requested a review from 0nko May 13, 2024 08:27
@hichamboushaba hichamboushaba marked this pull request as ready for review May 13, 2024 08:27
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented May 13, 2024

📲 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
Commitfca0184
Direct Downloadwoocommerce-prototype-build-pr11490-fca0184.apk

Comment on lines +41 to +44
.fetchProductList(
productFilterOptions = mapOf(ProductFilterOption.STATUS to ProductStatus.PUBLISH.value)
).getOrNull()
?.filterNot { it.isSampleProduct }
Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed this issue while working on this, when the cached list is empty, I was fetching a new list from the API, but I was checking against the list of all products, and not just published non-sample products.

@0nko 0nko self-assigned this May 13, 2024
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.

Confirming the fix works 👍

@0nko 0nko enabled auto-merge May 13, 2024 09:44
@0nko 0nko merged commit 24bdb38 into release/18.6 May 13, 2024
13 of 14 checks passed
@0nko 0nko deleted the issue/11489-fix-blaze-card-crash branch May 13, 2024 10:06
Copy link

sentry-io bot commented May 14, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ NoSuchElementException: List is empty. Dashboard View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: blaze Related to the Blaze project feature: dashboard Related to home screen project type: crash The worst kind of bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants