Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughI aligned deep-link handling to be org-aware: deep-link service extracts/decodes org_id, deep links are routed to a centralized deep_link_redirection with an optional orgId, and DeepLinkRedirection + SwitchOrg pages now accept and propagate Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeepLinkSvc as DeepLinkService
participant DeepLinkPage as DeepLinkRedirectionPage
participant EOU as EOUService
participant SwitchOrg as SwitchOrgPage
participant Nav as NavigationService
User->>DeepLinkSvc: Open deep link (sub_module, id, redirect_uri)
DeepLinkSvc->>DeepLinkSvc: Extract & decode orgId from redirect_uri
DeepLinkSvc->>DeepLinkPage: Navigate to ['/', 'deep_link_redirection', {sub_module, id, orgId, ...}]
DeepLinkPage->>EOU: getEou (current org)
EOU-->>DeepLinkPage: current orgId
alt orgId missing or matches current
DeepLinkPage->>Nav: Navigate to target module (report/advReq/my_expenses) within current org
Nav-->>User: Show target
else orgId differs
DeepLinkPage->>SwitchOrg: Navigate to `auth/switch_org` with orgId + params (reportId/advReqId/my_expenses_filters)
SwitchOrg->>SwitchOrg: show loader, switch org, refresh caches, fetch EOU
SwitchOrg->>Nav: Navigate to final target (deep_link_redirection or enterprise/my_expenses)
Nav-->>User: Show target in switched org
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/core/services/deep-link.service.ts (1)
113-126: 🧹 Nitpick | 🔵 TrivialOne thing to note, macha - consistency is the key to superstardom!
The
advance_requestbranch does not attachorgIdto its properties, unlike thereport,my_expenses, and expensetxnIdbranches. If org-aware deep linking is intended for advance requests as well, consider addingresolvedOrgIdhere for consistency. If this is intentional (no org switching needed for advance requests), disregard this observation.♻️ Proposed fix for consistency (if needed)
const properties: Record<string, string> = { sub_module: subModule, id: advReqId, }; + if (resolvedOrgId) { + properties.orgId = resolvedOrgId; + } if (notificationType) { properties.push_notification_type = notificationType; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/services/deep-link.service.ts` around lines 113 - 126, The advance_request branch creates properties using advReqId/subModule but omits resolvedOrgId; update the block around advReqId/subModule/properties in deep-link.service.ts to add the org id key (e.g., properties.resolved_org_id = resolvedOrgId or properties.org_id = resolvedOrgId consistent with the other branches) when resolvedOrgId is present, before calling this.router.navigate(['/', 'deep_link_redirection', properties]); ensure you use the same property name used elsewhere (resolvedOrgId or orgId) to keep deep-linking org-aware and consistent with report/my_expenses/txnId handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/auth/switch-org/switch-org.page.ts`:
- Around line 194-198: In ionViewWillEnter, remove the erroneous "return"
keywords before the void-returning calls (redirectToExpensePage,
redirectToReportPage, redirectToMyExpenses) so the method stays typed as void;
simply call those methods without returning their result and apply the same
change to any other early-exit calls in ionViewWillEnter that currently use
"return" with void-returning helpers.
In `@src/app/deep-link-redirection/deep-link-redirection.page.ts`:
- Around line 171-222: The current dual subscriptions to eou$ in
redirectToMyExpensesModule cause finalize() (and thus
loaderService.hideLoader()) to run twice; replace the two subscriptions with a
single subscription that applies a single finalize and then conditionally
navigates based on whether orgId === eou.ou.org_id (e.g., use a single pipe on
eou$ with take(1) or first(), then inside subscribe.next decide to call
router.navigate(...) for the matching case or the switch_org route for the
non-matching case), ensuring errors still call switchOrg() and that hideLoader()
is invoked only once.
- Around line 314-334: The redirectToReportModule function shows the loader
before checking org mismatch which leads to continuing execution (including
subscribing to spenderReport$ and making API calls) even after navigating to
switch_org; move the org-mismatch check to run before showing the loader (or
return immediately after detection) so that when orgId !== currentEou.ou.org_id
you call router.navigate(...) and return without showing the loader or executing
the subsequent report-fetch/subscription logic (references:
redirectToReportModule, loaderService.showLoader/hideLoader, authService.getEou,
router.navigate, spenderReport$).
---
Outside diff comments:
In `@src/app/core/services/deep-link.service.ts`:
- Around line 113-126: The advance_request branch creates properties using
advReqId/subModule but omits resolvedOrgId; update the block around
advReqId/subModule/properties in deep-link.service.ts to add the org id key
(e.g., properties.resolved_org_id = resolvedOrgId or properties.org_id =
resolvedOrgId consistent with the other branches) when resolvedOrgId is present,
before calling this.router.navigate(['/', 'deep_link_redirection', properties]);
ensure you use the same property name used elsewhere (resolvedOrgId or orgId) to
keep deep-linking org-aware and consistent with report/my_expenses/txnId
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 795dd857-3929-451c-aeff-077b2ac8ab90
📒 Files selected for processing (4)
src/app/auth/switch-org/switch-org.page.tssrc/app/core/services/deep-link.service.spec.tssrc/app/core/services/deep-link.service.tssrc/app/deep-link-redirection/deep-link-redirection.page.ts
Comment on lines
194
to
+198
| return this.redirectToExpensePage(orgId, txnId); | ||
| } else if (orgId && reportId) { | ||
| return this.redirectToReportPage(orgId, reportId); | ||
| } else if (orgId && myExpensesFilters) { | ||
| return this.redirectToMyExpenses(orgId, myExpensesFilters); |
There was a problem hiding this comment.
Naan oru dhadava sonna, nooru dhadava sonna madhiri! The linter is not happy, boss!
The static analysis tool flags these return statements because ionViewWillEnter has return type void, but you're returning the result of methods that also return void. While functionally correct (early exit pattern), this creates a type inconsistency.
Remove the return keyword to silence the linter - the early exit behavior is already achieved by the method call being the last statement in the if block.
🔧 Proposed fix
if (orgId && txnId) {
- return this.redirectToExpensePage(orgId, txnId);
+ this.redirectToExpensePage(orgId, txnId);
} else if (orgId && reportId) {
- return this.redirectToReportPage(orgId, reportId);
+ this.redirectToReportPage(orgId, reportId);
} else if (orgId && myExpensesFilters) {
- return this.redirectToMyExpenses(orgId, myExpensesFilters);
+ this.redirectToMyExpenses(orgId, myExpensesFilters);
} else if (openSMSOptInDialog === 'true' && orgId) {Note: Also ensure remaining early exits (lines 199-202) are similarly updated, though they weren't flagged in the provided hints.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return this.redirectToExpensePage(orgId, txnId); | |
| } else if (orgId && reportId) { | |
| return this.redirectToReportPage(orgId, reportId); | |
| } else if (orgId && myExpensesFilters) { | |
| return this.redirectToMyExpenses(orgId, myExpensesFilters); | |
| this.redirectToExpensePage(orgId, txnId); | |
| } else if (orgId && reportId) { | |
| this.redirectToReportPage(orgId, reportId); | |
| } else if (orgId && myExpensesFilters) { | |
| this.redirectToMyExpenses(orgId, myExpensesFilters); |
🧰 Tools
🪛 Biome (2.4.6)
[error] 194-194: The function should not return a value because its return type is void.
(lint/correctness/noVoidTypeReturn)
[error] 196-196: The function should not return a value because its return type is void.
(lint/correctness/noVoidTypeReturn)
[error] 198-198: The function should not return a value because its return type is void.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/auth/switch-org/switch-org.page.ts` around lines 194 - 198, In
ionViewWillEnter, remove the erroneous "return" keywords before the
void-returning calls (redirectToExpensePage, redirectToReportPage,
redirectToMyExpenses) so the method stays typed as void; simply call those
methods without returning their result and apply the same change to any other
early-exit calls in ionViewWillEnter that currently use "return" with
void-returning helpers.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/core/services/deep-link.service.ts`:
- Around line 39-40: The code extracts orgIdFromUri and computes resolvedOrgId
but then still uses redirectionParam.org_id in the org-aware redirect branch
(e.g., the corporate_cards case); update that branch (and any other org-aware
redirect code in the same method in deep-link.service.ts) to use resolvedOrgId
instead of redirectionParam.org_id so that org context recovered from
redirect_uri is preserved for the switch/redirect flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d75ae96-afa7-4f24-9984-88e1461d3405
📒 Files selected for processing (1)
src/app/core/services/deep-link.service.ts
…yle-mobile-app into fyle-fix-deep-links-with-org-id
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/app/deep-link-redirection/deep-link-redirection.page.ts (1)
191-222:⚠️ Potential issue | 🟡 MinorMind it! Dual-subscription pattern still present, macha!
The past review flagged the dual-subscription pattern where
eou$is subscribed twice (lines 191-202 and 204-221), causingfinalize()to execute twice andloaderService.hideLoader()to be called twice. While the previous review marked this as "addressed," the current code still shows the same pattern.This is wasteful and could cause timing issues. Consolidate into a single subscription with conditional navigation.
🔧 Proposed fix
- eou$ - .pipe( - filter((eou) => orgId === eou.ou.org_id), - finalize(() => from(this.loaderService.hideLoader())), - ) - .subscribe({ - next: () => - this.router.navigate(['/', 'enterprise', 'my_expenses'], { - queryParams: filters ? { filters } : {}, - }), - error: () => this.switchOrg(), - }); - - eou$ - .pipe( - filter((eou) => orgId !== eou.ou.org_id), - finalize(() => from(this.loaderService.hideLoader())), - ) - .subscribe({ - next: () => - this.router.navigate([ - '/', - 'auth', - 'switch_org', - { - orgId, - my_expenses_filters: filters, - }, - ]), - error: () => this.switchOrg(), - }); + eou$ + .pipe(finalize(() => from(this.loaderService.hideLoader()))) + .subscribe({ + next: (eou) => { + if (orgId === eou.ou.org_id) { + this.router.navigate(['/', 'enterprise', 'my_expenses'], { + queryParams: filters ? { filters } : {}, + }); + } else { + this.router.navigate([ + '/', + 'auth', + 'switch_org', + { + orgId, + my_expenses_filters: filters, + }, + ]); + } + }, + error: () => this.switchOrg(), + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/deep-link-redirection/deep-link-redirection.page.ts` around lines 191 - 222, The code subscribes to eou$ twice causing finalize() and loaderService.hideLoader() to run twice; replace the dual subscriptions with a single eou$ pipeline (keep the existing pipe operators like finalize(from(this.loaderService.hideLoader()))) and a single subscribe that in its next handler checks if orgId === eou.ou.org_id to call router.navigate(['/', 'enterprise', 'my_expenses'], { queryParams: filters ? { filters } : {} }) otherwise navigates to ['/', 'auth', 'switch_org', { orgId, my_expenses_filters: filters }]; keep the existing error handler that calls switchOrg(); reference eou$, finalize(from(this.loaderService.hideLoader())), router.navigate(...) and switchOrg() when implementing.src/app/auth/switch-org/switch-org.page.ts (1)
194-201:⚠️ Potential issue | 🟡 MinorNaan oru dhadava sonna, nooru dhadava sonna madhiri! Remove those return statements, boss!
The static analysis tool correctly flags these
returnstatements becauseionViewWillEnterhas return typevoid, but you're returning the result of methods that also returnvoid. While functionally it works as an early exit pattern, this creates a type inconsistency that the linter complains about.Remove the
returnkeyword to silence the linter - the early exit behavior is already achieved by the method call being the last statement in theifblock.🔧 Proposed fix
if (orgId && txnId) { - return this.redirectToExpensePage(orgId, txnId); + this.redirectToExpensePage(orgId, txnId); + return; } else if (orgId && reportId) { - return this.redirectToReportPage(orgId, reportId); + this.redirectToReportPage(orgId, reportId); + return; } else if (orgId && advReqId) { - return this.redirectToAdvReqPage(orgId, advReqId); + this.redirectToAdvReqPage(orgId, advReqId); + return; } else if (orgId && myExpensesFilters) { - return this.redirectToMyExpenses(orgId, myExpensesFilters); + this.redirectToMyExpenses(orgId, myExpensesFilters); + return; } else if (openSMSOptInDialog === 'true' && orgId) { - return this.redirectToDashboard(orgId); + this.redirectToDashboard(orgId); + return; } else if (orgId && openVirtualCards) { - return this.redirectToCorporateCards(orgId, openVirtualCards === 'true'); + this.redirectToCorporateCards(orgId, openVirtualCards === 'true'); + return; } else if (!choose) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/auth/switch-org/switch-org.page.ts` around lines 194 - 201, In ionViewWillEnter, remove the unnecessary "return" keywords before the calls to redirectToExpensePage, redirectToReportPage, redirectToAdvReqPage, and redirectToMyExpenses so the method's void signature is respected; keep the method calls as-is to preserve the early-exit behavior (i.e., change "return this.redirectToExpensePage(...)" to "this.redirectToExpensePage(...)" for each occurrence).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/auth/switch-org/switch-org.page.ts`:
- Around line 194-201: In ionViewWillEnter, remove the unnecessary "return"
keywords before the calls to redirectToExpensePage, redirectToReportPage,
redirectToAdvReqPage, and redirectToMyExpenses so the method's void signature is
respected; keep the method calls as-is to preserve the early-exit behavior
(i.e., change "return this.redirectToExpensePage(...)" to
"this.redirectToExpensePage(...)" for each occurrence).
In `@src/app/deep-link-redirection/deep-link-redirection.page.ts`:
- Around line 191-222: The code subscribes to eou$ twice causing finalize() and
loaderService.hideLoader() to run twice; replace the dual subscriptions with a
single eou$ pipeline (keep the existing pipe operators like
finalize(from(this.loaderService.hideLoader()))) and a single subscribe that in
its next handler checks if orgId === eou.ou.org_id to call router.navigate(['/',
'enterprise', 'my_expenses'], { queryParams: filters ? { filters } : {} })
otherwise navigates to ['/', 'auth', 'switch_org', { orgId, my_expenses_filters:
filters }]; keep the existing error handler that calls switchOrg(); reference
eou$, finalize(from(this.loaderService.hideLoader())), router.navigate(...) and
switchOrg() when implementing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9018c6e7-d266-4754-98b8-e0dd463d311c
📒 Files selected for processing (6)
src/app/auth/switch-org/switch-org.page.spec.tssrc/app/auth/switch-org/switch-org.page.tssrc/app/core/services/deep-link.service.spec.tssrc/app/core/services/deep-link.service.tssrc/app/deep-link-redirection/deep-link-redirection.page.spec.tssrc/app/deep-link-redirection/deep-link-redirection.page.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/core/services/deep-link.service.ts`:
- Around line 149-151: The telemetry call to trackingService.smsDeepLinkOpened
currently sends resolvedTxnOrgId even when the TX-link shape is invalid; update
the deep-link handling so you validate resolvedTxnOrgId before including it in
the payload (e.g., check the TX-link shape or that resolvedTxnOrgId matches the
expected org-id format/prefix) and only pass orgId when that validation
succeeds, otherwise omit orgId or pass null/undefined; modify the call site that
invokes trackingService.smsDeepLinkOpened to use the validated value (or a
sanitized fallback) so analytics only receives real org ids.
- Around line 116-130: The branch handling '/advance_request/areq' must ensure a
valid advReqId before building properties and calling this.router.navigate;
update the logic around advReqMatch/advReqId (the regex match and advReqId
variable) to early-return or route to a safe fallback when advReqId is falsy,
and only construct the properties object and call this.router.navigate(['/',
'deep_link_redirection', properties]) when advReqId is present; also ensure any
optional fields (resolvedOrgId, notificationType) are only added after the
advReqId guard so you never forward an undefined id to deep_link_redirection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bdd47186-3273-4341-a961-f619b8eca4be
📒 Files selected for processing (2)
src/app/core/services/deep-link.service.spec.tssrc/app/core/services/deep-link.service.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/core/services/deep-link.service.ts (2)
152-160: 🧹 Nitpick | 🔵 TrivialCorporate cards - style consistency needed, macha!
Line 155 sets
orgId: resolvedOrgIdunconditionally, which addsorgId: undefinedwhen no org context exists. Other branches (e.g., lines 70-72, 109-111) conditionally addorgIdonly whenresolvedOrgIdexists. Apply the same pattern here for consistency.♻️ Proposed fix
} else if (redirectUri.match('corporate_cards')) { - const properties: Record<string, string> = { + const properties: Record<string, string | boolean> = { sub_module: 'manage_corporate_cards', - orgId: resolvedOrgId, }; + if (resolvedOrgId) { + properties.orgId = resolvedOrgId; + } if (notificationType) { properties.push_notification_type = notificationType; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/services/deep-link.service.ts` around lines 152 - 160, The properties object for the 'corporate_cards' branch is always setting orgId, which can produce orgId: undefined; change this to match other branches by only adding orgId when resolvedOrgId is truthy: build properties with sub_module: 'manage_corporate_cards' and conditionally set properties.orgId = resolvedOrgId if resolvedOrgId exists, and conditionally set properties.push_notification_type = notificationType if notificationType exists, then call this.router.navigate(['/', 'deep_link_redirection', properties]) so the router payload never contains orgId: undefined.
161-172: 🧹 Nitpick | 🔵 TrivialMy dashboard - same story, da!
Line 168 sets
orgId: resolvedOrgIdunconditionally. Apply conditional assignment like the report/expense branches for consistency. Also,referreron line 163 could be undefined — consider the same conditional pattern.♻️ Proposed fix
} else if (redirectUri.match('my_dashboard')) { const referrer = redirectUri.match(/referrer=(\w+)/)?.[1]; const openSMSOptInDialog = redirectUri.includes('open_sms_dialog=true'); - const properties = { + const properties: Record<string, string | boolean> = { sub_module: 'my_dashboard', openSMSOptInDialog, - orgId: resolvedOrgId, - referrer, }; + if (resolvedOrgId) { + properties.orgId = resolvedOrgId; + } + if (referrer) { + properties.referrer = referrer; + } this.trackingService.smsDeepLinkOpened(properties); this.router.navigate(['/', 'deep_link_redirection', properties]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/services/deep-link.service.ts` around lines 161 - 172, The properties object for the "my_dashboard" deep link currently always sets orgId: resolvedOrgId and may include an undefined referrer; change it to mirror the report/expense branches by only adding orgId when resolvedOrgId is truthy and only adding referrer when the regex found a value. Update the block around trackingService.smsDeepLinkOpened and router.navigate to build properties conditionally (e.g., create properties with sub_module and openSMSOptInDialog, then if (resolvedOrgId) add orgId, and if (referrer) add referrer) before calling trackingService.smsDeepLinkOpened and this.router.navigate.
♻️ Duplicate comments (1)
src/app/core/services/deep-link.service.ts (1)
148-151:⚠️ Potential issue | 🟡 MinorTracking analytics - style-a irukanum, boss!
The
smsDeepLinkOpenedcall at line 148 executes regardless of whether the tx-link validation passed. When the URL is malformed,resolvedTxnOrgIdcould be junk (not starting with 'or'), polluting your analytics.🔧 Move tracking inside the validated block or sanitize
if (txnId && resolvedTxnOrgId && txnId.startsWith('tx') && resolvedTxnOrgId.startsWith('or')) { const properties: Record<string, string> = { sub_module: 'expense', id: txnId, orgId: resolvedTxnOrgId, }; if (notificationType) { properties.push_notification_type = notificationType; } this.router.navigate(['/', 'deep_link_redirection', properties]); + this.trackingService.smsDeepLinkOpened({ + orgId: resolvedTxnOrgId, + txnId, + }); } else { this.router.navigate(['/', 'auth', 'switch_org', { choose: true }]); } - this.trackingService.smsDeepLinkOpened({ - orgId: resolvedTxnOrgId, - txnId, - });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/services/deep-link.service.ts` around lines 148 - 151, The smsDeepLinkOpened tracking call is invoked even when tx-link validation fails, allowing junk values like resolvedTxnOrgId to pollute analytics; move the this.trackingService.smsDeepLinkOpened({...}) call inside the successful validation branch (the block that confirms the tx-link is valid) or add a guard before calling it that sanitizes/resolves resolvedTxnOrgId (e.g., check it startsWith('or') and txnId is present) so tracking only fires with validated values; locate the call to this.trackingService.smsDeepLinkOpened and adjust it inside the tx-link validation success path (or add the guard) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/deep-link-redirection/deep-link-redirection.page.ts`:
- Around line 341-356: Wrap the await this.authService.getEou() call in a
try/catch inside the deep-link flow so a thrown error cannot leave the loader
visible: call await this.authService.getEou() inside try, and in catch call
await this.loaderService.hideLoader(), optionally log the error (e.g., via
this.logger or console.error), then either rethrow or return/navigate to an
error/fallback route; mirror the error-handling pattern used in
redirectToAdvReqModule() to ensure the loader is always hidden on failures.
---
Outside diff comments:
In `@src/app/core/services/deep-link.service.ts`:
- Around line 152-160: The properties object for the 'corporate_cards' branch is
always setting orgId, which can produce orgId: undefined; change this to match
other branches by only adding orgId when resolvedOrgId is truthy: build
properties with sub_module: 'manage_corporate_cards' and conditionally set
properties.orgId = resolvedOrgId if resolvedOrgId exists, and conditionally set
properties.push_notification_type = notificationType if notificationType exists,
then call this.router.navigate(['/', 'deep_link_redirection', properties]) so
the router payload never contains orgId: undefined.
- Around line 161-172: The properties object for the "my_dashboard" deep link
currently always sets orgId: resolvedOrgId and may include an undefined
referrer; change it to mirror the report/expense branches by only adding orgId
when resolvedOrgId is truthy and only adding referrer when the regex found a
value. Update the block around trackingService.smsDeepLinkOpened and
router.navigate to build properties conditionally (e.g., create properties with
sub_module and openSMSOptInDialog, then if (resolvedOrgId) add orgId, and if
(referrer) add referrer) before calling trackingService.smsDeepLinkOpened and
this.router.navigate.
---
Duplicate comments:
In `@src/app/core/services/deep-link.service.ts`:
- Around line 148-151: The smsDeepLinkOpened tracking call is invoked even when
tx-link validation fails, allowing junk values like resolvedTxnOrgId to pollute
analytics; move the this.trackingService.smsDeepLinkOpened({...}) call inside
the successful validation branch (the block that confirms the tx-link is valid)
or add a guard before calling it that sanitizes/resolves resolvedTxnOrgId (e.g.,
check it startsWith('or') and txnId is present) so tracking only fires with
validated values; locate the call to this.trackingService.smsDeepLinkOpened and
adjust it inside the tx-link validation success path (or add the guard)
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd2b7f08-a3e6-4a9b-902c-b2cb5d0553ac
📒 Files selected for processing (2)
src/app/core/services/deep-link.service.tssrc/app/deep-link-redirection/deep-link-redirection.page.ts
bistaastha
changed the title
🔁 Code Duplication Report - Angular
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests