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

[Shipping Labels Revamp] Handle Package selection on the main form #13111

Merged
merged 38 commits into from
Dec 13, 2024

Conversation

ThomazFB
Copy link
Contributor

@ThomazFB ThomazFB commented Dec 11, 2024

Summary

Fix issue #12747 by introducing the complete handling of the Package selection into the main Shipping Labels form.

Screen Capture

Screen_recording_20241212_184222.mp4

How to Test

Scenario 1 - Carrier Package

  1. Go to the Order Details of an Order eligible for Shipping Label Creation
  2. Hit the Create Shipping Labels button
  3. Once inside the new Shipping Labels form, hit the Select a Package button
  4. Select a Carrier Package and hit the Add Package Button
  5. Verify that the Shipping Label form is presented back, with the Carrier package displayed properly
  6. Verify that the subtitle of the Package contains the Carrier Category of that given package
  7. Verify that the edit button re-enters the Package selection UI

Scenario 2 - Saved Package

  1. Go to the Order Details of an Order eligible for Shipping Label Creation
  2. Hit the Create Shipping Labels button
  3. Once inside the new Shipping Labels form, hit the Select a Package button
  4. Select a Saved Package and hit the Add Package Button
  5. Verify that the Shipping Label form is presented back, with the Saved package displayed properly
  6. Verify that the subtitle of the Package contains the Package type (Box or Envelope)
  7. Verify that the edit button re-enters the Package selection UI

Scenario 3 - Custom Package not saved as a template

  1. Go to the Order Details of an Order eligible for Shipping Label Creation
  2. Hit the Create Shipping Labels button
  3. Once inside the new Shipping Labels form, hit the Select a Package button
  4. Fill the Custom Package form, DO NOT select the save as template check and hit the Add Package Button
  5. Verify that the Shipping Label form is presented back, with the Custom package displayed properly
  6. Verify that the Package subtitle contains the Package type (Box or Envelope)
  7. Verify that the Package title is named as Custom Package
  8. Verify that the edit button re-enters the Package selection UI

Scenario 4 - Custom Package saved as a template

  1. Go to the Order Details of an Order eligible for Shipping Label Creation
  2. Hit the Create Shipping Labels button
  3. Once inside the new Shipping Labels form, hit the Select a Package button
  4. Fill the Custom Package form, SELECT the save as template check
  5. Add the package weight and name and hit the Add Package Button
  6. Verify that the Shipping Label form is presented back, with the Custom package displayed properly
  7. Verify that the Package subtitle contains the Package type (Box or Envelope)
  8. Verify that the Package title is named as the given name in the Custom package form
  9. Verify that the edit button re-enters the Package selection UI

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

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.

@ThomazFB ThomazFB added type: task An internally driven task. feature: shipping labels Related to creating, ordering, or printing shipping labels. labels Dec 11, 2024
@ThomazFB ThomazFB added this to the 21.3 milestone Dec 11, 2024
@ThomazFB ThomazFB changed the base branch from trunk to issue/add-split-package-data-dimensions December 11, 2024 23:09
@ThomazFB ThomazFB linked an issue Dec 11, 2024 that may be closed by this pull request
@dangermattic
Copy link
Collaborator

dangermattic commented Dec 11, 2024

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 11, 2024

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit5aff9b3
Direct Downloadwoocommerce-wear-prototype-build-pr13111-5aff9b3.apk

@ThomazFB ThomazFB mentioned this pull request Dec 11, 2024
4 tasks
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 11, 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
Commit5aff9b3
Direct Downloadwoocommerce-prototype-build-pr13111-5aff9b3.apk

Base automatically changed from issue/add-split-package-data-dimensions to trunk December 12, 2024 15:30
@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.

Project coverage is 40.36%. Comparing base (a58e05d) to head (5aff9b3).
Report is 39 commits behind head on trunk.

Files with missing lines Patch % Lines
...ckages/datasource/WooShippingLabelPackageMapper.kt 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #13111      +/-   ##
============================================
+ Coverage     40.34%   40.36%   +0.02%     
- Complexity     6204     6206       +2     
============================================
  Files          1296     1296              
  Lines         74932    74961      +29     
  Branches      10272    10275       +3     
============================================
+ Hits          30233    30261      +28     
- Misses        42061    42062       +1     
  Partials       2638     2638              

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

@ThomazFB ThomazFB marked this pull request as ready for review December 12, 2024 22:04
onClick = onSelectPackageClick
) {
Icon(
painter = painterResource(id = R.drawable.ic_edit),
Copy link
Contributor

Choose a reason for hiding this comment

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

np: We should use the material pencil here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's true! Thanks for the suggestion, it's fixed in 5aff9b3

Copy link
Contributor

@atorresveiga atorresveiga left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

Nice work @ThomazFB! Everything is working as described. I left a small observation about the pencil icon, but is not a blocker for this PR

@ThomazFB ThomazFB merged commit 95a74e7 into trunk Dec 13, 2024
15 checks passed
@ThomazFB ThomazFB deleted the issue/handle-selected-package branch December 13, 2024 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: shipping labels Related to creating, ordering, or printing shipping labels. type: task An internally driven task. unit-tests-exemption
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Shipping Labels: Packages] Add Package Opening UI Section
5 participants