Skip to content

[Refunds] Fix taxes handling #14064

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

Open
wants to merge 10 commits into
base: issue/WOOMOB-394-refunds-networking-update
Choose a base branch
from
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- [*] Fixes for the crash when tax lines JSON is malformed [https://github.com/woocommerce/woocommerce-android/pull/14049]
- [*] Minor design adjustments of the coupon creation flow.
- [***] We added products search into Woo POS [https://github.com/woocommerce/woocommerce-android/pull/14029]
- [*] Fixed an issue with taxes not being correctly included during refunds [https://github.com/woocommerce/woocommerce-android/pull/14064]

22.3
-----
Expand Down
16 changes: 13 additions & 3 deletions WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ data class Order(
val parent: Long? = null,
val configuration: ProductConfiguration? = null,
val configurationKey: Long? = null,
val containsMetadata: Boolean = false
val containsMetadata: Boolean = false,
val taxes: List<LineTaxEntry> = emptyList(),
) : Parcelable {
@IgnoredOnParcel
val uniqueId: Long = ProductHelper.productOrVariationId(productId, variationId)
Expand Down Expand Up @@ -205,10 +206,11 @@ data class Order(
val methodId: String?,
val methodTitle: String,
val totalTax: BigDecimal,
val total: BigDecimal
val total: BigDecimal,
val taxes: List<LineTaxEntry>
) : Parcelable {
constructor(methodId: String, methodTitle: String, total: BigDecimal) :
this(0L, methodId, methodTitle, BigDecimal.ZERO, total)
this(0L, methodId, methodTitle, BigDecimal.ZERO, total, emptyList())
}

@Parcelize
Expand All @@ -228,6 +230,7 @@ data class Order(
val total: BigDecimal,
val totalTax: BigDecimal,
var taxStatus: FeeLineTaxStatus,
val taxes: List<LineTaxEntry>
) : Parcelable {
fun getTotalValue(): BigDecimal = total + totalTax

Expand All @@ -238,6 +241,7 @@ data class Order(
total = BigDecimal.ZERO,
totalTax = BigDecimal.ZERO,
taxStatus = FeeLineTaxStatus.UNKNOWN,
taxes = emptyList()
)
}

Expand Down Expand Up @@ -275,6 +279,12 @@ data class Order(
}
}

@Parcelize
data class LineTaxEntry(
val rateId: Long,
val taxAmount: BigDecimal
) : Parcelable

fun getBillingName(defaultValue: String): String {
return when {
billingAddress.firstName.isEmpty() && billingAddress.lastName.isEmpty() -> defaultValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.wordpress.android.fluxc.model.metadata.get
import org.wordpress.android.fluxc.model.order.FeeLineTaxStatus
import org.wordpress.android.fluxc.model.order.OrderAddress
import org.wordpress.android.fluxc.model.order.TaxLine
import org.wordpress.android.fluxc.model.order.WCLineTaxEntry
import org.wordpress.android.fluxc.network.rest.wpcom.wc.order.OrderMappingConst.CHARGE_ID_KEY
import org.wordpress.android.fluxc.network.rest.wpcom.wc.order.OrderMappingConst.SHIPPING_PHONE_KEY
import org.wordpress.android.fluxc.persistence.entity.OrderEntity
Expand Down Expand Up @@ -84,7 +85,8 @@ class OrderMapper @Inject constructor(
FeeLineTaxStatus.Taxable -> Order.FeeLine.FeeLineTaxStatus.TAXABLE
FeeLineTaxStatus.None -> Order.FeeLine.FeeLineTaxStatus.NONE
else -> Order.FeeLine.FeeLineTaxStatus.UNKNOWN
}
},
taxes = it.taxes?.mapLineTaxes() ?: emptyList()
)
}

Expand All @@ -105,10 +107,12 @@ class OrderMapper @Inject constructor(
methodId = it.methodId,
methodTitle = it.methodTitle ?: StringUtils.EMPTY,
totalTax = it.totalTax?.toBigDecimalOrNull() ?: BigDecimal.ZERO,
total = it.total?.toBigDecimalOrNull() ?: BigDecimal.ZERO
total = it.total?.toBigDecimalOrNull() ?: BigDecimal.ZERO,
taxes = it.taxes?.mapLineTaxes() ?: emptyList(),
)
}

@Suppress("CyclomaticComplexMethod")
private fun List<WCLineItem>.mapLineItems(): List<Item> =
this.filter { it.productId != null && it.id != null }
.map {
Expand All @@ -129,7 +133,8 @@ class OrderMapper @Inject constructor(
},
it.bundledBy?.toLongOrNull() ?: it.compositeParent?.toLongOrNull(),
configurationKey = it.configurationKey,
containsMetadata = it.metaData?.isNotEmpty() ?: false
containsMetadata = it.metaData?.isNotEmpty() ?: false,
taxes = it.taxes?.mapLineTaxes() ?: emptyList()
)
}

Expand Down Expand Up @@ -171,4 +176,11 @@ class OrderMapper @Inject constructor(
discount = it.discount,
)
}

