Make payments code safer

[MiSikora]

Description

When installments were being implemented I noticed that some parts of the design of the :payment module are not clear making its further development unsafe. This PR address that. The main change is that the base payments and offer payments are stored in separate maps making accessing data type-safe.

Testing Instructions

Code review should be enough as the :payment module is highly covered in tests. But you can try making a purchase with the prototype or the release variant.

Checklist

• If this is a user-facing change, I have added an entry in CHANGELOG.md
• Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
• I have considered whether it makes sense to add tests for my changes
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
• Any jetpack compose components I added or changed are covered by compose previews
• I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

I have tested any UI changes...

• with different themes
• with a landscape orientation
• with the device set to have a large display and font size
• for accessibility with TalkBack

[Copilot]

Pull request overview

This PR refactors the :payment module to improve type safety by separating base subscription plans and offer plans into distinct maps. The changes address design issues discovered during installment plan implementation, making the codebase safer for future development.

Changes:

• Split the single plans map into separate basePlans and offerPlans maps with stronger type constraints
• Improved error handling in mapper functions to return PaymentResult instead of throwing exceptions
• Refactored and split test cases for better clarity

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
modules/services/payment/src/main/kotlin/au/com/shiftyjelly/pocketcasts/payment/Data.kt	Core refactoring: separate maps for base and offer plans, improved type safety, enhanced error handling
modules/services/payment/src/test/kotlin/au/com/shiftyjelly/pocketcasts/payment/SubscriptionPlansTest.kt	Fixed spelling error, refactored tests into smaller focused tests, removed obsolete validation test
modules/services/payment/src/main/kotlin/au/com/shiftyjelly/pocketcasts/payment/PaymentResult.kt	Removed unnecessary constructor keyword from data class

Comments suppressed due to low confidence (1)

modules/services/payment/src/main/kotlin/au/com/shiftyjelly/pocketcasts/payment/Data.kt:126

• The offerPlanKeys generation now includes keys with both offer and isInstallment=true, which violates the business rule that installment plans cannot have promotional offers. These keys will always fail to match products (due to the isValidInstallment check on line 205) and create unnecessary failure entries in the offerPlans map. The previous code explicitly filtered out installment plans before creating offer keys. Consider restoring the filter: .filter { !it.isInstallment } before the flatMap to maintain the business rule explicitly and avoid creating invalid key combinations.

        private val offerPlanKeys = basePlanKeys
            .flatMap { baseKey ->
                SubscriptionOffer.entries.map { offer ->
                    baseKey.copy(offer = offer)
                }
            }

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

modules/services/payment/src/main/kotlin/au/com/shiftyjelly/pocketcasts/payment/Data.kt:126

• The offerPlanKeys generation creates keys with isInstallment=true when copying from installment base keys, but the findOfferPlan function always creates keys with isInstallment=false (the default). This means half of the entries in the offerPlans map can never be accessed through findOfferPlan. Consider filtering out installment keys before creating offer keys to avoid storing unnecessary entries in the offerPlans map.

        private val offerPlanKeys = basePlanKeys
            .flatMap { baseKey ->
                SubscriptionOffer.entries.map { offer ->
                    baseKey.copy(offer = offer)
                }
            }