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

[Issue 10719]Added Support For Shipping Tax in Order Editing

Open jd-alexander opened this issue 1 year ago • 7 comments

Closes: #10719

Description

I have implemented an enhancement to display the shipping tax value within the order creation form. This feature aims to provide more detailed tax information, enhancing transparency and accuracy in order calculations.

There is an associated FluxC PR with the data layer changes needed to support the addition of this functionality.

View the PR here: https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2956

Implementation

  • Label: The shipping tax is labeled as "Shipping Tax" for clear identification.
  • Format: Consistent with existing tax lines, the format utilized is Shipping Tax and then the tax value, ensuring uniformity and ease of understanding.
  • Placement: Positioned at the bottom of the tax lines, this new entry integrates seamlessly with the current layout, maintaining a structured and logical flow of information.

Changes Summary

A total of 6 changes were made across the codebase to integrate this feature:

  • Order Model Update: Introduced a new field shippingTax in the Order data class to store the shipping tax value.
  • Order Mapper Adjustment: Updated OrderMapper to handle the mapping of the new shippingTax field from database entities to the model.
  • UI Enhancements: Modified the OrderCreateEditTotalsHelper to include the shipping tax line in the order totals UI, ensuring it is displayed to the user.
  • String Resource Addition: Added a new string resource for the "Shipping Tax" label to support localization and consistency across the app.

Testing instructions

  1. Create an order eligible for shipping.
  2. Ensure your store has a tax rate within the WooCommerce settings -> Tax -> Standard rates.
  3. Ensure you add a shipping option to the order.
  4. Once this is done add an amount for the shipping entry and press recalculate.
  5. You should notice the shipping tax appear within the UI.
  6. Save the order.
  7. Open the Woo Mobile Android app.
  8. Go to the Orders tab.
  9. Click on the Order that has the shipping tax rate.
  10. Press "Edit".
  11. Expand the "Order Total" pane.
  12. Notice the "Shipping Tax" underneath the "Taxes" header.

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.

jd-alexander avatar Feb 13 '24 23:02 jd-alexander

📲 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
Commit9a47f3777874a36e3794893cd91b576664225eec
Direct Downloadwoocommerce-prototype-build-pr10794-9a47f37.apk

wpmobilebot avatar Feb 13 '24 23:02 wpmobilebot

1 Warning
:warning: This PR is assigned to the milestone 17.5. 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 :no_entry_sign: Danger

dangermattic avatar Feb 14 '24 00:02 dangermattic

@samiuelson this PR has another revision to go but I decided to put it up to get a first pass from you. I will be AFK tomorrow. I will continue to work on this when I get back 👍🏾

jd-alexander avatar Feb 14 '24 00:02 jd-alexander

Nice work, @jd-alexander!

Thanks Sam!

Make sure to update the FluxC version to the trunk-based artifact version after you merge FluxC changes into trunk.

Good point. Just to confirm you mean, I would merge the FluxC PR into trunk and at that time I take that merge commit; the binary generated for it and I put that within the FluxC version of the woocommerce Android app? Let me know if that's what you mean.

Did you have a chance to review the dependencies diff https://github.com/woocommerce/woocommerce-android/pull/10794#issuecomment-1942857206 introduced by this PR? It would be nice to skim through their release notes to make sure we don't introduce any breaking changes.

Good catch!

The only thing I want to update here is the FluxC versions but it somehow pulled in some other library updates. I am going to revert them but also investigate what caused them to be updated.

jd-alexander avatar Feb 16 '24 01:02 jd-alexander

Good point. Just to confirm you mean, I would merge the FluxC PR into trunk and at that time I take that merge commit; the binary generated for it and I put that within the FluxC version of the woocommerce Android app? Let me know if that's what you mean.

Yes 👍🏼 The idea is to merge the FluxC feature first and update Woo last. This way we avoid conflicts and other problems in Woo.

samiuelson avatar Feb 16 '24 09:02 samiuelson

The only thing I want to update here is the FluxC versions but it somehow pulled in some other library updates. I am going to revert them but also investigate what caused them to be updated.

Simple FluxC version updates sometimes introduce transitive dependencies updates, so it's good to analyze the diff carefully. At a closer look, it looks like there are no updates introduced for external libraries this time. The diff just looks big at first glance. No need to revert anything!

