✅ app: fix onesignal on e2e with random id#698
Conversation
🦋 Changeset detectedLatest commit: 4380280 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughUpdates e2e workflow job dependencies and adds e2e-mode conditional behavior to OneSignal initialization: introduces a readiness promise that waits for a OneSignal user event and gates enablePrompt/login behavior in EXPO_PUBLIC_ENV="e2e". Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @cruzdanilo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the reliability and isolation of end-to-end (E2E) tests by specifically adjusting the OneSignal integration. It introduces logic to ensure that OneSignal operates with temporary, random user identifiers during E2E runs, preventing data contamination. Concurrently, it refines the CI/CD pipeline's E2E job dependencies, optimizing the build and test process for these environments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes an issue with OneSignal in end-to-end tests by logging in with a random user ID for each test run. The changes also include updates to the EAS workflow configuration to correct job dependencies. The approach to fix the e2e tests seems correct. The suggestion to improve how the random ID is generated for better clarity and to avoid potential issues with the ID format is valid and has been kept.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #698 +/- ##
==========================================
- Coverage 63.78% 63.71% -0.08%
==========================================
Files 169 169
Lines 5315 5324 +9
Branches 1497 1501 +4
==========================================
+ Hits 3390 3392 +2
- Misses 1738 1745 +7
Partials 187 187
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.eas/workflows/local.yaml:
- Around line 34-35: The workflow is incorrectly reading outputs from jobs
listed under "after" via "needs" (e.g., needs.android_build.outputs.build_id);
update those references to use the "after" context and fall back to the sibling
repack job where applicable — replace usages like
needs.android_build.outputs.build_id with after.android_build.outputs.build_id
|| after.android_repack.outputs.build_id (and do the same for ios:
after.ios_build.outputs.build_id || after.ios_repack.outputs.build_id) so the
workflow consistently reads outputs from conditionally-executed jobs.
In `@src/utils/onesignal.ts`:
- Around line 69-72: The code directly reads process.env.EXPO_PUBLIC_ENV in
onesignal initialization branches (around OneSignal.initialize and
OneSignal.login and the other occurrences noted at lines ~81-85 and ~91) which
bypasses the valibot-validated config; change these to use the validated
environment value exported from your central config (e.g., import the validated
env object or an exported boolean like isE2E) and replace direct checks of
process.env.EXPO_PUBLIC_ENV with that shared isE2E boolean so
OneSignal.initialize, OneSignal.login and the other branches use the validated
config instead of raw process.env.
- Around line 81-85: Extract the multi-line async arrow callback into a named
async function (e.g., async function handleOnboardTrigger()) and call that
function in place of the inline async () => { ... } so the block around
queryClient.getQueryData, DISMISS_EXPIRY check, the
OneSignal.User.getOnesignalId call, and
OneSignal.InAppMessages.addTrigger("onboard","1") becomes a function
declaration; also replace the inline comment "// HACK" with a lowercase comment
"// hack" to satisfy the comment-casing rule and keep the same try/catch
behavior for OneSignal.User.getOnesignalId().catch(() => undefined).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.eas/workflows/local.yaml:
- Around line 32-34: The workflow is incorrectly referencing outputs with
needs.android_build/needs.android_repack while those jobs are declared in the
after array; update the output accessors to use after.<job>.outputs.<name>
(e.g., after.android_build.outputs.build_id or
after.android_repack.outputs.build_id) wherever build_id is referenced (the
android_e2e job and the analogous iOS e2e references), or alternatively move
android_build and android_repack into the needs array so the existing
needs.<job>.outputs.<name> syntax becomes valid; ensure consistent use of
after.<job>.outputs.build_id for jobs listed under after (and the same change
for the iOS job names referenced).
In `@src/utils/onesignal.ts`:
- Line 84: Update the HACK comment next to the OneSignal.User.getOnesignalId()
call to include a ticket/issue reference (e.g., "HACK: temporary workaround, see
TICKET-1234") and replace the silent .catch(() => undefined) with a handler that
logs the caught error for debugging in e2e runs (use existing logger or
console.warn/error) while still returning/ignoring the value; locate the call in
the OneSignal helper where OneSignal.User.getOnesignalId() is awaited and apply
these changes to preserve behavior but surface errors for investigation.
1 participant
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.