Skip to content
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

[Woo POS][Non Simple product types] Fetch variations list #12910

Merged
merged 61 commits into from
Nov 21, 2024

Conversation

AnirudhBhat
Copy link
Contributor

@AnirudhBhat AnirudhBhat commented Nov 12, 2024

Closes: #12844

NOTE: I've extracted the UI in the products screen into separate file which is the cause for many line changes.

Description

This PR fetches variations list and displays on the variations screen.

It makes use of existing VariationsListHandler class for fetching variations.

Testing information

Testing Variations list

  1. Navigate to POS mode (more menu -> POS)
  2. Click on any variable product
  3. Ensure you see variations displayed

Testing Variations pagination

  1. Navigate to POS mode (more menu -> POS)
  2. Click on any variable product that has more than 10 variations
  3. Ensure you see variations displayed and it paginates as expected

Testing Variations pagination error

  1. Navigate to POS mode (more menu -> POS)
  2. Click on any variable product that has more than 10 variations
  3. Ensure you see variations displayed
  4. Turn on airplane mode
  5. Scroll down to end of the list
  6. Ensure you see snackbar message that says failed to fetch variations.

Testing Variations fetch error

  1. Navigate to POS mode (more menu -> POS)
  2. Turn on airplane mode
  3. Click on any variable product
  4. Ensure you see appropriate error screen

The tests that have been performed

Tested all the above scenario listed in both light and dark modes on emulator.

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.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

…or all items list screens like products, variations ...etc
…or all items list screens like products, variations ...etc
…ceRefresh set to true when variations view model is initiated.
…g property set to true when pull to refreshed
…g property set to true when pull to refreshed
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 12, 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
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit4e26906
Direct Downloadwoocommerce-prototype-build-pr12910-4e26906.apk

…nto issue/12844-fetch-variations

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsScreen.kt
#	WooCommerce/src/main/res/values/strings.xml
@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 32.02417% with 225 lines in your changes missing coverage. Please review.

Project coverage is 39.59%. Comparing base (fb44b49) to head (4e26906).
Report is 210 commits behind head on trunk.

Files with missing lines Patch % Lines
...ce/android/ui/woopos/home/items/WooPosItemsList.kt 0.00% 183 Missing ⚠️
...ome/items/variations/WooPosVariationsDataSource.kt 4.16% 23 Missing ⚠️
...home/items/variations/WooPosVariationsViewModel.kt 85.13% 3 Missing and 8 partials ⚠️
...droid/ui/woopos/home/items/WooPosItemsViewModel.kt 0.00% 4 Missing ⚠️
.../ui/woopos/home/items/WooPosVariationsViewState.kt 83.33% 3 Missing ⚠️
...oducts/variations/selector/VariationListHandler.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #12910      +/-   ##
============================================
- Coverage     39.63%   39.59%   -0.04%     
- Complexity     5932     5956      +24     
============================================
  Files          1261     1267       +6     
  Lines         72862    73179     +317     
  Branches       9965    10048      +83     
============================================
+ Hits          28879    28976      +97     
- Misses        41414    41629     +215     
- Partials       2569     2574       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@AnirudhBhat AnirudhBhat marked this pull request as ready for review November 18, 2024 04:11
@@ -0,0 +1,343 @@
package com.woocommerce.android.ui.woopos.home.items

import androidx.compose.foundation.ExperimentalFoundationApi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of this changes are because of extracting it into different file.

.collect { event ->
when (event) {
is WooPosVariationsViewModel.WooPosVariationEvents.PaginationError -> {
snackbarHostState.showSnackbar(
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 may change from showing snack bar into a card with retry button. If we decide on that, I will raise a separate PR for it.

…nto issue/12844-fetch-variations

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItem.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsScreen.kt
#	WooCommerce/src/main/res/values/strings.xml
@samiuelson samiuelson self-assigned this Nov 19, 2024
…nto issue/12844-fetch-variations

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsScreen.kt
Base automatically changed from issue/12843-navigate-to-variations-screen to trunk November 20, 2024 08:53
Copy link
Collaborator

@samiuelson samiuelson left a comment

Choose a reason for hiding this comment

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

LGTM!


@OptIn(ExperimentalFoundationApi::class)
@Composable
fun ItemsList(
Copy link
Collaborator

Choose a reason for hiding this comment

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

💡 n.p. In store management we follow the convention of singular+List (e.g. itemList) or just plural (with "s", without "List"). Maybe it's worth renaming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: 4e26906

@AnirudhBhat AnirudhBhat added this to the 21.2 milestone Nov 21, 2024
@AnirudhBhat AnirudhBhat merged commit 81b12c9 into trunk Nov 21, 2024
15 checks passed
@AnirudhBhat AnirudhBhat deleted the issue/12844-fetch-variations branch November 21, 2024 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[POS][Non-Simple Product type] Fetch variations of a variable products and display it in a list
5 participants