samiuelson avatar Feb 16 '24 09:02 samiuelson

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 41.27%. Comparing base (672695b) to head (9a47f37).

Files Patch % Lines
...ers/creation/totals/OrderCreateEditTotalsHelper.kt 16.66% 4 Missing and 1 partial :warning:
...otlin/com/woocommerce/android/model/OrderMapper.kt 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #10794      +/-   ##
============================================
- Coverage     41.27%   41.27%   -0.01%     
- Complexity     5084     5085       +1     
============================================
  Files          1034     1034              
  Lines         59556    59565       +9     
  Branches       7989     7991       +2     
============================================
+ Hits          24582    24585       +3     
- Misses        32788    32792       +4     
- Partials       2186     2188       +2     

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

codecov-commenter avatar Feb 22 '24 19:02 codecov-commenter

The only thing I want to update here is the FluxC versions but it somehow pulled in some other library updates. I am going to revert them but also investigate what caused them to be updated.

Simple FluxC version updates sometimes introduce transitive dependencies updates, so it's good to analyze the diff carefully. At a closer look, it looks like there are no updates introduced for external libraries this time. The diff just looks big at first glance. No need to revert anything!

Thanks! I will merge in the morning.

jd-alexander avatar Feb 23 '24 05:02 jd-alexander

I'm not sure which milestone we want to target - I've moved it to 17.6. In case, you'd want this included in 17.5, please update the milestone and make sure it's merged before the codefreeze.

malinajirka avatar Feb 23 '24 08:02 malinajirka

Found 1 violations:

The PR caused the following dependency changes:

expand

-+--- org.wordpress:fluxc:2959-fa5de726746db9abca205b9c433f61bbb4fdb9ae
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:2959-fa5de726746db9abca205b9c433f61bbb4fdb9ae
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.0 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
-|    |    +--- com.google.crypto.tink:tink-android:1.5.0
-|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.0 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- org.apache.commons:commons-text:1.10.0 (*)
-|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
-|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
-|    |    +--- androidx.room:room-common:2.5.2 (*)
-|    |    +--- androidx.room:room-runtime:2.5.2 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
-|    +--- com.google.dagger:dagger:2.42 -> 2.50
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
++--- org.wordpress:fluxc:trunk-d4257a89ad5ded8ca5b1d2de9726e2d8787cc8bc
+|    +--- org.wordpress:wellsql:2.0.0
+|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-d4257a89ad5ded8ca5b1d2de9726e2d8787cc8bc
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.0 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
+|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
+|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
+|    |    +--- com.google.crypto.tink:tink-android:1.5.0
+|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.0 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+|    +--- org.apache.commons:commons-text:1.10.0 (*)
+|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
+|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
+|    |    +--- androidx.room:room-common:2.5.2 (*)
+|    |    +--- androidx.room:room-runtime:2.5.2 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
+|    +--- com.google.dagger:dagger:2.42 -> 2.50
+|    |    \--- javax.inject:javax.inject:1
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:2959-fa5de726746db9abca205b9c433f61bbb4fdb9ae
-     +--- org.wordpress:wellsql:2.0.0 (*)
-     +--- org.wordpress.fluxc:fluxc-annotations:2959-fa5de726746db9abca205b9c433f61bbb4fdb9ae
-     +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
-     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.0 (*)
-     +--- org.wordpress:fluxc:2959-fa5de726746db9abca205b9c433f61bbb4fdb9ae (*)
-     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-     +--- com.google.dagger:dagger:2.42 -> 2.50 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
-     \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:trunk-d4257a89ad5ded8ca5b1d2de9726e2d8787cc8bc
+     +--- org.wordpress:wellsql:2.0.0 (*)
+     +--- org.wordpress.fluxc:fluxc-annotations:trunk-d4257a89ad5ded8ca5b1d2de9726e2d8787cc8bc
+     +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
+     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.0 (*)
+     +--- org.wordpress:fluxc:trunk-d4257a89ad5ded8ca5b1d2de9726e2d8787cc8bc (*)
+     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+     +--- com.google.dagger:dagger:2.42 -> 2.50 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
+     \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)

Please review and act accordingly

<this is a auto generated comment from violation-comments-lib F7F8ASD8123FSDF>

<ACCUMULATED-VIOLATIONS>

wpmobilebot avatar Feb 23 '24 15:02 wpmobilebot