private fun List<WCLineTaxEntry>.mapLineTaxes(): List<Order.LineTaxEntry> = map {
Order.LineTaxEntry(
rateId = it.rateId ?: 0L,
taxAmount = it.total ?: BigDecimal.ZERO,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ class IssueRefundViewModel @Inject constructor(
if (refundByItemsStateLiveData.hasInitialValue) {
refundByItemsState = refundByItemsState.copy(
currency = order.currency,
subtotal = formatCurrency(BigDecimal.ZERO),
taxes = formatCurrency(BigDecimal.ZERO),
shippingSubtotal = formatCurrency(order.shippingTotal),
shippingTaxes = formatCurrency(order.shippingLines.sumByBigDecimal { it.totalTax }),
feesSubtotal = formatCurrency(order.feesTotal),
Expand All @@ -230,8 +228,8 @@ class IssueRefundViewModel @Inject constructor(
orderItem = it,
maxQuantity = maxQuantity,
quantity = selectedQuantity,
subtotal = formatCurrency(BigDecimal.ZERO),
taxes = formatCurrency(BigDecimal.ZERO)
subtotal = BigDecimal.ZERO,
taxes = emptyList()
)
}
updateRefundItems(items)
Expand Down Expand Up @@ -305,7 +303,8 @@ class IssueRefundViewModel @Inject constructor(
refundSummaryState = refundSummaryState.copy(
isFormEnabled = true,
previouslyRefunded = formatCurrency(order.refundTotal),
refundAmount = formatCurrency(commonState.refundTotal)
refundAmount = commonState.refundTotal,
refundAmountFormatted = formatCurrency(commonState.refundTotal)
)

triggerEvent(ShowRefundSummary)
Expand Down Expand Up @@ -401,11 +400,12 @@ class IssueRefundViewModel @Inject constructor(
selectedFees?.forEach { allItems.add(it.toDataModel()) }

return refundStore.createItemsRefund(
selectedSite.get(),
order.id,
refundSummaryState.refundReason ?: "",
true,
gateway.supportsRefunds,
site = selectedSite.get(),
orderId = order.id,
amount = refundSummaryState.refundAmount,
reason = refundSummaryState.refundReason ?: "",
restockItems = true,
autoRefund = gateway.supportsRefunds,
items = allItems
)
}
Expand Down Expand Up @@ -510,13 +510,6 @@ class IssueRefundViewModel @Inject constructor(
refundSummaryState = refundSummaryState.copy(isSummaryTextTooLong = currLength > maxLength)
}

fun onProductsRefundAmountChanged(newAmount: BigDecimal) {
refundByItemsState = refundByItemsState.copy(
productsRefund = newAmount,
formattedProductsRefund = formatCurrency(newAmount)
)
}

fun onRefundQuantityChanged(uniqueId: Long, newQuantity: Int) {
val newItems = getUpdatedItemList(uniqueId, newQuantity)
updateRefundItems(newItems)
Expand All @@ -535,8 +528,6 @@ class IssueRefundViewModel @Inject constructor(
refundByItemsState = refundByItemsState.copy(
productsRefund = productsRefund,
formattedProductsRefund = formatCurrency(productsRefund),
taxes = formatCurrency(taxes),
subtotal = formatCurrency(subtotal),
selectButtonTitle = selectButtonTitle
)
}
Expand All @@ -549,8 +540,8 @@ class IssueRefundViewModel @Inject constructor(
var newItem = it.copy(quantity = newQuantity, maxQuantity = maxQuantities[uniqueId] ?: 0f)

// Update the subtotal and taxes based on the new quantity
val subtotal = formatCurrency(newItem.calculateTotalSubtotal())
val taxes = formatCurrency(newItem.calculateTotalTaxes())
val subtotal = newItem.calculateTotalSubtotal()
val taxes = newItem.calculateTaxesList()
newItem = newItem.copy(subtotal = subtotal, taxes = taxes)

newItems.add(newItem)
Expand Down Expand Up @@ -786,8 +777,6 @@ class IssueRefundViewModel @Inject constructor(
val currency: String? = null,
val productsRefund: BigDecimal = BigDecimal.ZERO,
val formattedProductsRefund: String? = null,
val subtotal: String? = null,
val taxes: String? = null,
val feesSubtotal: String? = null,
val feesTaxes: String? = null,
val feesRefund: BigDecimal = BigDecimal.ZERO,
Expand Down Expand Up @@ -821,7 +810,8 @@ class IssueRefundViewModel @Inject constructor(
data class RefundSummaryViewState(
val isFormEnabled: Boolean? = null,
val previouslyRefunded: String? = null,
val refundAmount: String? = null,
val refundAmount: BigDecimal? = null,
val refundAmountFormatted: String? = null,
val refundMethod: String? = null,
val refundReason: String? = null,
val isMethodDescriptionVisible: Boolean? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,8 @@ class RefundProductListAdapter(
.into(productImageView)
} ?: productImageView.setImageResource(R.drawable.ic_product)

subtotalTextView.text = item.subtotal

taxesTextView.text = item.taxes
subtotalTextView.text = formatCurrency(item.subtotal)
taxesTextView.text = formatCurrency(item.taxesTotal)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class RefundSummaryFragment : BaseFragment(R.layout.fragment_refund_summary), Ba
new.isSubmitButtonEnabled.takeIfNotEqualTo(old?.isSubmitButtonEnabled) {
binding.refundSummaryBtnRefund.isEnabled = new.isSubmitButtonEnabled
}
new.refundAmount?.takeIfNotEqualTo(old?.refundAmount) {
new.refundAmountFormatted?.takeIfNotEqualTo(old?.refundAmountFormatted) {
binding.refundSummaryRefundAmount.text = it
binding.toolbar.title = resources.getString(
R.string.order_refunds_title_with_amount, it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,13 @@ fun ProductRefundListItem.calculateTotalTaxes(): BigDecimal {
val singleItemTax = orderItem.totalTax.divide(orderItem.quantity.toBigDecimal(), 2, HALF_UP)
return quantity.times(singleItemTax)
}

fun ProductRefundListItem.calculateTaxesList(): List<TaxRefund> {
val quantity = quantity.toBigDecimal()
val taxes = orderItem.taxes

return taxes.map {
val tax = it.taxAmount.divide(orderItem.quantity.toBigDecimal(), 2, HALF_UP)
TaxRefund(it.rateId, quantity.times(tax))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,69 +5,87 @@ import com.woocommerce.android.model.Order
import kotlinx.parcelize.Parcelize
import org.wordpress.android.fluxc.model.refunds.RefundRequestItem
import org.wordpress.android.fluxc.model.refunds.RefundRequestTax
import java.math.RoundingMode.HALF_UP
import java.math.BigDecimal

@Parcelize
data class ProductRefundListItem(
val orderItem: Order.Item,
val maxQuantity: Float = 0f,
val quantity: Int = 0,
val subtotal: String? = null,
val taxes: String? = null
) : Parcelable {
val availableRefundQuantity
get() = maxQuantity.toInt()
sealed class RefundItem : Parcelable {
Copy link
Member Author

Choose a reason for hiding this comment

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

This refactoring is not really needed for this PR, but it will be useful for a future refactoring of the IssueRefundViewModel.

abstract val itemId: Long
abstract val quantity: Int?
abstract val subtotal: BigDecimal
abstract val taxes: List<TaxRefund>

fun toDataModel(): RefundRequestItem {
return RefundRequestItem(
itemId = orderItem.itemId,
itemId = itemId,
quantity = quantity,
refundTotal = quantity.toBigDecimal().times(orderItem.price),
refundTax = listOf(
refundTotal = subtotal,
refundTax = taxes.map {
RefundRequestTax(
taxRateId = 0L,
refundTotal = orderItem.totalTax.divide(orderItem.quantity.toBigDecimal(), 2, HALF_UP)
.times(quantity.toBigDecimal())
taxRateId = it.rateId,
refundTotal = it.tax
)
)
}
)
}
}

@Parcelize
data class TaxRefund(
val rateId: Long,
val tax: BigDecimal,
) : Parcelable

@Parcelize
data class ProductRefundListItem(
val orderItem: Order.Item,
val maxQuantity: Float = 0f,
override val quantity: Int = 0,
override val subtotal: BigDecimal = BigDecimal.ZERO,
override val taxes: List<TaxRefund> = emptyList()
) : RefundItem() {
override val itemId: Long
get() = orderItem.itemId

val availableRefundQuantity
get() = maxQuantity.toInt()

val taxesTotal: BigDecimal
get() = taxes.sumOf { it.tax }
}

@Parcelize
data class ShippingRefundListItem(
val shippingLine: Order.ShippingLine
) : Parcelable {
fun toDataModel(): RefundRequestItem {
return RefundRequestItem(
shippingLine.itemId,
quantity = 1, // Hardcoded because a shipping line always has a quantity of 1
refundTotal = shippingLine.total,
refundTax = listOf(
RefundRequestTax(
taxRateId = 0L,
refundTotal = shippingLine.totalTax
)
) : RefundItem() {
override val itemId: Long
get() = shippingLine.itemId
override val quantity: Int?
get() = null
override val subtotal: BigDecimal
get() = shippingLine.total
override val taxes: List<TaxRefund>
get() = shippingLine.taxes.map {
TaxRefund(
rateId = it.rateId,
tax = it.taxAmount
)
)
}
}
}

@Parcelize
data class FeeRefundListItem(
val feeLine: Order.FeeLine
) : Parcelable {
fun toDataModel(): RefundRequestItem {
return RefundRequestItem(
feeLine.id,
quantity = 1, // Hardcoded because a fee line always has a quantity of 1
refundTotal = feeLine.total,
refundTax = listOf(
RefundRequestTax(
taxRateId = 0L,
refundTotal = feeLine.totalTax,
)
) : RefundItem() {
override val itemId: Long
get() = feeLine.id
override val quantity: Int?
get() = null
override val subtotal: BigDecimal
get() = feeLine.total
override val taxes: List<TaxRefund>
get() = feeLine.taxes.map {
TaxRefund(
rateId = it.rateId,
tax = it.taxAmount
)
)
}
}
}
Loading