RI-6864: add reason to ANALYTICS_PERMISSION event.
#4403
Merged
Conversation
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
If the event is triggered by accepting consents, reason is `install`, if it is because of `sso\social login\oauth` reasons, appropriate strategy is sent `google | github | sso`. One last place where analytics is being enabled is in OAuthAgreement.tsx, where reason is sent as `oauth-agreement`
pd-redis
requested review from
ArtemHoruzhenko,
Copilot,
KIvanow,
pawelangelow,
KrumTy,
seth-riedel and
dantovska
There was a problem hiding this comment.
PR Overview
This PR adds support for including a reason when the analytics permission event is triggered. It updates both the API DTO and service layers as well as multiple UI components and Redux actions to consistently propagate a new analyticsReason parameter.
- Added analyticsReason property with validation in the settings DTO.
- Updated UI components and Redux actions to pass a reason for enabling analytics.
- Modified API services and event emitters to include the reason in analytics permission events.
Reviewed Changes
| File | Description |
|---|---|
| redisinsight/api/src/modules/settings/dto/settings.dto.ts | Added an optional analyticsReason field with validation and API documentation. |
| redisinsight/ui/src/components/consents-settings/ConsentsSettings.tsx | Updated consent logic to include analyticsReason when dispatching settings updates. |
| redisinsight/api/src/modules/settings/models/agreements.interface.ts | Added a new field requiredText to IAgreement for richer agreement spec support. |
| redisinsight/ui/src/components/oauth/shared/oauth-agreement/OAuthAgreement.tsx | Updated analytics action dispatch with a specific reason ('oauth-agreement'). |
| redisinsight/ui/src/slices/user/user-settings.ts | Updated error handling to cast errors as AxiosError and modified enableUserAnalyticsAction to accept a reason parameter. |
| redisinsight/ui/src/components/oauth/shared/oauth-form/OAuthForm.tsx | Modified analytics action dispatch to pass the selected auth strategy as the reason. |
| redisinsight/api/src/modules/settings/settings.analytics.ts | Propagated the reason parameter to analytics event emission. |
| redisinsight/api/src/modules/settings/settings.service.ts | Updated settings update flow to destructure and pass analyticsReason from the DTO to agreement updates. |
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
redisinsight/ui/src/slices/user/user-settings.ts:187
- [nitpick] The function now accepts a reason parameter with a default empty string. Consider adding documentation or comments to clarify the acceptable values and usage context for the reason parameter to aid future maintenance.
export function enableUserAnalyticsAction(reason = '') {
pd-redis
commented
Feb 28, 2025
| const handleCheck = (e: ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.checked) { | ||
| dispatch(enableUserAnalyticsAction()) | ||
| dispatch(enableUserAnalyticsAction('oauth-agreement')) |
There was a problem hiding this comment.
What do you thing about extracting these in something like AnalyticsReason?
| }) | ||
| @IsOptional() | ||
| @IsString() | ||
| analyticsReason?: string; |
There was a problem hiding this comment.
I didn't find any other usage, but there's a IsIn decorator that can provide validation on predefined set of values. And we can use a enum for it:
@IsIn(Object.values(AnalyticsReason)
…, does not change overall execution of the code
KIvanow
previously approved these changes
Mar 21, 2025
KIvanow
approved these changes
Mar 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If the event is triggered by accepting consents, reason is
install, if it is because ofsso\social-login\oauthreasons, appropriate strategy is sentgoogle | github | sso. One last place where analytics is being enabled is in OAuthAgreement.tsx, where reason is sent asoauth-agreement