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

Issue/12563 delete custom fields #12566

Merged
merged 10 commits into from
Sep 12, 2024
Merged

Conversation

hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented Sep 9, 2024

Closes: #12563

Description

This PR adds support for deleting custom fields.

Steps to reproduce

  1. Open wp-admin, and add at least one custom field to an order.
  2. Open the order in the app.
  3. Tap on "View custom fields"
  4. Open an existing custom field.
  5. Tap on the overflow menu button.
  6. Tap on Delete button.

Testing information

  1. Test the delete flow for an existing field.
  2. Test the delete flow for a new non-saved field.
  3. Test the undo SnackBar.
  4. Test saving changes after deleting some items.

The tests that have been performed

Same as above.

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.

@hichamboushaba hichamboushaba added type: enhancement A request for an enhancement. status: do not merge Dependent on another PR, ready for review but not ready for merge. feature: order details Related to order details. labels Sep 9, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Sep 9, 2024

1 Warning
⚠️ This PR is assigned to the milestone 20.4. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 9, 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
Commit2e53f06
Direct Downloadwoocommerce-wear-prototype-build-pr12566-2e53f06.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 9, 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
Commit2e53f06
Direct Downloadwoocommerce-prototype-build-pr12566-2e53f06.apk

@hichamboushaba hichamboushaba changed the base branch from issue/12525-custom-field-creation to trunk September 11, 2024 08:30
@hichamboushaba hichamboushaba removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Sep 11, 2024
@hichamboushaba hichamboushaba marked this pull request as ready for review September 11, 2024 09:27
@hichamboushaba hichamboushaba requested review from JorgeMucientes and irfano and removed request for JorgeMucientes September 11, 2024 09:28
@hichamboushaba hichamboushaba added this to the 20.4 milestone Sep 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 70.73171% with 12 lines in your changes missing coverage. Please review.

Project coverage is 40.61%. Comparing base (75e5ead) to head (2e53f06).
Report is 11 commits behind head on trunk.

Files with missing lines Patch % Lines
...merce/android/ui/compose/component/OverflowMenu.kt 0.00% 6 Missing ⚠️
...roid/ui/customfields/list/CustomFieldsViewModel.kt 84.00% 2 Missing and 2 partials ⚠️
...customfields/editor/CustomFieldsEditorViewModel.kt 80.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #12566      +/-   ##
============================================
+ Coverage     40.60%   40.61%   +0.01%     
- Complexity     5677     5680       +3     
============================================
  Files          1228     1228              
  Lines         69062    69091      +29     
  Branches       9568     9572       +4     
============================================
+ Hits          28044    28064      +20     
- Misses        38437    38444       +7     
- Partials       2581     2583       +2     

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

Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏻 Works well! I added a comment and will approve and merge after you respond to it.

Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

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

Good job! LGTM! You'll probably do, but as a reminder, don't forget to add RELEASE NOTES in one of your PRs for the custom fields feature.

@hichamboushaba
Copy link
Member Author

You'll probably do, but as a reminder, don't forget to add RELEASE NOTES in one of your PRs for the custom fields feature.

Thanks @irfano for the reminder, what we do generally when working on a feature that's behind a feature flag, is that we add the release notes entry in the PR that enables or removes the feature flag, so that it's not announced before we release it.

@hichamboushaba hichamboushaba merged commit b3f80f7 into trunk Sep 12, 2024
13 of 14 checks passed
@hichamboushaba hichamboushaba deleted the issue/12563-delete-custom-fields branch September 12, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: order details Related to order details. type: enhancement A request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Custom Fields] Deleting custom fields
5 participants