-
Notifications
You must be signed in to change notification settings - Fork 279
Fix mirroring between accountReferral's installerId and `accountA…
#5755
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
Conversation
💡 Codex Reviewedge-react-gui/src/actions/AccountReferralActions.ts Lines 40 to 66 in b51d5db
[P1] Losing late Apple Ads attribution for existing accounts Removing the ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| const cache = asDiskReferralCache(JSON.parse(cacheText)) | ||
| const referral = unpackAccountReferral(JSON.parse(referralText)) | ||
|
|
||
| // Ensure accountAppleAdsAttribution mirrors device-level attribution when missing or empty | ||
| const existingAttrib = referral.accountAppleAdsAttribution | ||
| const isAttribMissing = | ||
| existingAttrib == null || | ||
| (existingAttrib.campaignId == null && existingAttrib.keywordId == null) | ||
| if (isAttribMissing) { | ||
| const { appleAdsAttribution } = await getFirstOpenInfo() | ||
| if (appleAdsAttribution != null) { | ||
| referral.accountAppleAdsAttribution = | ||
| asAppleAdsAttribution(appleAdsAttribution) | ||
| } | ||
| } | ||
|
|
||
| // Reference info server promo data to see if: | ||
| // 1. Any of these `activePromotions` are no longer valid (e.g. a | ||
| // promotion expired) |
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.
[P1] Losing late Apple Ads attribution for existing accounts
Removing the getFirstOpenInfo fallback means loadAccountReferral no longer repopulates accountAppleAdsAttribution when it is missing. Apple Search Ads attribution often becomes available hours or days after install, so accounts created before the data exists will now persist with accountAppleAdsAttribution undefined even if the device later obtains valid attribution. Because logEvent now reports the account-level field, analytics for those users will stay empty and the installerId/attribution pair will never be mirrored. Consider keeping the fallback or scheduling a refresh so late-arriving attribution is stored on the account.
Useful? React with 👍 / 👎.
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.
We will just have to live with it not working if the value doesn't make it in after the retries in loadAccountReferral
The whole point is to make sure we are guaranteeing only new accounts have the attribution. If we really need to later we can add a "attributionFailed" flag or something.
Added reporting to compensate
b51d5db to
25a70a9
Compare
fb0a4f5 to
3559451
Compare
fe052a9 to
6081086
Compare
…ppleAdsAttribution` Also rename the reported value to start fresh
6081086 to
b606ec8
Compare
It is known that Apple Ads attribution is not always successful, so we need to track the success/failure of the attribution process. We may need to expand attribution beyond the first login, but keeping it simple for now.
b606ec8 to
21b5340
Compare
Fix mirroring between accountReferral's installerId and accountAppleAdsAttribution
Also rename the reported value to start fresh
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Restricts Apple Ads attribution to iOS with success/fail tracking, renames the analytics field, simplifies attribution fetching to return undefined when unavailable, and updates the changelog.
accountAppleAdsAttributionincreateAccountReferralwithAAA_Success/AAA_Failedevents (AccountReferralActions.ts).accountAppleAdsAttribution.refAccountAppleAdsAttributiontoaccountAppleAdsAttribution(tracking.ts).AAA_Success,AAA_Failed.getAppleAdsAttributionnow returnsundefinedon non‑iOS or failures; adjusts retry/error handling (FirstOpenActions.tsx).installerIdandaccountAppleAdsAttribution.Written by Cursor Bugbot for commit 21b5340. This will update automatically on new commits. Configure here.