Skip to content

Validate if we can create tax item with custom details#41

Open
singh-sukhvir wants to merge 2 commits intomasterfrom
tax-rate-persistence
Open

Validate if we can create tax item with custom details#41
singh-sukhvir wants to merge 2 commits intomasterfrom
tax-rate-persistence

Conversation

@singh-sukhvir
Copy link
Contributor

No description provided.

@vlaskhilkevich vlaskhilkevich marked this pull request as ready for review July 22, 2024 10:28
Copy link
Contributor Author

@singh-sukhvir singh-sukhvir left a comment

Choose a reason for hiding this comment

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

Need some changes related to how we are creating tax item

Comment on lines +303 to +332
final InvoiceItem taxItemWithEffectiveRate = transactionLineDetailModel.getEffectiveRate() != null
? new PluginInvoiceItem(new Builder<>()
.withId(taxItem.getId())
.withInvoiceItemType(taxItem.getInvoiceItemType())
.withInvoiceId(taxItem.getInvoiceId())
.withAccountId(taxItem.getAccountId())
.withChildAccountId(taxItem.getChildAccountId())
.withStartDate(taxItem.getStartDate())
.withEndDate(taxItem.getEndDate())
.withAmount(taxItem.getAmount())
.withCurrency(taxItem.getCurrency())
.withDescription(taxItem.getDescription())
.withSubscriptionId(taxItem.getSubscriptionId())
.withBundleId(taxItem.getBundleId())
.withCatalogEffectiveDate(taxItem.getCatalogEffectiveDate())
.withProductName(taxItem.getProductName())
.withPrettyProductName(taxItem.getPrettyProductName())
.withPlanName(taxItem.getPlanName())
.withPrettyPlanName(taxItem.getPrettyPlanName())
.withPhaseName(taxItem.getPhaseName())
.withPrettyPhaseName(taxItem.getPrettyPhaseName())
.withRate(taxItem.getRate())
.withLinkedItemId(taxItem.getLinkedItemId())
.withUsageName(taxItem.getUsageName())
.withPrettyUsageName(taxItem.getPrettyUsageName())
.withQuantity(taxItem.getQuantity())
.withItemDetails(String.valueOf(transactionLineDetailModel.getEffectiveRate()))
.withCreatedDate(taxItem.getCreatedDate())
.withUpdatedDate(taxItem.getUpdatedDate())
.validate().build())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can we move this to a separate function. Something like below -

final String taxRate = String.valueOf(transactionLineDetailModel.getEffectiveRate())
final InvoiceItem taxItem = createTaxInvoiceItem(taxableItem, invoiceId, adjustmentItem, calculatedTax, description, taxRate)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having value "null" as a result of valueOf(null) is not what we want in this case

.withUsageName(taxItem.getUsageName())
.withPrettyUsageName(taxItem.getPrettyUsageName())
.withQuantity(taxItem.getQuantity())
.withItemDetails(String.valueOf(transactionLineDetailModel.getEffectiveRate()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, we discussed to save it as json considering we may want to add additional details in future.
Current format won't support that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vlaskhilkevich This PR was experimental. Feel free to create a separate PR with your changes.

@vlaskhilkevich
Copy link
Contributor

@singh-sukhvir this pr can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants