-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[in_app_purchase] Add expiration date to Transaction #8030
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
c882310
to
c64a7b2
Compare
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.
Can you provide more details in PR description?
@@ -188,14 +188,15 @@ extension Product.PurchaseResult { | |||
extension Transaction { | |||
func convertToPigeon(restoring: Bool = false) -> SK2TransactionMessage { | |||
|
|||
let dateFromatter: DateFormatter = DateFormatter() | |||
dateFromatter.dateFormat = "yyyy-MM-dd HH:mm:ss" | |||
let dateFormatter: DateFormatter = DateFormatter() |
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.
nit: let dateFormatter = DateFormatter()
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.
(@feinstein we know you didn't write this and are just fixing the typo, but while you're here... 🙂 )
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.
Sure, will update shortly. Just keep in mind I am not a swift dev, so snippets like this one are really helpful!
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.
Done
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.
lgtm! just a couple nits about the pubspec.
@@ -28,3 +28,8 @@ dev_dependencies: | |||
|
|||
flutter: | |||
uses-material-design: true | |||
|
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.
I think these are triggering the repo check failures, you shouldn't need a these overrides
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.
I was under the impression, from the CONTRIBUITING docs, that we first send a PR with dependencies overrides, created by the repo tool, so all packages and examples can be tested with the latest changes. Then after the PR is approved I remove those in a new PR: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins But maybe I got it wrong?
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.
I was under the impression, from the CONTRIBUITING docs, that we first send a PR with dependencies overrides, created by the repo tool
That's for changes that span multiple packages; this PR only affects one package.
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.
Ah, I see, thanks, I thought it was needed to test the packages, using local references.
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.
Done
@@ -42,3 +42,8 @@ screenshots: | |||
path: doc/iap_ios.gif | |||
- description: 'Example of in-app purchase on android' | |||
path: doc/iap_android.gif | |||
|
|||
# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. | |||
# See https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins |
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.
this one can be removed too! The only pubspec.yaml change that this PR needs is the one in in_app_purchase_storekit
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.
Done
Done. |
1b1ed7a
to
c36dbcf
Compare
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.
Dang merge conflict with #7964. Can you regenerate the files as needed and bump the version and update CHANGELOG again? Sorry I know that's annoying.
It's ok, part of the life of a dev |
Make expirationDate non-required
c36dbcf
to
6f60a58
Compare
Format dart code
64663e3
to
e278b88
Compare
Reviewers, is this ready to |
LGTM! @feinstein you can add the |
@LouiseHsu Only project members can add labels; the final reviewer should add the label for non-member contributions. It looks like the PR that I just landed re-broke the version change; I'll fix that and add the label. |
(Reviewers, please remember as part of review to check that the type of the version increment follows semver; new public API additions are minor version changes, not bugfixes.) |
@stuartmorgan I though that would be the case, but when I saw past entries in the changelog the last ones were just incrementing patch, so I followed this. Thanks for the review and fixing it :). |
flutter/packages@72356fd...26e123a 2024-11-13 dasyad00@gmail.com [camera_windows] Set device media type for video preview explicitly (flutter/packages#7447) 2024-11-13 39979207+ThangVuNguyenViet@users.noreply.github.com [go_router] Add support for relative routes (flutter/packages#6825) 2024-11-13 pq@users.noreply.github.com [vector_graphics_compiler] fix a renamed method parameter lint (flutter/packages#8070) 2024-11-12 feinstein@users.noreply.github.com [in_app_purchase] Add expiration date to Transaction (flutter/packages#8030) 2024-11-12 stuartmorgan@google.com [various] Clean up contributing guides (flutter/packages#8032) 2024-11-12 ditman@gmail.com [ci] Remove web renderer option from tools. (flutter/packages#8055) 2024-11-12 stuartmorgan@google.com [url_launcher] Update Pigeon version for Linux (flutter/packages#8065) 2024-11-12 tobias@leafnode.se [go_router] Add support for preloading branches of StatefulShellRoute (revised solution) (flutter/packages#6467) 2024-11-12 stuartmorgan@google.com [pigeon] Make Linux type declarations public (flutter/packages#8040) 2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter from 73546b3 to c8510f2 (30 revisions) (flutter/packages#8042) 2024-11-11 magder@google.com Use dependabot multi-directory configuration for Android package updates (flutter/packages#8048) 2024-11-11 stuartmorgan@google.com [tools] Run `pub get` before `format` (flutter/packages#8052) 2024-11-11 stuartmorgan@google.com [file_selector] Fix Linux cancel regression (flutter/packages#8051) 2024-11-09 stuartmorgan@google.com [shared_preferences] Fix confusing language in README (flutter/packages#8049) 2024-11-08 magder@google.com Use dependabot multi-directory configuration for Android example gradle updates (flutter/packages#8036) 2024-11-08 43054281+camsim99@users.noreply.github.com [animations] Remove `.flutter-plugins` reference from example app (flutter/packages#8002) 2024-11-08 magder@google.com Group dependabot github-action update PRs, delete dead docker updates (flutter/packages#8044) 2024-11-08 37028599+EArminjon@users.noreply.github.com [vector_graphics_compiler] fix-null-exception (flutter/packages#8006) 2024-11-08 stuartmorgan@google.com [tools] Format Dart per-package (flutter/packages#8043) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Closes flutter/flutter#158226
Exposes the Transaction's expiration date, so this information can be used on the dart side.
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.