-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(con): unsubscribe from marketing emails on CON #995
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces modifications across multiple files to incorporate a new field, Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (15)
libs/data-access/src/lib/connect/con-profiles/con-profile.fragment.graphql (1)
41-41
: Add a description comment for the new field.Consider adding a description comment to document the field's purpose and default value.
+ """ + Indicates whether the user has opted in to receive marketing emails. + Defaults to true for new users. + """ isSubscribedToCONMarketingEmailslibs/common-types/src/lib/con-profile/con-profile.recordprops.ts (1)
40-40
: Add @type decorator for consistencyConsider adding the
@Type
decorator to maintain consistency with other boolean fields in the class:+ @Type(() => Boolean) Subscribed_to_CON_Marketing_Emails__c: boolean
libs/common-types/src/lib/con-profile/con-profile.record.ts (1)
64-64
: Consider documenting the field's behavior.Since this field is crucial for GDPR compliance and user preferences, consider adding a JSDoc comment to document:
- The default value for new profiles
- Whether it's required or optional
- Its relationship with email sending logic
+ /** Controls whether the user receives marketing emails. Default: true. + * Used by email sending logic to filter recipients of marketing content. + */ 'Subscribed_to_CON_Marketing_Emails__c',libs/common-types/src/lib/con-profile/con-profile.mapper.ts (2)
133-134
: LGTM! Consider adding bidirectional mapping tests.The mapping is correctly implemented and symmetrical with the fromPersistence method.
Would you like me to help create unit tests to verify the bidirectional mapping of the new subscription field? This would ensure that the value is preserved when converting between entity and persistence layers.
Line range hint
1-152
: Implementation aligns well with the unsubscribe feature requirements.The mapper changes are minimal, focused, and maintain consistency with the existing codebase. The bidirectional mapping of the new subscription field provides the necessary data layer support for the marketing email preferences feature described in the PR objectives.
Consider documenting the default value behavior for the subscription field, as the PR objectives mention that users should be subscribed by default. This could be handled either at the database level or in the entity creation logic.
apps/redi-connect/src/pages/app/me/Me.tsx (5)
Line range hint
71-73
: Address the TODO comment about error handlingThe TODO comment indicates missing error handling for query failures. This is particularly important for the new subscription feature to ensure users receive proper feedback when updates fail.
Consider implementing proper error handling:
- Display error messages to users
- Handle network failures gracefully
- Provide retry mechanisms
Would you like me to help implement comprehensive error handling for these scenarios?
245-265
: Enhance checkbox accessibility and user experienceThe mentee subscription checkbox implementation needs improvements in accessibility and user feedback.
Consider these enhancements:
<Element className="block-separator"> <Checkbox checked={conProfile?.isSubscribedToCONMarketingEmails} customOnChange={onSubscribeToMarketingEmailsChange} + disabled={isUpdating} + aria-label="Toggle mentee marketing email subscription" > <Tooltip title={ <span className="tooltip-text"> - By selecting this option, you'll receive notifications about - new mentors (if unmatched) and a check-in email for - mentorships lasting over 3 months. + By selecting this option, you'll receive marketing emails including: + • Notifications about new mentor opportunities + • Quarterly mentorship check-in reminders + • Platform updates and announcements + You can unsubscribe at any time. </span> } placement="top-start" > <span>Receive mentee updates</span> </Tooltip> </Checkbox> </Element>
267-286
: Apply similar enhancements to mentor checkboxThe mentor subscription checkbox needs the same improvements as the mentee checkbox.
Apply similar enhancements:
<Element className="block-separator"> <Checkbox checked={conProfile?.isSubscribedToCONMarketingEmails} customOnChange={onSubscribeToMarketingEmailsChange} + disabled={isUpdating} + aria-label="Toggle mentor marketing email subscription" > <Tooltip title={ <span className="tooltip-text"> - By selecting this option, you'll receive a check-in email for - mentorships lasting over 3 months. + By selecting this option, you'll receive marketing emails including: + • Quarterly mentorship check-in reminders + • Platform updates and announcements + You can unsubscribe at any time. </span> } placement="top-start" > <span>Receive mentor updates</span> </Tooltip> </Checkbox> </Element>
245-286
: Extract subscription checkbox into a reusable componentThe mentee and mentor subscription checkboxes share similar structure and behavior. Consider extracting them into a reusable component to improve maintainability.
Create a new component like this:
interface MarketingSubscriptionProps { userType: 'mentee' | 'mentor' isChecked: boolean isUpdating: boolean onToggle: () => Promise<void> } const MarketingSubscription: React.FC<MarketingSubscriptionProps> = ({ userType, isChecked, isUpdating, onToggle }) => { const tooltipContent = { mentee: "• Notifications about new mentor opportunities\n• Quarterly mentorship check-in reminders\n• Platform updates and announcements", mentor: "• Quarterly mentorship check-in reminders\n• Platform updates and announcements" } return ( <Element className="block-separator"> <Checkbox checked={isChecked} customOnChange={onToggle} disabled={isUpdating} aria-label={`Toggle ${userType} marketing email subscription`} > <Tooltip title={ <span className="tooltip-text"> By selecting this option, you'll receive marketing emails including: {tooltipContent[userType]} You can unsubscribe at any time. </span> } placement="top-start" > <span>Receive {userType} updates</span> </Tooltip> </Checkbox> </Element> ) }Then use it like this:
{isMentee && ( <MarketingSubscription userType="mentee" isChecked={conProfile?.isSubscribedToCONMarketingEmails} isUpdating={isUpdating} onToggle={onSubscribeToMarketingEmailsChange} /> )} {isMentor && ( <MarketingSubscription userType="mentor" isChecked={conProfile?.isSubscribedToCONMarketingEmails} isUpdating={isUpdating} onToggle={onSubscribeToMarketingEmailsChange} /> )}
245-286
: Consider adding GDPR compliance informationTo better align with the PR objectives and privacy regulations, consider adding explicit GDPR compliance information.
Suggestions:
- Add a link to privacy policy near the checkboxes
- Mention that this is a default opt-in feature
- Consider adding a first-time notification informing users about their default subscription status
Example tooltip addition:
title={ <span className="tooltip-text"> By selecting this option, you'll receive marketing emails including: ... - You can unsubscribe at any time. + You can unsubscribe at any time. By default, you're subscribed to these updates. + See our privacy policy for details on how we handle your data. </span> }apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (3)
655-658
: Improve URL constructionWhile the formatting change improves readability, consider extracting the base URL to a configuration constant.
+private readonly BASE_URL = 'https://connect.redi-school.org/app' + .replace( /\${logMentoringSessionUrl}/g, - `https://connect.redi-school.org/app/mentorships/${mentorshipMatchId}` + `${this.BASE_URL}/mentorships/${mentorshipMatchId}` )
238-238
: Type definition could be improvedThe type definition for
mentorshipMatchesWithSessionsCount
could be extracted to an interface for better maintainability.+interface MentorshipMatchWithSessions { + mentorshipMatchId: string + menteeId: string + mentorId: string + mentoringSessionsCount: number + matchMadeActiveOn: Date +} + -const mentorshipMatchesWithSessionsCount = {} as Record< - string, - { - mentorshipMatchId: string - menteeId: string - mentorId: string - mentoringSessionsCount: number - matchMadeActiveOn: Date - } -> +const mentorshipMatchesWithSessionsCount = {} as Record<string, MentorshipMatchWithSessions>
Line range hint
1-795
: Consider architectural improvementsThe service currently handles both data querying and email sending responsibilities. Consider the following architectural improvements:
Split the service into separate services:
ReminderQueriesService
for data retrievalReminderEmailsService
for email sendingCreate a shared helper for subscription filtering to ensure consistent application across all queries.
Implement proper error handling and logging for email sending operations instead of just console.log.
Consider using a queue system for email sending to handle failures and retries.
apps/redi-connect/src/assets/locales/en/translation.json (2)
88-91
: Consider clarifying the checkbox name for unsubscribing.The FAQ entry effectively explains the check-in process and mentions the unsubscribe option. However, to improve clarity, consider specifying the exact name of the checkbox in Profile Settings.
- "answer": "Yes! If your mentorship has continued for over three months, we'll reach out to hear about your experience, including any highlights, milestones, or challenges. Your feedback is optional but invaluable, as it helps us improve the mentorship program to keep it meaningful for everyone involved. You can also unsubscribe from these check-in emails by unchecking a box in your Profile Settings." + "answer": "Yes! If your mentorship has continued for over three months, we'll reach out to hear about your experience, including any highlights, milestones, or challenges. Your feedback is optional but invaluable, as it helps us improve the mentorship program to keep it meaningful for everyone involved. You can also unsubscribe from these check-in emails by unchecking the \"Receive mentee/mentor updates\" checkbox in your Profile Settings."
158-161
: Consider adding information about default subscription status.The FAQ entry clearly explains the reminder system and unsubscribe option. To set proper expectations, consider mentioning that users are subscribed by default, as stated in the PR objectives.
- "answer": "Absolutely! If you're still unmatched 45 days after your profile is approved, we'll send you a reminder email explaining the benefits of having a mentor and the next steps to take. You can unsubscribe from these reminder emails anytime by unchecking a box in your Profile Settings." + "answer": "Absolutely! If you're still unmatched 45 days after your profile is approved, we'll send you a reminder email explaining the benefits of having a mentor and the next steps to take. By default, all users are subscribed to these reminder emails, but you can unsubscribe anytime by unchecking the \"Receive mentee/mentor updates\" checkbox in your Profile Settings."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
apps/nestjs-api/src/con-profiles/dtos/patch-con-profile.entityinput.ts
(1 hunks)apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts
(4 hunks)apps/redi-connect/src/assets/locales/en/translation.json
(2 hunks)apps/redi-connect/src/pages/app/me/Me.scss
(1 hunks)apps/redi-connect/src/pages/app/me/Me.tsx
(5 hunks)apps/redi-connect/src/pages/app/profile/Profile.generated.ts
(1 hunks)libs/common-types/src/lib/con-profile/con-profile.entityprops.ts
(1 hunks)libs/common-types/src/lib/con-profile/con-profile.mapper.ts
(2 hunks)libs/common-types/src/lib/con-profile/con-profile.record.ts
(1 hunks)libs/common-types/src/lib/con-profile/con-profile.recordprops.ts
(1 hunks)libs/data-access/src/lib/connect/con-profiles/con-profile.fragment.generated.ts
(2 hunks)libs/data-access/src/lib/connect/con-profiles/con-profile.fragment.graphql
(1 hunks)libs/data-access/src/lib/connect/my-con-profile/my-con-profile.generated.ts
(1 hunks)libs/data-access/src/lib/types/types.ts
(2 hunks)schema.graphql
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/redi-connect/src/pages/app/me/Me.scss
🔇 Additional comments (16)
apps/nestjs-api/src/con-profiles/dtos/patch-con-profile.entityinput.ts (1)
36-36
: LGTM! Clean implementation of the marketing email preference field.
The addition of isSubscribedToCONMarketingEmails
to the DTO is well-structured and aligns with the existing pattern.
Let's verify the GraphQL schema integration and field usage:
libs/common-types/src/lib/con-profile/con-profile.recordprops.ts (1)
40-40
: Verify migration strategy for required field
The new field is marked as required (non-optional) which could affect existing records. Please ensure:
- A migration strategy is in place to set default values for existing records
- The frontend handles this field appropriately during profile creation
Let's check for other required boolean fields in the codebase to ensure consistency:
libs/common-types/src/lib/con-profile/con-profile.record.ts (1)
64-64
: LGTM! Verify related components.
The new Salesforce field follows proper naming conventions and is appropriately placed in the metadata.
Let's verify the integration with related components:
✅ Verification successful
Re-verifying related components with corrected file types.
Re-verifying related components with updated search parameters.
Verified integration of isSubscribedToCONMarketingEmails
across related components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for corresponding GraphQL schema updates and UI components
# Check GraphQL schema for the new field
rg -A 5 "isSubscribedToCONMarketingEmails" --type graphql
# Check UI components that might need updates
rg -A 5 "isSubscribedToCONMarketingEmails" --type typescript --type tsx
Length of output: 196
Script:
#!/bin/bash
# Description: Re-check GraphQL schema updates and UI components with corrected file types
# Check GraphQL schema for the new field
rg -A 5 "isSubscribedToCONMarketingEmails" --type graphql
# Check UI components that might need updates
rg -A 5 "isSubscribedToCONMarketingEmails" --type ts --type tsx
Length of output: 181
Script:
#!/bin/bash
# Description: Re-check GraphQL schema updates and UI components using file extensions
# Check GraphQL schema for the new field
rg -A 5 "isSubscribedToCONMarketingEmails" --glob "*.graphql"
# Check UI components that might need updates
rg -A 5 "isSubscribedToCONMarketingEmails" --glob "*.ts" --glob "*.tsx"
Length of output: 19592
libs/data-access/src/lib/connect/con-profiles/con-profile.fragment.generated.ts (2)
1-1
:
Warning: Direct modifications to generated files
This file is marked as generated with the comment "THIS FILE IS GENERATED, DO NOT EDIT!". Instead of modifying this file directly, you should update the source schema or configuration that generates it.
Let's verify the generation source:
Also applies to: 4-4, 47-47
4-4
: Verify default value handling for isSubscribedToCONMarketingEmails
The PR objectives mention that users should be subscribed to marketing emails by default. We should verify that this boolean field is properly initialized with a default value of true
in the resolver or mutation handlers.
Let's check the resolver implementation:
Also applies to: 47-47
apps/redi-connect/src/pages/app/profile/Profile.generated.ts (2)
12-12
: LGTM: Type definition aligns with requirements
The addition of isSubscribedToCONMarketingEmails: boolean
to the ProfilePageQueryQuery
type correctly implements the marketing email subscription feature as specified in the PR objectives.
Let's verify the completeness of the implementation:
Line range hint 1-1
: Warning: This is a generated file
The file header indicates this is a generated file that should not be edited directly. Any changes should be made to the GraphQL schema instead, and then this file should be regenerated.
Let's verify the source of these changes:
✅ Verification successful
Generated file correctly updated via GraphQL schema. No direct edits necessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the GraphQL schema defines this field and verify the generation setup
# Look for the field definition in schema files
rg -t graphql "isSubscribedToCONMarketingEmails"
# Check for GraphQL codegen configuration
fd -e yml -e yaml -e json "codegen" | xargs cat
Length of output: 951
Script:
#!/bin/bash
# Description: Explicitly search for the field in .graphql files and list all .graphql files for verification
# Search for the field definition in .graphql files
rg "isSubscribedToCONMarketingEmails" --type-add 'graphql:*.graphql' -t graphql
# List all .graphql files to verify their locations
fd -e graphql
Length of output: 9290
Script:
Length of output: 0
Script:
#!/bin/bash
# Description: Verify usage of `isSubscribedToCONMarketingEmails` across GraphQL documents
# Search for all GraphQL documents that include the `isSubscribedToCONMarketingEmails` field
rg "isSubscribedToCONMarketingEmails" --type-add 'graphql:*.graphql' -t graphql
# Verify that the codegen configuration includes paths to all relevant GraphQL documents
rg "documents" --type-add 'yaml:*.yml,*.yaml' -t yaml -A 5
Length of output: 753
libs/common-types/src/lib/con-profile/con-profile.mapper.ts (1)
79-80
: LGTM! Verify type consistency.
The mapping follows the established pattern and naming conventions.
Let's verify the type definitions in related files:
libs/data-access/src/lib/connect/my-con-profile/my-con-profile.generated.ts (2)
Line range hint 1-1
: Note: This is an auto-generated file
This file is automatically generated from GraphQL schema definitions. Direct modifications to this file should be avoided as they will be overwritten during the next generation.
13-13
: Verify GraphQL schema changes
The new isSubscribedToCONMarketingEmails
boolean field has been consistently added to all profile-related types. Let's verify that the corresponding GraphQL schema changes have been properly implemented.
Also applies to: 20-20, 25-25
✅ Verification successful
GraphQL Schema Verification Successful
The isSubscribedToCONMarketingEmails
field has been correctly added to all relevant profile-related types without any conflicting duplicates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify GraphQL schema changes for the new marketing emails field
# Search for GraphQL schema files
echo "Searching for GraphQL schema files..."
fd -e graphql -e gql
# Search for the new field in schema definitions
echo -e "\nVerifying field definition in schema..."
rg -l "isSubscribedToCONMarketingEmails"
# Search for any existing marketing email related fields to ensure no duplicates
echo -e "\nChecking for similar existing fields..."
rg -i "marketing.*email|email.*marketing|subscrib.*email|email.*subscrib"
Length of output: 42446
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (2)
191-191
: LGTM: Subscription filter properly added
The subscription filter is correctly added to ensure only subscribed users receive three-month-old mentorship match reminders.
118-118
: Verify consistent application of subscription filter
The subscription filter has been added here but similar filters might be needed in other reminder-related queries to maintain consistency.
Consider creating a reusable query builder or helper method to consistently apply subscription filters across all marketing email queries.
✅ Verification successful
Consistent application of subscription filter confirmed
All relevant queries have been reviewed, and the subscription filter is consistently applied.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other reminder-related queries that might need the subscription filter
ast-grep --pattern 'findAll({
$$$
Profile_Status__c: $_,
$$$
})'
Length of output: 72
Script:
#!/bin/bash
# Search for 'Subscribed_to_CON_Marketing_Emails__c' in both 'findAll' and 'find' query methods
ast-grep --pattern 'findAll({
$$$
Subscribed_to_CON_Marketing_Emails__c: $_,
$$$
})'
ast-grep --pattern 'find({
$$$
Subscribed_to_CON_Marketing_Emails__c: $_,
$$$
})'
Length of output: 182
schema.graphql (2)
120-120
: LGTM! Well-structured field definition.
The non-nullable boolean field is appropriately defined for tracking marketing email subscription status.
1454-1454
: LGTM! Verify the mutation implementation.
The nullable boolean field is correctly defined for partial profile updates.
Let's verify the mutation implementation:
✅ Verification successful
Implementation verified - field is handled through standard profile patch mutation
The field isSubscribedToCONMarketingEmails
is correctly implemented through the existing profile patch mutation mechanism:
- Field is properly defined in schema and types
- Updates are handled by ConProfilesService.patch() method
- Field is included in allowed patch fields
- Mapper correctly handles persistence
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the mutation implementation properly handles the new field
# Search for the mutation implementation
ast-grep --pattern 'class $_ {
$$$
async patchConProfile($_, $_) {
$$$
}
$$$
}'
# Search for field validation
rg -A 5 'isSubscribedToCONMarketingEmails.*validation'
Length of output: 150
Script:
#!/bin/bash
# Description: List all mutation methods in the codebase
ast-grep --pattern 'class $_ {
$$$
async $_($_, $_) {
$$$
}
$$$
}'
# Description: Search for any usage or validation of the `isSubscribedToCONMarketingEmails` field within mutations
rg 'isSubscribedToCONMarketingEmails' --context 5
Length of output: 477635
libs/data-access/src/lib/types/types.ts (2)
147-147
: LGTM! Field addition to ConProfile type
The new boolean field isSubscribedToCONMarketingEmails
is correctly typed as non-nullable, following GraphQL best practices for required fields.
1792-1792
: LGTM! Field addition to UpdateConProfileInput type
The new field isSubscribedToCONMarketingEmails
is correctly typed as an optional boolean using InputMaybe
, which is appropriate for update operations.
@@ -38,4 +38,5 @@ fragment AllConProfileFields on ConProfile { | |||
updatedAt | |||
userActivatedAt | |||
userType | |||
isSubscribedToCONMarketingEmails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Field isSubscribedToCONMarketingEmails
is not defined in the GraphQL schema. Please add it to the ConProfile
type to ensure proper functionality.
🔗 Analysis chain
Verify the field type in the GraphQL schema.
The field addition looks good and aligns with the PR objectives. However, let's ensure it's properly typed in the schema.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the field type in the GraphQL schema
# Expected: The field should be of Boolean type
# Search for the field type definition in schema files
fd -e graphql -e gql | xargs rg -A 5 "type.*ConProfile|input.*ConProfile"
Length of output: 2854
@@ -59,6 +59,7 @@ export class ConProfileEntityProps implements EntityProps { | |||
menteeCountCapacity?: number | |||
// userActivated?: boolean //! REINSTATE, COMPLEX CASE | |||
userActivatedAt?: Date | |||
isSubscribedToCONMarketingEmails: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add GraphQL decorator and documentation for the new field.
The new field needs the following improvements:
- Add
@Field()
decorator to expose it in GraphQL schema - Add default value of
true
as per requirements - Add JSDoc comment explaining its purpose
Apply this diff:
+ /**
+ * Controls whether the user receives marketing emails.
+ * Default is opt-in (true) with option to unsubscribe.
+ */
+ @Field((type) => Boolean)
- isSubscribedToCONMarketingEmails: boolean
+ isSubscribedToCONMarketingEmails: boolean = 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.
isSubscribedToCONMarketingEmails: boolean | |
/** | |
* Controls whether the user receives marketing emails. | |
* Default is opt-in (true) with option to unsubscribe. | |
*/ | |
@Field((type) => Boolean) | |
isSubscribedToCONMarketingEmails: boolean = true |
💡 Codebase verification
Field isSubscribedToCONMarketingEmails
is not fully implemented in GraphQL resolvers and database migrations.
- Ensure resolver functions handle
isSubscribedToCONMarketingEmails
. - Add necessary database migrations for the field.
🔗 Analysis chain
Verify the field implementation across the codebase.
Let's ensure this field is properly integrated with related components:
- GraphQL resolvers
- Database migrations
- Frontend components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of isSubscribedToCONMarketingEmails across the codebase
# Check GraphQL resolvers
echo "Checking GraphQL resolvers..."
rg -t ts "isSubscribedToCONMarketingEmails.*resolver"
# Check for database migrations
echo "Checking database migrations..."
fd -t f "migration" | rg -l "isSubscribedToCONMarketingEmails"
# Check frontend components using the field
echo "Checking frontend components..."
rg -t ts "isSubscribedToCONMarketingEmails.*component"
# Check for GraphQL queries/mutations
echo "Checking GraphQL operations..."
rg -t graphql "isSubscribedToCONMarketingEmails"
Length of output: 540
Script:
#!/bin/bash
# Description: Retry verifying the implementation of isSubscribedToCONMarketingEmails across the codebase without specifying file types
# Check GraphQL resolvers
echo "Rechecking GraphQL resolvers..."
rg "isSubscribedToCONMarketingEmails.*resolver"
# Check for database migrations
echo "Rechecking database migrations..."
fd -t f "migration" | rg -l "isSubscribedToCONMarketingEmails"
# Check frontend components using the field
echo "Rechecking frontend components..."
rg "isSubscribedToCONMarketingEmails.*component"
# Check for GraphQL queries/mutations without specifying file type
echo "Rechecking GraphQL operations..."
rg "isSubscribedToCONMarketingEmails"
Length of output: 13388
Script:
#!/bin/bash
# Description: Further verify the implementation of isSubscribedToCONMarketingEmails in GraphQL resolvers and database migrations
# Search for resolver implementations handling isSubscribedToCONMarketingEmails
echo "Searching for GraphQL resolver implementations for isSubscribedToCONMarketingEmails..."
ast-grep 'resolver Mutation { $$$ loadConProfile { $$$ isSubscribedToCONMarketingEmails $$$ } $$$ }' --lang ts
# Alternatively, search for resolver functions that include isSubscribedToCONMarketingEmails
echo "Searching for resolver functions related to isSubscribedToCONMarketingEmails..."
rg "isSubscribedToCONMarketingEmails\s*:"
# Search for database migration files that add or modify isSubscribedToCONMarketingEmails
echo "Searching for database migrations involving isSubscribedToCONMarketingEmails..."
rg -i "isSubscribedToCONMarketingEmails" $(fd -t f "migration" | grep -E "\.(ts|js)$")
# Verify modifications in ORM or database schema definitions
echo "Searching for ORM schema definitions involving isSubscribedToCONMarketingEmails..."
rg "isSubscribedToCONMarketingEmails" $(fd -t f "schema" | grep -E "\.(ts|js)$")
Length of output: 26078
const onSubscribeToMarketingEmailsChange = async () => { | ||
const mutationResult = await patchMyProfileMutation.mutateAsync({ | ||
input: { | ||
isSubscribedToCONMarketingEmails: | ||
!conProfile?.isSubscribedToCONMarketingEmails, | ||
}, | ||
}) | ||
queryClient.setQueryData( | ||
useLoadMyProfileQuery.getKey({ | ||
loopbackUserId, | ||
}), | ||
{ conProfile: mutationResult.patchConProfile } | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance mutation handling with loading states and user feedback
The subscription toggle handler needs additional error handling and user feedback mechanisms.
Consider these improvements:
const onSubscribeToMarketingEmailsChange = async () => {
+ try {
+ // Show loading state
+ setIsUpdating(true)
const mutationResult = await patchMyProfileMutation.mutateAsync({
input: {
isSubscribedToCONMarketingEmails:
!conProfile?.isSubscribedToCONMarketingEmails,
},
})
queryClient.setQueryData(
useLoadMyProfileQuery.getKey({
loopbackUserId,
}),
{ conProfile: mutationResult.patchConProfile }
)
+ // Show success message
+ showNotification('Subscription preferences updated successfully')
+ } catch (error) {
+ // Show error message
+ showErrorNotification('Failed to update subscription preferences')
+ } finally {
+ setIsUpdating(false)
+ }
}
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one comment
import { | ||
Button, | ||
Checkbox, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we rather be using Checkbox
from @talent-connect/shared-atomic-design-components
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using the Checkbox
from @talent-connect/shared-atomic-design-components
here 🤔
What Github issue does this PR relate to? Insert link.
Closes #884
What should the reviewer know?
This PR introduces the possibility of unsubscribing mentees & mentors from receiving ReDI Connect marketing content (from receiving the “Platform And New Mentors Reminder” and “Follow Up On The Long-Term Mentorship” reminders). By default, all users are subscribed to receive marketing content. To unsubscribe, they can uncheck the “Receive mentee/mentor updates” checkbox on their profile page on ReDI Connect, or the student manager can uncheck the exact checkbox in their ReDI Connect profile in Salesforce.
Salesforce changes:
Subscribed_to_CON_Marketing_Emails__c
to the ReDI Connect object onpartial
,contp
&production
OrgsCodebase changes:
isSubscribedToCONMarketingEmails
field to the related entities on the backendSubscribed_to_CON_Marketing_Emails__c
check for two marketing remindersTo do after deployment:
Screenshots
Mentor profile page:
Mentee profile page:
New field in the ReDI Connect profile on Salesforce:
Tooltips:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Documentation
These updates enhance user experience by providing more control over marketing communications and improving informational resources.