Skip to content
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

Creates two flavors for easier publication of BillingClient 3 compatible library #444

Merged
merged 22 commits into from Feb 7, 2022

Conversation

vegaro
Copy link
Contributor

@vegaro vegaro commented Jan 14, 2022

[sc-12566]

Unity IAP is not compatible with the BillingClient 4 yet. We point devs to use an old version of purchases-unity that has an old version of purchases-android with BillingClient 3.

This is bad because those versions are missing bugfixes, identity v3, etags, etc.

This PR adds another flavor of purchases-android that uses BillingClient 3.

Tested the deploys in #446

@vegaro vegaro mentioned this pull request Jan 14, 2022
Comment on lines 17 to 24
val Purchase.firstSku: String
get() = skus[0].also {
if (skus.size > 0) {
log(LogIntent.GOOGLE_WARNING, BillingStrings.BILLING_PURCHASE_MORE_THAN_ONE_SKU)
}
}

val Purchase.listOfSkus: ArrayList<String>
Copy link
Member

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Glad you like the name because I didn't know what to name it haha. We need something different than skus so it doesn't collide 🤷‍♂️

purchases/build.gradle Outdated Show resolved Hide resolved
@beylmk
Copy link
Contributor

beylmk commented Jan 14, 2022

😻 this is such a cool idea

@vegaro vegaro force-pushed the billing-client-3-using-flavors branch from 70f8f98 to 99a7c7a Compare January 19, 2022 12:18
@shortcut-integration
Copy link

library.gradle Show resolved Hide resolved
library.gradle Outdated Show resolved Hide resolved
Copy link
Member

@aboedo aboedo left a comment

Choose a reason for hiding this comment

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

looking great so far!

Comment on lines 10 to 11
newBillingImplementation "com.android.billingclient:billing:$billingVersion"
oldBillingCompileOnly "com.android.billingclient:billing:$oldBillingVersion"
Copy link
Member

Choose a reason for hiding this comment

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

this may just be me, but I'm somewhat allergic to new vs old naming, since it doesn't tell you much about when it was considered "new" or what being "old" means. I'd personally go with names that clearly outline billing 3 vs billing 4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for some reason I thought I couldn't add numbers to the variant names but looks like we can so I will just go with billingClient3 and billingClient4

library.gradle Show resolved Hide resolved
@vegaro vegaro force-pushed the billing-client-3-using-flavors branch from 96d9012 to 17aeced Compare January 31, 2022 17:16
@vegaro vegaro marked this pull request as ready for review February 4, 2022 16:19
@vegaro
Copy link
Contributor Author

vegaro commented Feb 4, 2022

This is finally ready for review. I fixed tests and added Amazon v2 dependency, which thankfully didn't introduced any code changes.

@vegaro vegaro requested a review from a team February 4, 2022 16:20
Copy link
Member

@aboedo aboedo left a comment

Choose a reason for hiding this comment

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

amazing job on this one!!

@@ -0,0 +1,25 @@
package com.revenuecat.purchases.common
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's be worth having the file name reflect the fact that this is billing client 4. I see that the path does say latestDependencies, but like, when you're looking up a file by name, you might miss the path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good idea. It's easier to look for the files. The thing with the path is that gradle will compile anything in main and in latestDependencies if the latestDependencies flavor is selected. It will compile anything in unityIAP and main if the unityIAP is selected. Pretty magical huh!

this.sku
}, orderId: ${this.orderId}, purchaseToken: ${this.purchaseToken}"

val Purchase.firstSku: String
Copy link
Member

Choose a reason for hiding this comment

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

❤️

Comment on lines -493 to 489
type = productTypes[purchase.sku]
presentedOffering = presentedOfferingsByProductIdentifier[purchase.sku]
type = productTypes[purchase.firstSku]
presentedOffering = presentedOfferingsByProductIdentifier[purchase.firstSku]
}
Copy link
Member

Choose a reason for hiding this comment

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

👏

@vegaro vegaro merged commit aa7ff5c into main Feb 7, 2022
@vegaro vegaro deleted the billing-client-3-using-flavors branch February 7, 2022 19:13
@vegaro vegaro mentioned this pull request Feb 7, 2022
@hansemannn
Copy link

Quick question if you allow: What exactly would the Unity IAP have with Play Billing v4? As far as I can see, it's mostly abstracted under the hood, but I am curious what could have been broken. Thank you!

Copy link
Contributor

@NachoSoto NachoSoto left a comment

Choose a reason for hiding this comment

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

I'm not able to run individual tests through Android Studio (right-clicking on a test and running). I think I narrowed it down to this commit. After it I get this now:

Task 'testDebugUnitTest' not found in project ':common'.

if (publishVariant == "unityIAPRelease") {
project.ext.POM_ARTIFACT_ID = artifactId + "-unityiap"
}
println "$project.ext.POM_ARTIFACT_ID"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to leave this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No lol. But I will remove it. Better late than never haha. Sorry I missed this until now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vegaro
Copy link
Contributor Author

vegaro commented Feb 15, 2022

@NachoSoto what build variants do you see in the Build Variants section of Android Studio?

@NachoSoto
Copy link
Contributor

Screen Shot 2022-02-15 at 12 12 48

The workaround is for me to change the automatically generated run configuration from :common:testDebugUnitTest to :common:test.

@NachoSoto
Copy link
Contributor

I filed #490.

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.

None yet

5 participants