woocommerce-android icon indicating copy to clipboard operation
woocommerce-android copied to clipboard

Issue/12414 custom fields editor

Open hichamboushaba opened this issue 1 year ago • 4 comments

Part of: #12365

Description

This PR is the first part in implementing the custom fields editor, it adds a new screen to edit existing custom fields, the same screen will be used when adding new fields too, and that should be taken into consideration when reviewing the code.

Many parts are still missing to allow keeping the PR size manageable, this includes the following:

  1. Saving changes.
  2. Returning results to the list screen and updating its state.
  3. Using Aztec when editing HTML values.

While the PR seems large, a big part of the changes is just tests and XML file changes due to using a nested nav graph, so it should be still easy to review.

Testing information

  1. Open an order in wp-admin.
  2. Tap on the "Custom Fields" section is visible (from the screen options at the top of the page)
  3. Add some custom fields (for a more comprehensive testing, add fields that contain a URL, an email and a phone number)
  4. Open the order in the app.
  5. Tap on "View custom fields"
  6. Tap on a custom field.
  7. Make some changes.
  8. Tap on the back button
  9. Confirm a dialog is shown.
  10. Test the behavior of the dialog buttons.

The tests that have been performed

The above steps in addition to the validation rules.

Images/gif

  • [x] 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.

hichamboushaba avatar Aug 26 '24 20:08 hichamboushaba

2 Warnings
:warning: This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
:warning: Class UiState is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by :no_entry_sign: Danger

dangermattic avatar Aug 26 '24 20:08 dangermattic

📲 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
Commita5d1fed368633cbf7806f15e3aba1c893442c82d
Direct Downloadwoocommerce-wear-prototype-build-pr12416-a5d1fed.apk

wpmobilebot avatar Aug 26 '24 20:08 wpmobilebot

📲 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
Commita5d1fed368633cbf7806f15e3aba1c893442c82d
Direct Downloadwoocommerce-prototype-build-pr12416-a5d1fed.apk

wpmobilebot avatar Aug 26 '24 20:08 wpmobilebot

Codecov Report

Attention: Patch coverage is 76.82927% with 19 lines in your changes missing coverage. Please review.

Project coverage is 40.74%. Comparing base (3601574) to head (a5d1fed). Report is 287 commits behind head on trunk.

Files with missing lines Patch % Lines
...customfields/editor/CustomFieldsEditorViewModel.kt 79.41% 5 Missing and 9 partials :warning:
.../android/ui/customfields/CustomFieldsRepository.kt 0.00% 5 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #12416      +/-   ##
============================================
+ Coverage     40.70%   40.74%   +0.04%     
- Complexity     5607     5619      +12     
============================================
  Files          1214     1215       +1     
  Lines         68138    68218      +80     
  Branches       9356     9363       +7     
============================================
+ Hits          27733    27794      +61     
- Misses        37864    37874      +10     
- Partials       2541     2550       +9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Aug 27 '24 16:08 codecov-commenter