-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add config flag to include/exclude PayPal invoice items #696
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
Conversation
32972f1 to
7e535df
Compare
7e535df to
d9230c3
Compare
d9230c3 to
6a30617
Compare
| val payPalKoinModule = | ||
| module { | ||
| single<PayPalRepository> { PayPalRepositoryImpl(get(), get(QUALIFIER_PAYPAL_HTTP_CLIENT), get()) } | ||
| single<PayPalRepository> { PayPalRepositoryImpl(get(), get(), get(QUALIFIER_PAYPAL_HTTP_CLIENT), get()) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enabled by default until tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a configurable flag to control whether itemized invoice details are included in PayPal order creation requests. The feature allows NEWM to toggle between showing detailed line items (Distribution cost, Royalty split(s) fee, Service fee) or just the total amount when creating PayPal orders.
Key changes:
- Added
paypal.itemizedInvoiceEnabledconfiguration flag (defaults totrue) - Made the
itemsfield nullable in PayPal API request model - Updated PayPal repository to conditionally include invoice items based on config
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| PayPalRepositoryImpl.kt | Adds ConfigRepository dependency and conditional logic to include/exclude invoice items based on config flag |
| PayPalCreateOrderRequest.kt | Makes items field nullable to support optional itemization |
| PayPalKoinModule.kt | Updates dependency injection to include ConfigRepository |
| V83__ConfigUpdates.kt | Database migration that creates the config entry with default value 'true' |
| ConfigRepository.kt | Adds constant for the new config key |
| items = takeIf { configRepository.getBoolean(CONFIG_KEY_PAYPAL_ITEMIZED_INVOICE_ENABLED) }?.let { | ||
| listOf( | ||
| PayPalItem( | ||
| name = "Distribution cost", | ||
| unitPriceUsd = paymentOption.dspPrice.toBigDecimalUsd() | ||
| ), | ||
| PayPalItem( | ||
| name = "Royalty split(s) fee", | ||
| unitPriceUsd = paymentOption.collabPrice.toBigDecimalUsd() | ||
| ), | ||
| PayPalItem( | ||
| name = "Service fee", | ||
| unitPriceUsd = paymentOption.mintPrice.toBigDecimalUsd() | ||
| ) | ||
| ) | ||
| ) | ||
| } |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The takeIf is being called on the wrong receiver. Currently it's called on an implicit receiver (likely this from outer scope), but it should be called on the boolean result of configRepository.getBoolean(). This will cause items to always be null when the condition should be false, but will incorrectly return the PayPalRepositoryImpl instance when it should check the config flag. Replace with: if (configRepository.getBoolean(CONFIG_KEY_PAYPAL_ITEMIZED_INVOICE_ENABLED)) listOf(...) else null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wlara I agree with this comment. A simple items = if (configRepository.getBoolean()) {...} else null seems like clearer code.
6a30617 to
fcd10d8
Compare
No description provided.