Release 3.6.0 #30
Release 3.6.0 #30
Conversation
Implements a flexible tracking system with: - Provider-agnostic tracking interface - Matomo implementation - Google Analytics stub implementation - Automatic page view tracking - Session-based user tracking - Convenience methods for common tracking events
…andeling at amrit tracking service
…odule feat(tracking): Add analytics tracking module to Common-UI
…odule Added userID in amrit tracking service
feat: AMM-1337 Feedback Module added
…odule Documentation: Added Tracking Service Documentation and GA functions
Release 3.5.0
WalkthroughThe PR introduces a comprehensive analytics tracking system supporting Matomo and Google Analytics through a provider pattern, integrates field-level tracking across registration forms, adds a new feedback module with dialog component, updates location selectors to autocomplete patterns, standardizes ABHA localization keys, and includes detailed documentation for Matomo setup and tracking architecture. Changes
Sequence DiagramsequenceDiagram
participant App as Application<br/>Initialization
participant ATS as AmritTrackingService
participant Router as Angular Router
participant TP as TrackingProvider<br/>(Matomo/GA)
participant SS as SessionStorage
participant Component as UI Component
App->>ATS: new AmritTrackingService(provider, storage, router)
ATS->>TP: init() [configuration applied]
ATS->>SS: Load stored userID
ATS->>TP: setUserId(userID)
ATS->>Router: Subscribe to NavigationEnd
Router-->>ATS: Navigation event emitted
ATS->>TP: pageView(url)
Component->>ATS: trackFieldInteraction('fieldName')
ATS->>TP: event('Registration', 'fieldInteraction', 'fieldName')
TP-->>TP: Queue/send to backend
ATS-->>Component: Tracking logged
Note over ATS,TP: Cleanup on destroy<br/>via destroy$ Subject
sequenceDiagram
participant User as User
participant FDialog as FeedbackDialogComponent
participant FService as FeedbackService
participant API as Backend API
User->>FDialog: Open feedback dialog
FDialog->>FService: listCategories(serviceLine)
FService->>API: GET /common-api/categories?serviceLine=AAM
API-->>FService: CategoryDto[]
FService-->>FDialog: Categories loaded
FDialog->>FDialog: Render form (rating, category, comment)
User->>FDialog: Set rating, select category, add comment
User->>FDialog: Click Submit
FDialog->>FDialog: Construct SubmitFeedbackRequest payload
alt User is Anonymous
FDialog->>FDialog: isAnonymous = true
else User is Identified
FDialog->>FDialog: Include userId, isAnonymous = false
end
FDialog->>FService: submitFeedback(payload)
FService->>API: POST /common-api/platform-feedback
API-->>FService: { id, createdAt }
FService-->>FDialog: Success response
FDialog->>User: Show success message & reset form
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/registrar/search-dialog/search-dialog.component.ts (1)
315-326: Unsubscribe frommasterDataSubscriptionto prevent memory leak.The
masterDataSubscriptionis assigned but never cleaned up. Since this is a dialog component that can be opened/closed multiple times, each instance will leave an active subscription.Implement
OnDestroyand unsubscribe:-export class SearchDialogComponent implements OnInit, DoCheck { +export class SearchDialogComponent implements OnInit, DoCheck, OnDestroy {Add cleanup method:
ngOnDestroy() { if (this.masterDataSubscription) { this.masterDataSubscription.unsubscribe(); } }
🧹 Nitpick comments (17)
src/registrar/search-dialog/search-dialog.component.ts (1)
191-251: Selection handlers implement cascading logic correctly.The cascading selection pattern properly clears dependent fields when parent selections change. The service calls handle success cases with user feedback via
confirmationService.alert.Consider adding error callbacks to the
subscribecalls to handle network failures gracefully:.subscribe({ next: (res: any) => { /* existing success logic */ }, error: (err) => { this.confirmationService.alert( this.currentLanguageSet.alerts.info.issueFetching, 'error' ); } });matomo-documentation.md (1)
33-33: Tighten markdown formatting and config/import examples
- Line 33 and 307: avoid using emphasis as a pseudo-heading; either make these real headings (e.g.
#### High-level architecture...,#### Last updated) or plain text without italics.- Lines 64 and 195: fenced blocks lack a language; add something like
```textfor the ASCII diagram and```text(or```ini) for the Matomo website details block to satisfy MD040.- Line 300: wrap the bare URL as a proper Markdown link, e.g.
[Matomo documentation](https://matomo.org/docs/), to fix MD034.- The environment snippet defines both
platformandtrackingPlatform(withtrackingPlatform: "platform"), while the code usesenvironment.tracking.trackingPlatform; please double‑check the intended meaning and naming so consumers don’t misconfigure it.- Import examples mix
Common-UI/src/trackingand@common-ui/tracking; consider standardizing on the actual supported entrypoint so examples match the real integration path. Based on learnings,Common-UI/src/trackingis known to work when Common‑UI is used as a submodule.Also applies to: 64-64, 195-195, 214-221, 300-300, 307-307
src/tracking/lib/tracking.tokens.ts (1)
2-8: Verify tracking config shape and consider future configurabilityThe tokens themselves look fine, but a couple of points to verify/consider:
TRACKING_PLATFORMreadsenvironment.tracking.trackingPlatform, while the documentation and examples also show aplatformproperty. Please confirm that all environment files definetrackingPlatform(and that its intended meaning is clear vs.platform) to avoid runtimeundefinedvalues.- Since these tokens are hard‑wired to
environment, this tracking library is tightly coupled to a specific app config. If you ever need to reuse Common‑UI across apps with different configs, consider moving these factories behind a configurable module/provider instead of importingenvironmentdirectly.Also applies to: 10-23, 25-25
src/registrar/registration/abha-information/abha-information.component.ts (1)
1-1: Tracking integration viaInjectorlooks consistent; direct injection would be a small simplificationThe new
trackFieldInteractionmethod and use ofInjectorto resolveAmritTrackingServiceare consistent with the tracking pattern used in other registration components, and the wiring from the template is straightforward.If you don’t need the extra indirection, you could slightly simplify and avoid repeated
injector.get(...)calls by injectingAmritTrackingServicedirectly into the constructor (or caching the result on first access), but this is purely optional.Also, the import path
Common-UI/src/trackingmatches the established submodule usage pattern for this repo. Based on learnings, this is expected.Also applies to: 14-14, 40-48, 233-236
src/tracking/lib/tracking-provider.ts (1)
1-11: AlignsetUserIdsignature with documented usageThe
TrackingProviderinterface is clear, butsetUserIdcurrently accepts onlystring, whereas the API reference documentssetUserId(userId: string | number).If you expect numeric user IDs as well, consider updating the interface to:
- setUserId(userId: string): void; + setUserId(userId: string | number): void;so implementations and call sites stay consistent with the docs and avoid unnecessary casting.
matomo-local-setup.md (1)
1-219: LGTM: Comprehensive Matomo setup guide.This documentation provides a thorough, step-by-step installation guide with appropriate security considerations:
- Idempotent database setup script
- Proper file permissions and ownership
- Nginx security rules to protect sensitive directories
- Production password warnings
The guide is well-structured and includes helpful troubleshooting notes.
Optional: Minor style refinement.
Line 197 repeats "want to" in close proximity. Consider rephrasing for variety, though this is a minor style point.
src/feedback/shared/feedback-dialog/feedback-dialog.component.html (1)
110-112: Clarify method name for close button.The close button calls
login(), which is a potentially confusing method name. If this method closes the dialog, consider renaming it toclose()orcloseDialog()in the component TypeScript for better code clarity.src/registrar/registration/location-information/location-information.component.html (1)
15-15: Add fallback for tracking field name.For consistency with the personal information component, consider adding a fallback:
trackFieldInteraction(item.fieldTitle || item.fieldName)to ensure tracking works even iffieldTitleis missing.src/tracking/lib/matomo-tracking.service.ts (1)
57-58:isInitializedflag is set before script loads and appears unused.The flag is set synchronously before the async script load completes. Since the
_paqqueue pattern already handles deferred execution, consider either removing this unused flag or moving it inside thescript.onloadcallback for accuracy.src/tracking/lib/ga-tracking.service.ts (2)
17-19: Confusing token reuse:MATOMO_SITE_IDused for GA tracking ID.Using
MATOMO_SITE_IDtoken forgaTrackingIdis misleading. Consider creating a dedicatedGA_TRACKING_IDtoken for clarity and to allow independent configuration.-import { MATOMO_SITE_ID, TRACKING_ENABLED } from './tracking.tokens'; +import { GA_TRACKING_ID, TRACKING_ENABLED } from './tracking.tokens'; @Injectable() export class GATrackingService implements TrackingProvider { // ... constructor( - @Inject(MATOMO_SITE_ID) private gaTrackingId: string, + @Inject(GA_TRACKING_ID) private gaTrackingId: string, @Inject(TRACKING_ENABLED) private trackingEnabled: boolean ) {You'd need to add the token in
tracking.tokens.ts:export const GA_TRACKING_ID = new InjectionToken<string>('ga.trackingId', { providedIn: 'root', factory: () => environment.tracking.gaTrackingId, });
53-54: Excessiveconsole.logstatements in production code.Multiple
console.logcalls (lines 53, 64, 76, 101, 127, 149, 186) will clutter production logs. Consider removing them or gating behind a debug flag.Also applies to: 64-64, 76-77
src/tracking/lib/amrit-tracking.service.ts (2)
48-53: Redundant null/undefined checks.After
typeof userId === 'string', checkinguserId !== null && userId !== undefinedis redundant since a string type excludes null/undefined.private setupUserTracking() { const userId = this.sessionStorage.getItem('userID'); - if (typeof userId === 'string' && userId !== null && userId !== undefined && userId !== '') { + if (typeof userId === 'string' && userId !== '') { this.trackingProvider.setUserId(userId); } }
21-36: Fallback pattern silently degrades tracking.While defensive, replacing the provider with a no-op on any initialization error silently disables all tracking. Consider emitting a more prominent warning or exposing a flag to indicate degraded state for monitoring.
src/feedback/shared/feedback-dialog/feedback-dialog.component.ts (3)
87-94: Redundant conditional: both branches are identical.Both the
ifandelsebranches setisAnonymoustotrue. The comments suggest deliberation about privacy defaults, but the current code does not differentiate behavior based on login state.Either simplify to a single statement or implement the intended differentiation:
- // If user is logged in, default to NOT anonymous so they explicitly can opt-in; if logged out, force anonymous - if (this.isLoggedIn) { - // default to anonymous=true to respect privacy; but you asked to ask consent — show unchecked by default - // we'll set it to true so users must actively uncheck to identify themselves OR you can flip to false to encourage identified - // choose default = true to be conservative: - this.form.controls.isAnonymous.setValue(true); - } else { - this.form.controls.isAnonymous.setValue(true); - } + // Default to anonymous for privacy; logged-out users are always anonymous + this.form.controls.isAnonymous.setValue(true);
99-100: Consider updatingCategoryDtointerface instead of double type casting.The cast
(c as any)followed by(c as any).activesuggestsCategoryDtomay be missing theactiveproperty. If the API returns anactivefield, update the interface definition for type safety.
143-150: Consider defining a typed interface for the payload.Using
anyfor the payload loses type safety. A dedicated interface would improve maintainability and catch errors at compile time.src/registrar/registration/personal-information/personal-information.component.ts (1)
850-853: Cache the tracking service to avoid repeated injector lookups.
injector.get(AmritTrackingService)is called on every field interaction. IftrackFieldInteractionis invoked frequently (e.g., on focus/blur events), this creates unnecessary overhead. Consider caching the service reference.+ private _trackingService?: AmritTrackingService; + + private get trackingService(): AmritTrackingService { + if (!this._trackingService) { + this._trackingService = this.injector.get(AmritTrackingService); + } + return this._trackingService; + } + trackFieldInteraction(fieldName: string) { - const trackingService = this.injector.get(AmritTrackingService); - trackingService.trackFieldInteraction(fieldName, 'Personal Information'); + this.trackingService.trackFieldInteraction(fieldName, 'Personal Information'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/images/tracking-high-level-diagram.pngis excluded by!**/*.png
📒 Files selected for processing (33)
matomo-documentation.md(1 hunks)matomo-local-setup.md(1 hunks)src/feedback/feedback-routing.module.ts(1 hunks)src/feedback/feedback.module.ts(1 hunks)src/feedback/pages/feedback-public-page/feedback-public-page-component.ts(1 hunks)src/feedback/services/feedback.service.ts(1 hunks)src/feedback/shared/feedback-dialog/feedback-dialog.component.html(1 hunks)src/feedback/shared/feedback-dialog/feedback-dialog.component.scss(1 hunks)src/feedback/shared/feedback-dialog/feedback-dialog.component.ts(1 hunks)src/public-api.ts(1 hunks)src/registrar/abha-components/abha-generation-success-component/abha-generation-success-component.component.html(1 hunks)src/registrar/abha-components/abha-verify-success-component/abha-verify-success-component.component.html(1 hunks)src/registrar/abha-components/download-search-abha/download-search-abha.component.html(1 hunks)src/registrar/abha-components/download-search-abha/download-search-abha.component.ts(1 hunks)src/registrar/abha-components/generate-abha-component/generate-abha-component.component.ts(1 hunks)src/registrar/abha-components/health-id-display-modal/health-id-display-modal.component.html(2 hunks)src/registrar/registration/abha-information/abha-information.component.html(4 hunks)src/registrar/registration/abha-information/abha-information.component.ts(4 hunks)src/registrar/registration/location-information/location-information.component.html(2 hunks)src/registrar/registration/location-information/location-information.component.ts(9 hunks)src/registrar/registration/personal-information/personal-information.component.html(6 hunks)src/registrar/registration/personal-information/personal-information.component.ts(5 hunks)src/registrar/registration/registration.component.ts(1 hunks)src/registrar/search-dialog/search-dialog.component.html(3 hunks)src/registrar/search-dialog/search-dialog.component.ts(7 hunks)src/registrar/search/search.component.ts(2 hunks)src/tracking/index.ts(1 hunks)src/tracking/lib/amrit-tracking.service.ts(1 hunks)src/tracking/lib/ga-tracking.service.ts(1 hunks)src/tracking/lib/matomo-tracking.service.ts(1 hunks)src/tracking/lib/tracking-provider.ts(1 hunks)src/tracking/lib/tracking.module.ts(1 hunks)src/tracking/lib/tracking.tokens.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2024-12-10T08:55:14.016Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 4
File: src/registrar/abha-components/generate-abha-component/generate-abha-component.component.ts:79-84
Timestamp: 2024-12-10T08:55:14.016Z
Learning: In the Angular component `GenerateAbhaComponentComponent` (file path `src/registrar/abha-components/generate-abha-component/generate-abha-component.component.ts`), the `res.data.message` displayed using `confirmationService.alert()` contains only safe success messages and does not require additional sanitization before displaying to users.
Applied to files:
src/registrar/abha-components/abha-verify-success-component/abha-verify-success-component.component.htmlsrc/registrar/registration/abha-information/abha-information.component.tssrc/registrar/abha-components/generate-abha-component/generate-abha-component.component.tssrc/registrar/abha-components/abha-generation-success-component/abha-generation-success-component.component.htmlsrc/registrar/registration/abha-information/abha-information.component.htmlsrc/registrar/abha-components/download-search-abha/download-search-abha.component.htmlsrc/registrar/abha-components/health-id-display-modal/health-id-display-modal.component.htmlsrc/registrar/registration/registration.component.tssrc/registrar/search-dialog/search-dialog.component.tssrc/registrar/abha-components/download-search-abha/download-search-abha.component.ts
📚 Learning: 2024-12-12T10:49:59.197Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 5
File: src/registrar/abha-components/abha-enter-mobile-otp-component/abha-enter-mobile-otp-component.component.ts:85-89
Timestamp: 2024-12-12T10:49:59.197Z
Learning: In the `verifyMobileAuthAfterAbhaCreation()` method of `AbhaEnterMobileOtpComponentComponent` (file `src/registrar/abha-components/abha-enter-mobile-otp-component/abha-enter-mobile-otp-component.component.ts`), the OTP should be sent under the `loginId` field in the request object, as per the API specifications.
Applied to files:
src/registrar/abha-components/abha-verify-success-component/abha-verify-success-component.component.htmlsrc/registrar/abha-components/generate-abha-component/generate-abha-component.component.tssrc/registrar/abha-components/download-search-abha/download-search-abha.component.htmlsrc/registrar/abha-components/download-search-abha/download-search-abha.component.ts
📚 Learning: 2024-12-12T10:50:37.574Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 5
File: src/registrar/abha-components/abha-enter-mobile-otp-component/abha-enter-mobile-otp-component.component.ts:51-54
Timestamp: 2024-12-12T10:50:37.574Z
Learning: In this Angular project, input validations for fields like the OTP input are defined in the HTML templates rather than in the component TypeScript code.
Applied to files:
src/registrar/abha-components/abha-verify-success-component/abha-verify-success-component.component.htmlsrc/registrar/registration/abha-information/abha-information.component.tssrc/registrar/registration/abha-information/abha-information.component.htmlsrc/feedback/shared/feedback-dialog/feedback-dialog.component.htmlsrc/registrar/search-dialog/search-dialog.component.htmlsrc/feedback/shared/feedback-dialog/feedback-dialog.component.tssrc/registrar/registration/location-information/location-information.component.htmlsrc/registrar/registration/personal-information/personal-information.component.htmlsrc/registrar/search-dialog/search-dialog.component.tssrc/registrar/abha-components/download-search-abha/download-search-abha.component.ts
📚 Learning: 2025-07-09T12:40:58.733Z
Learnt from: abhijeetw035
Repo: PSMRI/Common-UI PR: 20
File: src/registrar/registration/personal-information/personal-information.component.ts:29-29
Timestamp: 2025-07-09T12:40:58.733Z
Learning: The Common-UI repository is designed to be used as a git submodule in other AMRIT applications. Import paths like `import { AmritTrackingService } from 'Common-UI/src/tracking';` are correct because they resolve properly in consuming applications where Common-UI is included as a submodule, even though the Common-UI repository itself doesn't contain a .gitmodules file.
Applied to files:
src/public-api.tssrc/registrar/registration/abha-information/abha-information.component.tssrc/tracking/index.tssrc/registrar/registration/location-information/location-information.component.tssrc/feedback/feedback.module.ts
📚 Learning: 2024-12-12T10:46:10.098Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 5
File: src/registrar/abha-components/abha-enter-mobile-otp-component/abha-enter-mobile-otp-component.component.ts:44-48
Timestamp: 2024-12-12T10:46:10.098Z
Learning: In this Angular project, it is common practice to directly instantiate components like `SetLanguageComponent` for language management in methods such as `assignSelectedLanguage()`, rather than using a service. This is done to maintain consistency across the codebase.
Applied to files:
src/registrar/registration/abha-information/abha-information.component.tssrc/registrar/search-dialog/search-dialog.component.ts
📚 Learning: 2024-12-10T08:41:11.391Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 4
File: src/registrar/abha-components/abha-generation-success-component/abha-generation-success-component.component.ts:70-78
Timestamp: 2024-12-10T08:41:11.391Z
Learning: In the `AbhaGenerationSuccessComponentComponent` class (`src/registrar/abha-components/abha-generation-success-component/abha-generation-success-component.component.ts`), the `GivePageToMobileEnterOtp()` method intentionally closes the newly opened dialog immediately after opening it with `dialogRef.close();` because, after routing to another component, the previous component should be closed.
Applied to files:
src/registrar/registration/abha-information/abha-information.component.tssrc/registrar/abha-components/generate-abha-component/generate-abha-component.component.tssrc/registrar/search/search.component.tssrc/registrar/search-dialog/search-dialog.component.htmlsrc/registrar/search-dialog/search-dialog.component.tssrc/registrar/abha-components/download-search-abha/download-search-abha.component.ts
📚 Learning: 2025-07-09T12:40:58.733Z
Learnt from: abhijeetw035
Repo: PSMRI/Common-UI PR: 20
File: src/registrar/registration/personal-information/personal-information.component.ts:29-29
Timestamp: 2025-07-09T12:40:58.733Z
Learning: In the AMRIT project, Common-UI is a git submodule (as defined in .gitmodules) that provides shared components and services. Import paths like `import { AmritTrackingService } from 'Common-UI/src/tracking';` are correct and resolve properly in the runtime environment where the submodule is initialized, even though they may not be visible in development environments where submodules aren't initialized.
Applied to files:
src/registrar/registration/abha-information/abha-information.component.tssrc/tracking/index.tssrc/registrar/registration/location-information/location-information.component.tssrc/feedback/feedback.module.ts
📚 Learning: 2024-12-12T10:52:38.837Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 5
File: src/registrar/abha-components/abha-enter-mobile-otp-component/abha-enter-mobile-otp-component.component.html:28-38
Timestamp: 2024-12-12T10:52:38.837Z
Learning: In our Angular project, number validation for OTP input fields is handled using the `numberOnly($event)` keypress event handler, and length is enforced using `minlength` and `maxlength` attributes in the template (e.g., in `src/registrar/abha-components/abha-enter-mobile-otp-component/abha-enter-mobile-otp-component.component.html`). Additional `pattern` validation or `inputmode` attributes are not required in these cases.
Applied to files:
src/registrar/registration/abha-information/abha-information.component.htmlsrc/registrar/abha-components/download-search-abha/download-search-abha.component.htmlsrc/registrar/search-dialog/search-dialog.component.htmlsrc/registrar/registration/location-information/location-information.component.htmlsrc/registrar/registration/personal-information/personal-information.component.html
📚 Learning: 2024-12-10T08:42:10.330Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-UI PR: 4
File: src/registrar/abha-components/abha-generation-success-component/abha-generation-success-component.component.ts:83-85
Timestamp: 2024-12-10T08:42:10.330Z
Learning: In this project, calling `assignSelectedLanguage()` inside `ngDoCheck()` is the standard approach used across components to handle language updates consistently.
Applied to files:
src/registrar/search-dialog/search-dialog.component.ts
🧬 Code graph analysis (7)
src/feedback/feedback-routing.module.ts (1)
src/feedback/feedback.module.ts (1)
NgModule(35-48)
src/tracking/lib/tracking.module.ts (1)
src/tracking/lib/tracking.tokens.ts (5)
TRACKING_PLATFORM(5-8)TRACKING_ENABLED(10-13)MATOMO_SITE_ID(15-18)MATOMO_URL(20-23)TRACKING_PROVIDER(25-25)
src/tracking/lib/matomo-tracking.service.ts (3)
src/tracking/lib/tracking-provider.ts (1)
TrackingProvider(1-11)src/tracking/lib/tracking.tokens.ts (3)
MATOMO_SITE_ID(15-18)MATOMO_URL(20-23)TRACKING_ENABLED(10-13)src/tracking/lib/tracking.module.ts (3)
MATOMO_SITE_ID(36-36)MATOMO_URL(36-36)TRACKING_ENABLED(36-36)
src/feedback/pages/feedback-public-page/feedback-public-page-component.ts (1)
src/feedback/shared/feedback-dialog/feedback-dialog.component.ts (1)
Component(35-185)
src/tracking/lib/amrit-tracking.service.ts (3)
src/tracking/lib/tracking.tokens.ts (1)
TRACKING_PROVIDER(25-25)src/tracking/lib/tracking.module.ts (1)
TRACKING_PROVIDER(36-36)src/tracking/lib/tracking-provider.ts (1)
TrackingProvider(1-11)
src/feedback/feedback.module.ts (1)
src/feedback/feedback-routing.module.ts (1)
NgModule(33-37)
src/tracking/lib/ga-tracking.service.ts (2)
src/tracking/lib/tracking-provider.ts (1)
TrackingProvider(1-11)src/tracking/lib/tracking.tokens.ts (2)
MATOMO_SITE_ID(15-18)TRACKING_ENABLED(10-13)
🪛 LanguageTool
matomo-local-setup.md
[style] ~197-~197: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...dd this to the pages of the website you want to track. * Done: The installation...
(REP_WANT_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
matomo-documentation.md
33-33: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
64-64: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
195-195: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
300-300: Bare URL used
(MD034, no-bare-urls)
307-307: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
307-307: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
307-307: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (31)
src/registrar/abha-components/abha-verify-success-component/abha-verify-success-component.component.html (1)
15-17: ABHA number label key standardized toabhaNumberSwitching to
currentLanguageSet?.abhaNumberaligns this label with the standardized key used elsewhere; just ensure all language bundles define this key correctly.src/registrar/abha-components/abha-generation-success-component/abha-generation-success-component.component.html (1)
11-13: Consistent ABHA number label keyUsing
currentLanguageSet?.abhaNumberhere keeps the label consistent with other ABHA-related UIs while retaining the same bound value.src/registrar/abha-components/download-search-abha/download-search-abha.component.html (1)
90-94: ABHA number hint now matches standardized key and formatThe note now uses
currentLanguageSet?.abhaNumberand still documents thexx-xxxx-xxxx-xxxxpattern, which matches the ABHA-number validation logic in the TS component.src/registrar/abha-components/download-search-abha/download-search-abha.component.ts (1)
195-200: Dialog result condition now routes on any value exceptfalseChanging the check to
if (result !== false)broadens the behavior: routing to the OTP page will now occur forundefined/null/other non‑falsevalues, not just truthy ones. This is fine ifConfirmationService.alertonly returnsfalsefor an explicit cancel/negative action; otherwise it may route even when the dialog is dismissed implicitly (e.g. ESC/backdrop).src/registrar/abha-components/health-id-display-modal/health-id-display-modal.component.html (1)
123-131: Unified ABHA number column headersUpdating both tables to use
currentLanguageSet?.abhaNumberfor the ABHA number columns keeps the headers consistent across all views in this modal.Also applies to: 212-223
src/registrar/abha-components/generate-abha-component/generate-abha-component.component.ts (1)
75-83: BroaderafterClosedcondition may change when OTP routing occursHere too, switching to
if (result !== false)means the OTP page will be opened unless the dialog explicitly returnsfalse, including cases where the alert closes withundefinedor similar. Confirm this matches howConfirmationService.alertsignals cancel vs. close so you don’t route when the user intended to abort.src/registrar/search/search.component.ts (1)
389-430: LGTM - Good defensive check for dialog result.The truthy check on
resultbefore proceeding with the search prevents unnecessary API calls when the dialog is dismissed without a selection. The localization key update frombeneficiarynotfoundtobeneficiaryNotFoundaligns with the camelCase standardization seen across the PR.src/registrar/search-dialog/search-dialog.component.html (1)
80-92: Verify form validation for typed-but-not-selected autocomplete values.The
requiredattribute on the autocomplete input (line 84) provides HTML5 validation, but sincestateCtrlis a standaloneFormControlseparate fromnewSearchForm, the form'sstateIDvalidator depends ononStateSelectedbeing triggered. If a user types a partial match without selecting from the dropdown, the input appears filled butstateIDinnewSearchFormremains null, potentially causing a confusing UX where the search button is disabled despite visible input.Consider adding validation to ensure a selection was made, or sync the standalone controls with the form group.
src/registrar/search-dialog/search-dialog.component.ts (1)
253-301: Display functions and reset logic are well-implemented.The
displayWithfunctions correctly handle null/undefined by returning empty strings. TheresetBeneficiaryFormmethod properly clears both the reactive form and standalone autocomplete controls, then re-fetches states to restore initial state.src/public-api.ts (1)
1-1: Expose tracking barrel via public API (LGTM)Re‑exporting
./trackinghere is a clean way to surface all tracking modules/services/tokens through the public API; this will simplify imports for consumers.src/feedback/feedback-routing.module.ts (1)
1-37: Feedback routing module is correctly structuredThe routing module cleanly maps the empty path to
FeedbackPublicPageComponentand usesRouterModule.forChild(routes)/exports: [RouterModule]as expected for a feature module; no issues spotted.src/feedback/shared/feedback-dialog/feedback-dialog.component.scss (1)
1-71: Feedback dialog styles look cohesive and accessibleThe SCSS cleanly scopes styling to the feedback dialog, provides reasonable spacing/typography, and includes focus outlines for inputs/selects; nothing here appears problematic from a layout or basic accessibility standpoint.
src/registrar/registration/personal-information/personal-information.component.html (3)
27-27: LGTM: Tracking integration for image capture.The addition of tracking for the photo capture button is well-implemented and aligns with the broader tracking integration across forms.
48-48: LGTM: Text input tracking with appropriate fallback.The focus-based tracking with a fallback to
fieldNamewhenfieldTitleis unavailable is a robust approach.
80-80: LGTM: Consistent tracking across form controls.The tracking integration uses appropriate events for each control type:
focusfor date inputsselectionChangefor select/multi-select controlschangefor radio groupsThis provides comprehensive field interaction analytics.
Also applies to: 105-105, 130-130, 152-152
src/feedback/feedback.module.ts (1)
22-48: LGTM: Well-structured feedback module.The module follows Angular best practices:
- Clean separation between public (dialog) and internal (page) components
- Appropriate imports and provider registration
- Proper routing integration
src/feedback/shared/feedback-dialog/feedback-dialog.component.html (1)
24-39: LGTM: Accessible star rating implementation.The star rating system properly implements radio semantics with appropriate ARIA attributes (
role="radio",aria-checked,aria-label), ensuring screen reader compatibility.src/tracking/index.ts (1)
1-6: LGTM: Clean barrel export pattern.The barrel file provides a clean single entry point for all tracking-related exports, following Angular best practices.
src/tracking/lib/tracking.module.ts (1)
11-32: LGTM: Well-structured tracking module with provider factory.The
forRoot()pattern correctly implements environment-driven configuration and uses a factory to select between Matomo and Google Analytics providers based on platform configuration.The use of
||for platform default (line 15) and??for boolean enabled (line 16) is intentional and appropriate:
||treats empty string as falsy, defaulting to 'matomo'??only defaults on null/undefined, preserving explicitfalsefor enabledsrc/registrar/registration/location-information/location-information.component.html (1)
118-141: LGTM: Autocomplete implementation for location selection.The transition from
mat-selectto autocomplete with filtering improves user experience for potentially long location lists. The implementation correctly:
- Binds form control and autocomplete reference
- Triggers filtering on input
- Shows all options on focus
- Handles selection via
optionSelectedeventConsider whether field interaction tracking should be added to the autocomplete input (similar to line 15 for text inputs) for consistency with other form fields.
src/registrar/registration/location-information/location-information.component.ts (3)
83-86: LGTM: Proper initialization of filtered options.The initialization of
filteredOptionsfor each dropdown field ensures the autocomplete has data before the template renders.
113-122: LGTM: Clean autocomplete filtering logic.The helper methods provide intuitive autocomplete behavior:
showAllOptionsdisplays all options on focusfilterOptionsperforms case-insensitive substring matching
524-527: LGTM: Tracking integration using injector pattern.The tracking method correctly uses the Injector to resolve
AmritTrackingServiceat runtime, which is the appropriate pattern for the Common-UI submodule architecture. Based on learnings, this import path resolves correctly in consuming applications.src/tracking/lib/matomo-tracking.service.ts (1)
141-148: Event tracking correctly handles optional parameters.The conditional building of the
argsarray properly handles the optionallabelandvalueparameters per Matomo's API requirements.src/feedback/services/feedback.service.ts (1)
57-62: LGTM!The
submitFeedbackmethod correctly POSTs the payload and returns the typed response.src/tracking/lib/ga-tracking.service.ts (1)
81-112: Well-implemented script loading with duplicate detection.The Promise-based loading with existing script detection and proper error handling is a good pattern.
src/feedback/pages/feedback-public-page/feedback-public-page-component.ts (1)
58-71: Detection logic is reasonable.The path-based service line detection with case-insensitive matching and sensible fallback is appropriate.
src/tracking/lib/amrit-tracking.service.ts (2)
56-70:trackEventlogic is correct but verbose.The conditional branching handles all parameter combinations correctly. This could be simplified since the underlying provider already validates parameters.
106-109: Proper cleanup withdestroy$Subject.The
ngOnDestroycorrectly signals and completes the Subject, ensuring the router subscription is cleaned up.src/feedback/shared/feedback-dialog/feedback-dialog.component.ts (1)
103-104: Default category selection may not honor@Input() defaultCategorySlug.The current logic prioritizes the first loaded category's slug over the input
defaultCategorySlug:const def = this.categories[0]?.slug || this.defaultCategorySlug || "";If the intent is to use
defaultCategorySlugwhen provided, consider reversing the priority:- const def = this.categories[0]?.slug || this.defaultCategorySlug || ""; + const def = this.defaultCategorySlug || this.categories[0]?.slug || "";src/registrar/registration/personal-information/personal-information.component.ts (1)
29-30: LGTM!The imports for
AmritTrackingServiceandInjectorare correctly added and follow the established pattern used in sibling components for field interaction tracking.
| import { ActivatedRoute } from "@angular/router"; | ||
| import { map } from "rxjs/operators"; | ||
|
|
||
| type SL = "1097" | "104" | "AAM" | "MMU" | "TM" | "ECD"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate type definition.
SL duplicates ServiceLine from feedback.service.ts. Import and reuse it instead.
import { Component } from "@angular/core";
import { ActivatedRoute } from "@angular/router";
import { map } from "rxjs/operators";
+import { ServiceLine } from "../../services/feedback.service";
-type SL = "1097" | "104" | "AAM" | "MMU" | "TM" | "ECD";
+type SL = ServiceLine;Or use ServiceLine directly throughout the component.
📝 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.
| type SL = "1097" | "104" | "AAM" | "MMU" | "TM" | "ECD"; | |
| import { Component } from "@angular/core"; | |
| import { ActivatedRoute } from "@angular/router"; | |
| import { map } from "rxjs/operators"; | |
| import { ServiceLine } from "../../services/feedback.service"; | |
| type SL = ServiceLine; |
🤖 Prompt for AI Agents
In src/feedback/pages/feedback-public-page/feedback-public-page-component.ts
around line 26, the local type alias SL duplicates the existing ServiceLine type
in feedback.service.ts; remove the SL declaration, import ServiceLine from the
service file, and replace all uses of SL with ServiceLine (or use ServiceLine
directly wherever the alias was referenced) to avoid duplication and ensure a
single source of truth.
| constructor(private route: ActivatedRoute) { | ||
| // Check query param ?sl= | ||
| this.route.queryParamMap | ||
| .pipe(map((q) => (q.get("sl") as SL) || this.detectFromLocation())) | ||
| .subscribe((sl) => (this.serviceLine = sl)); | ||
| } |
There was a problem hiding this comment.
Subscription in constructor without cleanup causes memory leak.
The queryParamMap subscription is never unsubscribed when the component is destroyed.
+import { Component, OnDestroy } from "@angular/core";
+import { Subject } from "rxjs";
+import { takeUntil } from "rxjs/operators";
-export class FeedbackPublicPageComponent {
+export class FeedbackPublicPageComponent implements OnDestroy {
serviceLine: SL = "AAM";
+ private destroy$ = new Subject<void>();
constructor(private route: ActivatedRoute) {
this.route.queryParamMap
.pipe(
+ takeUntil(this.destroy$),
map((q) => (q.get("sl") as SL) || this.detectFromLocation())
)
.subscribe((sl) => (this.serviceLine = sl));
}
+ ngOnDestroy() {
+ this.destroy$.next();
+ this.destroy$.complete();
+ }📝 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.
| constructor(private route: ActivatedRoute) { | |
| // Check query param ?sl= | |
| this.route.queryParamMap | |
| .pipe(map((q) => (q.get("sl") as SL) || this.detectFromLocation())) | |
| .subscribe((sl) => (this.serviceLine = sl)); | |
| } | |
| import { Component, OnDestroy } from "@angular/core"; | |
| import { Subject } from "rxjs"; | |
| import { takeUntil } from "rxjs/operators"; | |
| export class FeedbackPublicPageComponent implements OnDestroy { | |
| serviceLine: SL = "AAM"; | |
| private destroy$ = new Subject<void>(); | |
| constructor(private route: ActivatedRoute) { | |
| // Check query param ?sl= | |
| this.route.queryParamMap | |
| .pipe( | |
| takeUntil(this.destroy$), | |
| map((q) => (q.get("sl") as SL) || this.detectFromLocation()) | |
| ) | |
| .subscribe((sl) => (this.serviceLine = sl)); | |
| } | |
| ngOnDestroy() { | |
| this.destroy$.next(); | |
| this.destroy$.complete(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/feedback/pages/feedback-public-page/feedback-public-page-component.ts
around lines 51 to 56, the component subscribes to route.queryParamMap in the
constructor but never unsubscribes, causing a memory leak; change this so the
subscription is tied to the component lifecycle by moving the subscription to
ngOnInit (or keeping it where it is) and use a teardown pattern such as a
private destroy$ = new Subject<void>() with .pipe(takeUntil(this.destroy$)) on
the observable, then implement ngOnDestroy to call this.destroy$.next() and
this.destroy$.complete(); alternatively store the Subscription in a field and
call unsubscribe() in ngOnDestroy—ensure the subscription is cleaned up when the
component is destroyed.
| listCategories(serviceLine: ServiceLine): Observable<CategoryDto[]> { | ||
| const params = new HttpParams().set("serviceLine", serviceLine); | ||
| return this.http.get<CategoryDto[]>( | ||
| `${this.apiBase}/platform-feedback/categories`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
HttpParams created but not used in the request.
The params variable with serviceLine is constructed but never passed to http.get(), so the API receives no filter.
listCategories(serviceLine: ServiceLine): Observable<CategoryDto[]> {
const params = new HttpParams().set("serviceLine", serviceLine);
return this.http.get<CategoryDto[]>(
`${this.apiBase}/platform-feedback/categories`,
+ { params }
);
}📝 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.
| listCategories(serviceLine: ServiceLine): Observable<CategoryDto[]> { | |
| const params = new HttpParams().set("serviceLine", serviceLine); | |
| return this.http.get<CategoryDto[]>( | |
| `${this.apiBase}/platform-feedback/categories`, | |
| ); | |
| } | |
| listCategories(serviceLine: ServiceLine): Observable<CategoryDto[]> { | |
| const params = new HttpParams().set("serviceLine", serviceLine); | |
| return this.http.get<CategoryDto[]>( | |
| `${this.apiBase}/platform-feedback/categories`, | |
| { params } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/feedback/services/feedback.service.ts around lines 50 to 55, the
HttpParams named `params` is created but never passed to the HTTP call; update
the method to pass the params into this.http.get as the options argument (e.g.
this.http.get<CategoryDto[]>(`${this.apiBase}/platform-feedback/categories`, {
params })) and ensure `serviceLine` is converted to a string if it’s an enum or
non-string type so the query parameter is serialized correctly.
| <select | ||
| id="fb-cat" | ||
| class="fb-select" | ||
| [formControl]="form.controls['categorySlug']" | ||
| aria-placeholder="Select a category" | ||
| aria-label="Select a category" | ||
| > |
There was a problem hiding this comment.
Remove invalid aria-placeholder attribute.
The aria-placeholder attribute on line 52 is not valid for <select> elements. The aria-label on line 53 is sufficient for accessibility, and the disabled option provides the placeholder text visually.
Apply this diff:
<select
id="fb-cat"
class="fb-select"
[formControl]="form.controls['categorySlug']"
- aria-placeholder="Select a category"
aria-label="Select a category"
>📝 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.
| <select | |
| id="fb-cat" | |
| class="fb-select" | |
| [formControl]="form.controls['categorySlug']" | |
| aria-placeholder="Select a category" | |
| aria-label="Select a category" | |
| > | |
| <select | |
| id="fb-cat" | |
| class="fb-select" | |
| [formControl]="form.controls['categorySlug']" | |
| aria-label="Select a category" | |
| > |
🤖 Prompt for AI Agents
In src/feedback/shared/feedback-dialog/feedback-dialog.component.html around
lines 48 to 54, remove the invalid aria-placeholder attribute from the <select>
element; keep the existing aria-label and the disabled placeholder option for
accessibility and visual cue, ensuring no other attributes change.
| this.form.reset({ | ||
| rating: 0, | ||
| categorySlug: this.categories[0]?.slug ?? "", | ||
| comment: "", | ||
| isAnonymous: this.isLoggedIn ? true : true, | ||
| }); |
There was a problem hiding this comment.
Redundant ternary expression: always evaluates to true.
The expression this.isLoggedIn ? true : true is equivalent to just true. This appears to be leftover from incomplete logic or a copy-paste error.
this.form.reset({
rating: 0,
categorySlug: this.categories[0]?.slug ?? "",
comment: "",
- isAnonymous: this.isLoggedIn ? true : true,
+ isAnonymous: true,
});📝 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.
| this.form.reset({ | |
| rating: 0, | |
| categorySlug: this.categories[0]?.slug ?? "", | |
| comment: "", | |
| isAnonymous: this.isLoggedIn ? true : true, | |
| }); | |
| this.form.reset({ | |
| rating: 0, | |
| categorySlug: this.categories[0]?.slug ?? "", | |
| comment: "", | |
| isAnonymous: true, | |
| }); |
🤖 Prompt for AI Agents
In src/feedback/shared/feedback-dialog/feedback-dialog.component.ts around lines
167 to 172, the form reset sets isAnonymous using a redundant ternary
this.isLoggedIn ? true : true; replace that with the intended boolean logic
(e.g., this.isLoggedIn ? false : true or simply true/false depending on desired
behavior) so isAnonymous reflects the user's login state — update the expression
to the correct conditional or a direct boolean literal and run tests to confirm
the form default is correct.
| setUserId(userId: string | number) { | ||
| if (userId && userId !== null && userId !== undefined && userId !== '') { | ||
| const userIdString = userId.toString(); | ||
| this.trackingProvider.setUserId(userIdString); | ||
| console.log('User ID manually set in tracking provider:', userIdString); | ||
| } else { | ||
| console.warn('Invalid user ID provided to setUserId:', userId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Logging user ID may expose PII.
Line 100 logs the user ID to console, which could expose personally identifiable information in browser developer tools or log aggregation.
setUserId(userId: string | number) {
if (userId && userId !== null && userId !== undefined && userId !== '') {
const userIdString = userId.toString();
this.trackingProvider.setUserId(userIdString);
- console.log('User ID manually set in tracking provider:', userIdString);
+ // User ID set successfully - avoid logging actual value for privacy
} else {
console.warn('Invalid user ID provided to setUserId:', userId);
}
}📝 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.
| setUserId(userId: string | number) { | |
| if (userId && userId !== null && userId !== undefined && userId !== '') { | |
| const userIdString = userId.toString(); | |
| this.trackingProvider.setUserId(userIdString); | |
| console.log('User ID manually set in tracking provider:', userIdString); | |
| } else { | |
| console.warn('Invalid user ID provided to setUserId:', userId); | |
| } | |
| } | |
| setUserId(userId: string | number) { | |
| if (userId && userId !== null && userId !== undefined && userId !== '') { | |
| const userIdString = userId.toString(); | |
| this.trackingProvider.setUserId(userIdString); | |
| // User ID set successfully - avoid logging actual value for privacy | |
| } else { | |
| console.warn('Invalid user ID provided to setUserId:', userId); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/tracking/lib/amrit-tracking.service.ts around lines 96 to 104, the
current console.log prints the raw userId which can expose PII; remove or
replace the console.log to avoid logging raw identifiers — either remove the log
entirely or log only a non-sensitive indicator (e.g. "user id set") or a safely
masked/hashed version of userId, and ensure any logging of identifiers is gated
behind a debug flag and does not print the original value.
| this.executeWhenReady(() => { | ||
| if (typeof gtag !== 'undefined') { | ||
| gtag('config', this.gaTrackingId, { | ||
| user_id: userId | ||
| }); | ||
| console.log('GA User ID set:', userId); |
There was a problem hiding this comment.
Tracking ID inconsistency: setUserId uses this.gaTrackingId instead of configured ID.
If init() is called with a different gaTrackingId parameter, subsequent setUserId and pageView calls still use the original injected this.gaTrackingId. Store the configured ID during init() for consistent usage.
+ private configuredTrackingId: string = '';
+
init(gaTrackingId?: string | number, trackerUrl?: string) {
// ...
const finalTrackingId = gaTrackingId?.toString() || this.gaTrackingId?.toString();
+ this.configuredTrackingId = finalTrackingId;
// ...
}
setUserId(userId: string) {
// ...
this.executeWhenReady(() => {
if (typeof gtag !== 'undefined') {
- gtag('config', this.gaTrackingId, {
+ gtag('config', this.configuredTrackingId, {
user_id: userId
});📝 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.
| this.executeWhenReady(() => { | |
| if (typeof gtag !== 'undefined') { | |
| gtag('config', this.gaTrackingId, { | |
| user_id: userId | |
| }); | |
| console.log('GA User ID set:', userId); | |
| this.executeWhenReady(() => { | |
| if (typeof gtag !== 'undefined') { | |
| gtag('config', this.configuredTrackingId, { | |
| user_id: userId | |
| }); | |
| console.log('GA User ID set:', userId); |
🤖 Prompt for AI Agents
In src/tracking/lib/ga-tracking.service.ts around lines 122 to 127, setUserId
(and related pageView calls) still reference the original injected
this.gaTrackingId instead of the GA ID passed to init(); persist the GA ID
provided to init() on the instance (e.g., set this.configuredGaTrackingId =
gaTrackingId inside init()) and update setUserId and pageView to use that stored
configured ID (fallback to the injected one only if configuredGaTrackingId is
undefined); ensure executeWhenReady closures/readers reference the stored
configuredGaTrackingId so subsequent calls use the latest configured ID.
| constructor( | ||
| @Inject(MATOMO_SITE_ID) private siteId: number, | ||
| @Inject(MATOMO_URL) private trackerUrl: string, | ||
| @Inject(TRACKING_ENABLED) private trackingEnabled: boolean | ||
| ) { |
There was a problem hiding this comment.
Type mismatch with MATOMO_SITE_ID token.
The MATOMO_SITE_ID token is typed as string | number in tracking.tokens.ts, but the constructor declares siteId: number. If the environment provides a string, the validation at line 34 will reject it.
constructor(
- @Inject(MATOMO_SITE_ID) private siteId: number,
+ @Inject(MATOMO_SITE_ID) private siteId: string | number,
@Inject(MATOMO_URL) private trackerUrl: string,
@Inject(TRACKING_ENABLED) private trackingEnabled: boolean
) {Then update validation at line 34 to coerce strings:
- if (!finalSiteId || typeof finalSiteId !== 'number') {
+ const numericSiteId = typeof finalSiteId === 'string' ? parseInt(finalSiteId, 10) : finalSiteId;
+ if (!numericSiteId || isNaN(numericSiteId)) {📝 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.
| constructor( | |
| @Inject(MATOMO_SITE_ID) private siteId: number, | |
| @Inject(MATOMO_URL) private trackerUrl: string, | |
| @Inject(TRACKING_ENABLED) private trackingEnabled: boolean | |
| ) { | |
| constructor( | |
| @Inject(MATOMO_SITE_ID) private siteId: string | number, | |
| @Inject(MATOMO_URL) private trackerUrl: string, | |
| @Inject(TRACKING_ENABLED) private trackingEnabled: boolean | |
| ) { |
🤖 Prompt for AI Agents
In src/tracking/lib/matomo-tracking.service.ts around lines 10 to 14, the
constructor types the injected MATOMO_SITE_ID as number but the token is string
| number; change the constructor parameter to accept string | number (or
unknown) for siteId, then update the validation at line 34 to coerce string
values into a number (e.g., if typeof siteId === 'string' and it represents an
integer parse it to a number) and reject only truly invalid/non-numeric values;
ensure subsequent uses of siteId use the parsed number.
| _paq.push(['setTrackerUrl', finalTrackerUrl + 'matomo.php']); | ||
| _paq.push(['setSiteId', finalSiteId]); |
There was a problem hiding this comment.
Missing trailing slash normalization may cause malformed URLs.
If finalTrackerUrl doesn't end with /, the concatenation produces URLs like https://matomo.examplematomo.php instead of https://matomo.example/matomo.php.
- _paq.push(['setTrackerUrl', finalTrackerUrl + 'matomo.php']);
+ const baseUrl = finalTrackerUrl.endsWith('/') ? finalTrackerUrl : finalTrackerUrl + '/';
+ _paq.push(['setTrackerUrl', baseUrl + 'matomo.php']);Apply the same fix in loadMatomoScript at line 69.
📝 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.
| _paq.push(['setTrackerUrl', finalTrackerUrl + 'matomo.php']); | |
| _paq.push(['setSiteId', finalSiteId]); | |
| const baseUrl = finalTrackerUrl.endsWith('/') ? finalTrackerUrl : finalTrackerUrl + '/'; | |
| _paq.push(['setTrackerUrl', baseUrl + 'matomo.php']); | |
| _paq.push(['setSiteId', finalSiteId]); |
🤖 Prompt for AI Agents
In src/tracking/lib/matomo-tracking.service.ts around lines 52-53, the code
concatenates finalTrackerUrl with 'matomo.php' and 'matomo.php' causing
malformed URLs when finalTrackerUrl lacks a trailing slash; normalize
finalTrackerUrl by ensuring it ends with a single '/' (e.g., if it doesn't end
with '/', append one) before concatenation, and apply the same normalization in
loadMatomoScript at line 69 so both uses safely produce valid URLs.
| const firstScript = document.getElementsByTagName('script')[0]; | ||
| if (firstScript && firstScript.parentNode) { | ||
| firstScript.parentNode.insertBefore(script, firstScript); | ||
| } |
There was a problem hiding this comment.
Script insertion silently fails if no existing script tags are found.
If document.getElementsByTagName('script')[0] returns undefined, the script is never inserted.
const firstScript = document.getElementsByTagName('script')[0];
if (firstScript && firstScript.parentNode) {
firstScript.parentNode.insertBefore(script, firstScript);
+ } else {
+ document.head.appendChild(script);
}📝 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.
| const firstScript = document.getElementsByTagName('script')[0]; | |
| if (firstScript && firstScript.parentNode) { | |
| firstScript.parentNode.insertBefore(script, firstScript); | |
| } | |
| const firstScript = document.getElementsByTagName('script')[0]; | |
| if (firstScript && firstScript.parentNode) { | |
| firstScript.parentNode.insertBefore(script, firstScript); | |
| } else { | |
| document.head.appendChild(script); | |
| } |
🤖 Prompt for AI Agents
In src/tracking/lib/matomo-tracking.service.ts around lines 79 to 82, the code
only inserts the new script before the first existing <script> and silently
fails if none exist; change the insertion logic to fall back to appending the
script to a safe container (prefer document.head, then document.body, then
document.documentElement) when getElementsByTagName('script')[0] or its
parentNode is missing, and use insertBefore when a parent exists otherwise use
appendChild on the chosen fallback element so the script is always inserted.
PR for Production 3.6
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.