[AppStore, Subscription] Add verification returning both receipt and receipt collection #15
Conversation
1 similar comment
Please Rubocop 👮🏻 Bump Rubocop 👮🏻 Fix rubocop issues Bump version to 0.1.0.pre Add information about PlayStore configto README Refs #12 Unify code style Add failing test for expired check Base expired? check based on time comparison Satisfy recent rubocop suggestions Update dev Ruby version to 2.4.1 Test more Ruby versions on Travis CI Update dev dependencies Fix Travis JRuby modes for JRuby 1.7 Use JRuby 1.7.26 on Travis Drop rubinius test on Travis Bump version to 0.1.1 Fix some merge conflicts Add information about PlayStore configto README
| def valid? | ||
| @response && @response['status'] == STATUS_OK && @response['receipt'] | ||
| @response && response_status_ok? && @response['receipt'] |
There was a problem hiding this comment.
It feels a bit like response_status_ok? is not self-contained if you have to check @response before you use it. Maybe it would be nicer with two black boxes: response_status_ok? and response_has_receipt? which both take care of validating all that they need. This would mean duplicating the @response check, but I think that's acceptable, or if the response is only set once, then you could even cache/memoize this in a has_response? method
| # @return [SubscriptionReceipt] if successful | ||
| # @return [VerificationFailure] otherwise | ||
| def verify_subscription_with_full_response(receipt_data, secret = nil) | ||
| @verifier = FullSubscriptionVerification |
There was a problem hiding this comment.
it's weird to have @verifier as instance state. It introduces coupling in the order of invocation of the public methods here, which can cause race conditions if run in parallel.
There was a problem hiding this comment.
I agree. However I would like to keep it consistent with the rest of the code.
| @@ -25,17 +27,18 @@ def initialize(attributes) | |||
|
|
|||
| # Check if the expiration date is passed | |||
| # @return [bool] | |||
| # rubocop:disable Style/NumericPredicate | |||
There was a problem hiding this comment.
Not supported in older versions.
| @@ -26,7 +28,7 @@ | |||
|
|
|||
| it 'has positive overdue days' do | |||
| overdue = subject.overdue_days | |||
| overdue.must_be_instance_of Fixnum | |||
| overdue.is_a?(Integer).must_be_true | |||
There was a problem hiding this comment.
maybe we shouldn't care this much about types anyway. Maybe asserting that it is .positive? should suffice?
|
@antonwestman thanks for your work. I'm currently in a situation where I cannot work on this project for a couple of weeks. I'm very sorry for this, but my family goes first. |
|
Good priorities @jnbt. Family should always come first. 👍 We have tested against the sandbox environment with positive results. I'm looking forward to your response. 🖖🏻 |
This PR adds two new objects
FullSubscriptionVerificationandSubscriptionReceiptas well as a new methodCandyCheck::AppStore::Verifier#verify_subscription_with_full_response.The
SubscriptionReceiptconsist of aReceiptand aReceiptCollection.I also added the undocumented response code 21009.