Skip to content

use transactions.all to observe transactions in observer mode with SK2#2063

Closed
aboedo wants to merge 2 commits into
mainfrom
fix/missing_transactions_in_sk2_observer_mode
Closed

use transactions.all to observe transactions in observer mode with SK2#2063
aboedo wants to merge 2 commits into
mainfrom
fix/missing_transactions_in_sk2_observer_mode

Conversation

@aboedo

@aboedo aboedo commented Nov 17, 2022

Copy link
Copy Markdown
Member

While it's not documented, Transaction.updates will only send updates for purchases being made on other devices or renewed. Purchases made on the same device will not receive an update through .updates. Instead, those get them through .all.

This draft PR updates the logic so that in observer mode we don't miss transactions made on the same device.

@aboedo aboedo requested a review from a team November 17, 2022 10:05
@aboedo aboedo self-assigned this Nov 17, 2022
@RevenueCat-Danger-Bot

Copy link
Copy Markdown
1 Error
🚫 Label the PR using one of the change type labels
Label Description
breaking Changes that are breaking
build Changes that affect the build system
ci Changes to our CI configuration files and scripts
docs Documentation only changes
feat A new feature
fix A bug fix
perf A code change that improves performance
refactor A code change that neither fixes a bug nor adds a feature
style Changes that don't affect the meaning of the code (white-space, formatting, missing semi-colons, etc
test Adding missing tests or correcting existing tests
next_release Preparing a new release
dependencies Updating a dependency

Generated by 🚫 Danger

Comment on lines 43 to 44

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@NachoSoto I can wrap this up tonight if you don't have the time, basically we need to pass in system info or the setting here

@NachoSoto NachoSoto force-pushed the fix/missing_transactions_in_sk2_observer_mode branch from 8db9225 to bb57c1a Compare November 17, 2022 18:34

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See #2069 for why this implementation would differ from SK1.

@aboedo

aboedo commented Nov 24, 2022

Copy link
Copy Markdown
Member Author

closing since this isn't needed

@aboedo aboedo closed this Nov 24, 2022
NachoSoto added a commit that referenced this pull request Nov 28, 2022
I wrote all these to cover the change in #2063. I wrote a failing test that only passed with the change in #2063. However, when running that in SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the moment you access the sequence. The sequence emits a finite number of transactions. If the App Store processes new transactions for the user while you’re accessing this sequence, the new transactions appear in the transaction listener, updates.

So all these new tests verify the behavior of the SDK when encountering transactions created prior to initializing the SDK.

Depends on #2064 and #2066.
NachoSoto added a commit that referenced this pull request Dec 15, 2022
I wrote all these to cover the change in #2063. I wrote a failing test that only passed with the change in #2063. However, when running that in SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the moment you access the sequence. The sequence emits a finite number of transactions. If the App Store processes new transactions for the user while you’re accessing this sequence, the new transactions appear in the transaction listener, updates.

So all these new tests verify the behavior of the SDK when encountering transactions created prior to initializing the SDK.

Depends on #2064 and #2066.
NachoSoto added a commit that referenced this pull request Dec 15, 2022
)

See [TRIAGE-179].

I wrote all these to cover the change in #2063. I wrote a failing test
that only passed with the change in #2063. However, when running that in
SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ
from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the
moment you access the sequence. The sequence emits a finite number of
transactions. If the App Store processes new transactions for the user
while you’re accessing this sequence, the new transactions appear in the
transaction listener, updates.

These new tests verify the behavior of the SDK when encountering
transactions created prior to initializing the SDK, and some made after
initialization.

Integration tests now are as follows:
- `BaseStoreKitIntegrationTests`
  - `StoreKit1IntegrationTests`
  - `StoreKit2IntegrationTests`
  - `BaseStoreKitObserverModeIntegrationTests`:
    - `StoreKit1ObserverModeIntegrationTests`
    - `StoreKit2ObserverModeIntegrationTests`
    - `StoreKit1ObserverModeWithExistingPurchasesTests`
    - `StoreKit2ObserverModeWithExistingPurchasesTests`

Depends on #2064, #2066, and #2134.

[TRIAGE-179]:
https://revenuecats.atlassian.net/browse/TRIAGE-179?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@NachoSoto NachoSoto deleted the fix/missing_transactions_in_sk2_observer_mode branch May 26, 2023 17:17
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.

3 participants