refactor partnerCode handling#6520
Conversation
There was a problem hiding this comment.
Pull request overview
This PR centralizes “partnerCode” resolution and persistence into a dedicated Angular service, replacing duplicated ad-hoc fragment/localStorage handling in the transaction/tracker flows. This aligns partnerCode behavior across components and supports consistent precedence between Enterprise-derived codes and one-off fragment codes.
Changes:
- Added
PartnerCodeServiceas a single source of truth for partnerCode (Enterprise code takes precedence over stored one-off code). - Updated
TransactionComponentandTrackerComponentto consumepartnerCodefrom the service and to strippartnerCodefrom the URL fragment via Angular Router navigation. - Updated
AccelerateCheckoutto consume (clear) one-off codes via the service after successful acceleration/payment flows.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/app/services/partner-code.service.ts | Introduces centralized partnerCode state, combining Enterprise info and persisted one-off codes. |
| frontend/src/app/components/transaction/transaction.component.ts | Removes inline fragment/localStorage partnerCode handling; subscribes to service and cleans URL fragment. |
| frontend/src/app/components/tracker/tracker.component.ts | Removes inline fragment/localStorage partnerCode handling; subscribes to service, consumes fragment partnerCode, and cleans URL fragment. |
| frontend/src/app/components/accelerate-checkout/accelerate-checkout.component.ts | Clears one-off partnerCode through the service after successful payment/acceleration completion paths. |
| fragmentParams.delete('partnerCode'); | ||
| this.router.navigate([], { | ||
| relativeTo: this.route, | ||
| fragment: fragmentParams.toString() || null, | ||
| queryParamsHandling: 'merge', | ||
| replaceUrl: true, | ||
| }); |
There was a problem hiding this comment.
I think we can also wrap this in the partner code service?
There was a problem hiding this comment.
All components which needs to read the partner code follow the same logic right.
- Get partner code (from subdomain || from url || from local storage)
- Clean up the url
There was a problem hiding this comment.
So I would go as far as saying that consumers of the partner-code service should only have one call it to it getPartnerCode() and the partner-code service would just take care of everything maybe.
There was a problem hiding this comment.
ideally yes, but the awkward part is how that interacts with the other pages' fragment handling and navigation.
it's nice for e.g. the transaction page component to fully "own" the url and internal navigation for its own route, then you can consider that logic in isolation without worrying about side-effects leaking in from elsewhere.
| const enterprisePartnerCode$ = this.enterpriseService.info$.pipe( | ||
| filter((info: object | null) => !isEnterprise || info !== null), | ||
| map((info: object | null) => (info as { name?: string } | null)?.name || undefined), | ||
| ); | ||
|
|
||
| // enterprise partner code takes precedence, and remains applicable for all requests on this instance | ||
| // one-off code applies to the first successful request after being set, iff no enterprise code is present | ||
| this.partnerCode$ = combineLatest([ | ||
| enterprisePartnerCode$, | ||
| this.storedPartnerCode$, | ||
| ]).pipe( | ||
| map(([enterpriseCode, storedCode]) => enterpriseCode || storedCode), | ||
| distinctUntilChanged(), | ||
| shareReplay(1), | ||
| ); |
There was a problem hiding this comment.
I think this is fine because if the $info call fails, the user will probably be redirected to the main website anyway so all this does not matter anymore?
| import { Injectable } from '@angular/core'; | ||
| import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; | ||
| import { distinctUntilChanged, filter, map, shareReplay } from 'rxjs/operators'; | ||
| import { EnterpriseService } from '@app/services/enterprise.service'; |
|
I've noticed that http://localhost:4200/tx/cad8ef7199c14bd9d887241e7d6f27d6767dfb3d1526e09e5c0e7e16b1491785#accelerate&partnerCode=sHDE72yN would result in http://localhost:4200/tx/cad8ef7199c14bd9d887241e7d6f27d6767dfb3d1526e09e5c0e7e16b1491785#accelerate= with a trailing |
nymkappa
left a comment
nymkappa
left a comment
There was a problem hiding this comment.
Otherwise my local tests looks good to me 👍🏻
mononaut
force-pushed
the
mononaut/partner-codes
branch
from
0093469 to
361fbbe
Compare
nymkappa
left a comment
nymkappa
left a comment
There was a problem hiding this comment.
Re-tested ACK @ 361fbbe. Thanks for simplifying the string manipulation.
The extra = does not seem to be a an issue anymore, and the subdomain code still properly replaces the one passed in the url (if any).
I see some doc updated sneaked in but that's cool haha.
|
thanks! I just rebased on master and fixed the trailing '=' issue - the doc change you saw was probably just the diff between the original branch and the rebased one with recent commits included, not actually part of this PR. |
This PR extracts partnerCode handling into a small self-contained service so that we have a single source-of-truth and more consistent/predictable behavior across the various components that rely on this state.
also resolves https://github.com/mempool/mempool.space/issues/2228 and supersedes #6459