Skip to content

Virtual Currency Balances #2210

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Mar 10, 2025
Merged

Virtual Currency Balances #2210

merged 19 commits into from
Mar 10, 2025

Conversation

fire-at-will
Copy link
Contributor

@fire-at-will fire-at-will commented Mar 3, 2025 •
edited
Loading

Motivation

This PR allows developers to check the balances of their Virtual Currencies through the SDK's CustomerInfo object.

Description

This PR allows developers to access the balances of their Virtual Currencies like so:

val coinBalance: Long? = customerInfo.virtualCurrencies["COIN"]?.balance

New APIs

It accomplishes this with a few API additions:

data class CustomerInfo(
   // ...
   val virtualCurrencies: Map<String, VirtualCurrencyInfo>,
)

@Parcelize
data class VirtualCurrencyInfo internal constructor(
    val balance: Long,
) : Parcelable

The balance property is a Long because Longs represent signed 64 bit integers, which is what is used in the iOS SDK and backend.

Testing

  • Adds the new APIs to the Kotlin & Java API Testers
  • Adds unit tests for parsing the virtual currency balances from JSON into the new objects on CustomerInfo
  • Manually tested in the purchase tester app

@fire-at-will fire-at-will self-assigned this Mar 3, 2025
@emerge-tools Emerge Tools
Copy link

emerge-tools bot commented Mar 3, 2025 •
edited
Loading

📸 Snapshot Test

262 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
TestPurchasesUIAndroidCompatibility
com.revenuecat.testpurchasesuiandroidcompatibility		 0 0 0 0 262 0 N/A

🛸 Powered by Emerge Tools

@codecov Codecov
Copy link

codecov bot commented Mar 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.45%. Comparing base (210e75e) to head (6946a09).

Files with missing lines Patch % Lines
...in/com/revenuecat/purchases/VirtualCurrencyInfo.kt 77.77% 0 Missing and 2 partials ⚠️
Additional details and impacted files 
@@                   Coverage Diff                    @@
##           virtual-currency-dev    #2210      +/-   ##
========================================================
+ Coverage                 80.42%   80.45%   +0.02%     
========================================================
  Files                       278      279       +1     
  Lines                      9467     9490      +23     
  Branches                   1336     1339       +3     
========================================================
+ Hits                       7614     7635      +21     
  Misses                     1293     1293              
- Partials                    560      562       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fire-at-will fire-at-will marked this pull request as ready for review March 5, 2025 15:58
fire-at-will
fire-at-will commented Mar 5, 2025
View reviewed changes
purchases/src/main/kotlin/com/revenuecat/purchases/VirtualCurrencyInfo.kt Outdated

companion object {
fun fromJson(json: JSONObject): VirtualCurrencyInfo {
val balance = json.getLong("amount")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The value returned by the server may change in the future to "balance" in the future - we'll update that in a follow-up PR if/when it happens.

@fire-at-will fire-at-will changed the title add virtual currency balances Virtual Currency Balances Mar 5, 2025
@fire-at-will fire-at-will requested review from MarkVillacampa and a team March 5, 2025 17:49
tonidero
tonidero reviewed Mar 6, 2025
View reviewed changes
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Mostly some questions but looking great!

JayShortway
JayShortway reviewed Mar 6, 2025
View reviewed changes
Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

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

Good stuff! No real comments besides Toni's.

tonidero
tonidero approved these changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

I think this look great! Great job on this!

purchases/src/main/kotlin/com/revenuecat/purchases/CustomerInfo.kt
@@ -50,6 +52,9 @@ data class CustomerInfo(
val originalAppUserId: String,
val managementURL: Uri?,
val originalPurchaseDate: Date?,

@ExperimentalPreviewRevenueCatPurchasesAPI
val virtualCurrencies: Map<String, VirtualCurrencyInfo>,
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the problems with data classes is that this modifies the constructor + copy constructor for CustomerInfo, in case someone is using it... We could try to limit the breaking change by adding a deprecated constructor without the new param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea @tonidero, I've added a deprecated constructor without the new param in 1126563 👍

mvichos
mvichos reviewed Mar 10, 2025
View reviewed changes
Copy link

@mvichos mvichos left a comment

Choose a reason for hiding this comment

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

FYI: This has been merged. Some fields in the backend response have been renamed.

JayShortway
JayShortway approved these changes Mar 10, 2025
View reviewed changes
Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

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

Looking good! 💪 Avoiding the breaking constructor in CustomerInfo might be nice, as Toni suggested. Then we at least remain source compatible.

@fire-at-will fire-at-will merged commit 0312796 into virtual-currency-dev Mar 10, 2025
11 checks passed
@fire-at-will fire-at-will deleted the virtual-currency-balances branch March 10, 2025 19:04
This was referenced Mar 11, 2025
Closed
Closed
Closed
@fire-at-will fire-at-will mentioned this pull request Mar 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvichos mvichos mvichos left review comments

@tonidero tonidero tonidero approved these changes

@JayShortway JayShortway JayShortway approved these changes

@MarkVillacampa MarkVillacampa Awaiting requested review from MarkVillacampa

Assignees

@fire-at-will fire-at-will

Labels
feat:Virtual Currency pr:feat A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fire-at-will @tonidero @mvichos @JayShortway