Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[in_app_purchase] Added currency code and numerical price to product detail model. #3794

Merged
merged 12 commits into from
Apr 9, 2021
Merged

[in_app_purchase] Added currency code and numerical price to product detail model. #3794

merged 12 commits into from
Apr 9, 2021

Conversation

BeMacized
Copy link
Contributor

Adds the currency code and numerical price to the ProductDetail model, so users of the plugin are able to take care of formatting.

Solves flutter/flutter#65758

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@BeMacized BeMacized requested review from cyanglaz and LHLL as code owners April 7, 2021 14:35
@google-cla google-cla bot added the cla: yes label Apr 7, 2021
mvanbeusekom
mvanbeusekom approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM!

cyanglaz
cyanglaz suggested changes Apr 7, 2021
View reviewed changes
...es/in_app_purchase/in_app_purchase/test/in_app_purchase_connection/product_details_test.dart
originalPriceAmountMicros: 1000,
);

ProductDetails productDetails =
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also test productDetails.currencyCode

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 main reason I only decided to test the rawPrice property is because it is the only one that has some additional logic in the constructors (String parsing for iOS, some math for Android).

I can of course also test currencyCode, but since that's nothing more than a value being set, just like all the other pre-existing properties, it does make me wonder if those should then also be tested (e.g. title, description, etc).

I'm curious if you have any thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

it does make me wonder if those should then also be tested (e.g. title, description, etc).

Ideally, if we can, it would be nice to test all the properties being set with the constructor. It would be awesome if you could add those in this PR!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing! The changes have been added.

cyanglaz
cyanglaz approved these changes Apr 9, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM

@cyanglaz
Copy link
Contributor

cyanglaz commented Apr 9, 2021

There seems to be some merge conflicts.

@cyanglaz cyanglaz added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Apr 9, 2021
@fluttergithubbot fluttergithubbot merged commit ee5d15e into flutter:master Apr 9, 2021
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 9, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 9, 2021
@engine-flutter-autoroll
ee5d15e [in_app_purchase] Added currency code and numerical price to …
4888650 
…product detail model. (flutter/plugins#3794)
@cyanglaz cyanglaz mentioned this pull request Apr 9, 2021
Closed
fluttergithubbot pushed a commit to flutter/flutter that referenced this pull request Apr 9, 2021
@engine-flutter-autoroll
ee5d15e [in_app_purchase] Added currency code and numerical price to …
e2dc794 
…product detail model. (flutter/plugins#3794) (#80124)
bhaveshptl pushed a commit to bhaveshptl/plugins that referenced this pull request Apr 23, 2021
@bhaveshhzt
Merge branch 'master' into webview_file_picker
72e55e2 
* master: (397 commits)
  [in_app_purchase] Implementation of platform interface (flutter#3781)
  [google_sign_in] Add todo WRT correctly setting X-Goog-AuthUser header (flutter#3819)
  [tools] fix version check command not working for new packages (flutter#3818)
  [camera] android-rework part 1: Base classes to support Android Camera features (flutter#3795)
  fix MD (flutter#3815)
  Path provider windows crash fix (flutter#3814)
  [local_auth] docs update (flutter#3103)
  Update PULL_REQUEST_TEMPLATE.md (flutter#3801)
  [quick_actions] handle cold start on iOS correctly (flutter#3811)
  Replace path_provider_linux widget tests with simple unit tests (flutter#3812)
  [sensors] format dart code based on the new dart formatter (flutter#3809)
  [google_sign_in] Fix "pick account" on iOS (flutter#3805)
  [image_picker_platform_interface] Added pickMultiImage (flutter#3782)
  [in_app_purchase] Added currency code and numerical price to product detail model. (flutter#3794)
  [local_auth] Fix iOS crash when no localizedReason (flutter#3780)
  Fix and update version checks (flutter#3792)
  [in_app_purchase] Configured example app to use StoreKit Testing on iOS 14 (flutter#3772)
  [local_auth] Unnecessary reassignment in example removed (flutter#2983)
  [flutter_webview] Fix `allowsInlineMediaPlayback` ignored on iOS (flutter#3791)
  Switch script/tools over to the new analysis options (flutter#3777)
  ...
@mvanbeusekom mvanbeusekom mentioned this pull request Apr 26, 2021
Merged
11 tasks
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
@BeMacized @fotiDim
[in_app_purchase] Added currency code and numerical price to product …
fd1b2e9 
…detail model. (flutter#3794)
@mvanbeusekom mvanbeusekom deleted the issue/65758 branch September 21, 2021 09:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cyanglaz cyanglaz cyanglaz approved these changes

@mvanbeusekom mvanbeusekom mvanbeusekom approved these changes

@LHLL LHLL Awaiting requested review from LHLL

Assignees
No one assigned
Labels
cla: yes p: in_app_purchase waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BeMacized @cyanglaz @mvanbeusekom @fluttergithubbot