Wizcorp / node-iap

In-app purchase validation for Apple, Google, Amazon, Roku
262 stars 92 forks source link

add subscription renewal information on apple receipt #46

Closed MarcEtienneDery closed 6 years ago

ronkorving commented 6 years ago

Thanks for your contribution! This LGTM. Wouldn't mind someone else signing off on it too before I merge it though.

cc @KLVTZ

justinpage commented 6 years ago

@MarcEtienneDery thanks for including this!

@ronkorving I would like to get #40 in first. Afterwards, we can adopt the same pattern in having a fallback when pending_renewal_info is not found. I have plenty of active receipts that don't have this field so a discrete fallback would explain why the value may be null or some other default value.

sarunw commented 6 years ago

@KLVTZ #40 is already merged, do you have plan to merge this?

justinpage commented 6 years ago

@sarunw Can you review how #40 has fallbacks when a field is not available? In other words, we should check to see if this field exists before including it. I have plenty of active receipts that don't have this field so a discrete fallback would explain why the value may be null or some other default value.

sarunw commented 6 years ago

@KLVTZ If this field is not present it will be undefined just like latestExpiredReceiptInfo what behavior do you want if it not present? Remove the key or set it to null.

This is not my PR, but happy to help create one to make this happen.

superandrew213 commented 6 years ago

@KLVTZ handling this like latestExpiredReceiptInfo is the way to go.

pending_renewal_info won't be used for the common fields so it can just be undefined or null.

In #40 we decided to use null if data is not available. So I guess we could use null here too for both latestExpiredReceiptInfo and pending_renewal_info

sarunw commented 6 years ago

I made new PR here https://github.com/Wizcorp/node-iap/pull/48

justinpage commented 6 years ago

I think we should merge #48 instead of #46.

48 PR would satisfy #46 in addition to including some safe fallbacks.

ronkorving commented 6 years ago

48 is merged. Let's close this.