feat(tp): unsubscribe from marketing emails on TP #986
Conversation
WalkthroughThe changes introduced in this pull request primarily focus on enhancing the job seeker profile functionality by adding subscription management for marketing emails. A new field, Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (17)
apps/nestjs-api/src/tp-jobseeker-profile/dto/tp-jobseeker-profile-patch.entityinput.ts (2)
30-30: LGTM! Consider grouping related fields.The addition of
isSubscribedToTPMarketingEmailsis correct and aligns with the PR objectives. It allows jobseekers to manage their marketing email preferences as required.Consider grouping related fields together for better code organization. You might want to move
isSubscribedToTPMarketingEmailsnext to other communication-related fields if they exist, or create a comment section for subscription-related fields.
Line range hint
1-32: Consider improvements for handling temporary fields and code organization.The addition of
isSubscribedToTPMarketingEmailsis implemented correctly. However, there are a couple of suggestions for improving the overall code structure:
Temporary Fields: The commented-out job fair field suggests a need for a more flexible approach to handle temporary or event-specific fields. Consider implementing a dynamic system for managing these fields without requiring code changes.
Field Grouping: As the number of fields grows, it might be beneficial to group related fields together. This could improve readability and make it easier to manage the DTO in the future.
Here's a suggestion for grouping fields:
@InputType('TpJobseekerProfilePatchInput') export class TpJobseekerProfilePatchInput extends PartialType( PickType(_TpJobseekerProfileEntityProps, [ // Personal Information 'aboutYourself', 'profileAvatarImageS3Key', 'location', 'federalState', 'rediLocation', 'immigrationStatus', // Job Preferences 'availability', 'ifAvailabilityIsDate_date', 'desiredEmploymentType', 'desiredPositions', 'topSkills', 'willingToRelocate', // Profile Visibility 'isProfileVisibleToCompanies', 'state', // Communication Preferences 'isSubscribedToTPMarketingEmails', // Event Participation (consider a dynamic approach) // 'joinsMunich24SummerJobFair', ] as const) ) {}This grouping improves readability and makes it easier to manage related fields. Additionally, consider implementing a dynamic system for event participation fields to avoid frequent code changes.
libs/data-access/src/lib/tp/jobseeker-profiles/tp-jobseeker-profile.fragment.graphql (1)
Line range hint
1-48: Consider improving schema structure and naming conventionsWhile the addition of
isSubscribedToTPMarketingEmailsis correct, there are a few suggestions to improve the overall schema:
The commented Job Fair section suggests frequent schema modifications for specific events. Consider implementing a more flexible solution, such as a separate
jobFairsfield with an array of objects, to avoid frequent schema changes.The
ifAvailabilityIsDate_datefield name is inconsistent with the camelCase convention. Consider renaming it to improve readability and consistency.The fields in the fragment lack clear grouping or ordering. Consider organizing fields into logical groups (e.g., personal information, professional details, social media links) to improve maintainability as the schema grows.
Here's an example of how you might restructure part of the fragment:
fragment AllTpJobseekerDirectoryEntryFields on TpJobseekerDirectoryEntry { # Personal Information id firstName lastName fullName email telephoneNumber genderPronouns # Professional Details aboutYourself experience { ...AllTpJobseekerProfileExperienceRecordFields } education { ...AllTpJobseekerProfileEducationRecordFields } topSkills desiredPositions desiredEmploymentType # Availability and Location availability availabilityDate # Renamed from ifAvailabilityIsDate_date federalState location willingToRelocate # Platform Preferences isProfileVisibleToCompanies isSubscribedToTPMarketingEmails # Social Media and Web Presence githubUrl linkedInUrl personalWebsite behanceUrl stackOverflowUrl twitterUrl dribbbleUrl # Additional Information jobFairs { # New flexible structure for job fairs name year season isParticipating } # ... rest of the fields }This structure improves readability and makes it easier to maintain and extend the schema in the future.
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.entityprops.ts (1)
48-48: Approve addition with suggestions for improvementThe new
isSubscribedToTPMarketingEmailsproperty aligns well with the PR objectives. However, consider the following suggestions:
- Add the
@Field()decorator if this field needs to be exposed in the GraphQL schema for querying or mutation.- Include a JSDoc comment explaining the purpose and usage of this field.
- Consider setting a default value (e.g.,
true) to align with the PR objective of having users initially subscribed.Example implementation:
@Field(() => Boolean, { description: 'Indicates if the user is subscribed to Talent Pool marketing emails' }) isSubscribedToTPMarketingEmails: boolean = true;libs/shared-types/src/lib/TpJobseekerProfile.ts (1)
53-53: LGTM! Consider adding a JSDoc comment for clarity.The addition of
isSubscribedToTPMarketingEmailsis well-placed and correctly typed. It aligns with the PR objectives to allow jobseekers to manage their marketing email preferences.Consider adding a JSDoc comment to provide more context about this field. For example:
/** Indicates whether the jobseeker has opted in to receive marketing emails related to the Talent Pool. */ isSubscribedToTPMarketingEmails: booleanThis addition would improve code documentation and make the purpose of the field clearer to other developers.
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.entityprops.ts (1)
68-68: Consider adding @field() decorator to the new propertyThe new
isSubscribedToTPMarketingEmailsproperty aligns well with the PR objectives, allowing for management of marketing email subscriptions. However, I noticed that it's not decorated with@Field()like some other properties in this class.Consider adding the
@Field()decorator to expose this field in the GraphQL schema:+ @Field() isSubscribedToTPMarketingEmails: booleanThis would ensure consistency with other boolean fields in the class (e.g.,
isProfileVisibleToCompanies) and make the field queryable via GraphQL if needed.libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.recordprops.ts (1)
67-67: Approve with suggestions: New subscription propertyThe addition of the
Subscribed_to_TP_Marketing_Emails__cproperty aligns well with the PR objectives for managing marketing email subscriptions. However, consider the following suggestions to improve clarity and consistency:
- Add a comment explaining the purpose and usage of this property.
- Mark the property as optional using the
?operator, as it may not always be present in the data.- Consider renaming the property to follow the camelCase convention used in other properties, e.g.,
subscribedToTpMarketingEmails.Here's a suggested implementation incorporating these changes:
/** Indicates whether the job seeker has subscribed to marketing emails */ subscribedToTpMarketingEmails?: boolean;apps/redi-talent-pool/src/components/templates/LoggedIn.tsx (3)
49-49: Approve removal of console.log statementThe removal of the
console.logstatement is a good practice for production code. It eliminates unnecessary logging and potential security risks.For future debugging needs, consider using a configurable logging library that can be easily toggled between development and production environments.
Line range hint
1-124: Consider adding email preference checkbox to LoggedIn templateThe PR objectives mention adding a checkbox for managing email preferences. While this file doesn't contain changes related to this feature, consider if the
LoggedIntemplate would be an appropriate place to add this checkbox, especially if it should be consistently available across multiple pages.If you decide to add the checkbox here, ensure it's only displayed for jobseeker profiles and that it updates the
isSubscribedToTPMarketingEmailsfield.
Line range hint
1-124: Improve code quality and component structureWhile the overall code quality is good, consider the following improvements:
- Remove the commented-out Hotjar code if it's no longer needed.
- Improve type safety for the Hotjar integration. Consider creating a typed wrapper for Hotjar or using a declaration file.
- The
LoggedIncomponent has multiple responsibilities. Consider splitting it into smaller, more focused components for better maintainability.Here's an example of how you could improve the Hotjar integration:
// Create a typed wrapper for Hotjar declare global { interface Window { hj?: (method: string, id: string, data: object) => void; } } // Use the typed wrapper if (window.hj) { window.hj('identify', myTpData.data?.tpCurrentUserDataGet?.userContact?.id, { userType, }); }libs/data-access/src/lib/tp/jobseeker-directory-entries/tp-jobseeker-directory-entries-find-all-visible.generated.ts (1)
12-12: Approve changes with a suggestion for backward compatibilityThe addition of the
isSubscribedToTPMarketingEmailsfield to theTpJobseekerDirectoryEntriesFindAllVisibleQuerytype is correct and aligns with the PR objectives. This field will allow for the implementation of the unsubscribe feature for marketing emails.Consider making the field optional by adding a question mark after the field name:
-isSubscribedToTPMarketingEmails: boolean +isSubscribedToTPMarketingEmails?: booleanThis change would ensure backward compatibility with existing entries that might not have this field yet.
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.mapper.ts (2)
56-57: LGTM! Consider adding type assertion for improved type safety.The addition of the
isSubscribedToTPMarketingEmailsproperty is correct and aligns with the PR objectives. To enhance type safety, consider adding a type assertion:props.isSubscribedToTPMarketingEmails = raw.props.Subscribed_to_TP_Marketing_Emails__c as boolean;This ensures that the property is always of type boolean, which can help prevent potential type-related issues in the future.
Line range hint
1-108: Overall assessment: Changes are well-implemented and align with PR objectives.The additions to both
fromPersistenceandtoPersistencemethods are consistent and correctly implement the new marketing email subscription feature. The changes follow the existing patterns in the file and maintain code quality.To further improve the implementation:
- Consider adding type assertions for better type safety.
- Verify the consistent usage of the new property names across the codebase.
- Ensure that the new field is properly documented in the relevant interface or type definition files.
As this change introduces a new field that affects user preferences, consider implementing a migration strategy for existing users, if not already planned. This could involve setting a default value for existing profiles or notifying users to update their preferences.
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.mapper.ts (1)
95-96: LGTM! Consider adding type annotation for clarity.The addition of the
isSubscribedToTPMarketingEmailsproperty aligns well with the PR objectives and follows the existing code structure. Good job!For improved type safety and code clarity, consider adding a type annotation to the property:
props.isSubscribedToTPMarketingEmails = jobseekerProfileRecord.Subscribed_to_TP_Marketing_Emails__c as boolean;This ensures that the property is explicitly typed as a boolean, matching the expected type in the entity.
apps/redi-talent-pool/src/pages/app/jobseeker-profile/JobseekerProfileForJobseekerEyes.tsx (3)
48-56: LGTM! Consider adding error handling.The
onSubscribeToMarketingEmailsChangefunction correctly implements the toggle functionality for marketing email subscriptions, aligning with the PR objectives. The use ofqueryClient.invalidateQueries()ensures that the UI will reflect the updated state.Consider adding error handling to improve user experience:
const onSubscribeToMarketingEmailsChange = async () => { - await mutation.mutateAsync({ - input: { - isSubscribedToTPMarketingEmails: - !profile?.isSubscribedToTPMarketingEmails, - }, - }) - queryClient.invalidateQueries() + try { + await mutation.mutateAsync({ + input: { + isSubscribedToTPMarketingEmails: + !profile?.isSubscribedToTPMarketingEmails, + }, + }) + queryClient.invalidateQueries() + } catch (error) { + console.error('Failed to update subscription status:', error) + // TODO: Add user-friendly error notification + } }
143-148: LGTM! Consider adding a tooltip for clarity.The new checkbox for marketing email subscriptions is correctly implemented and aligns with the PR objectives. Its placement next to the existing "Hide my profile from companies" checkbox is logical and user-friendly.
Consider adding a tooltip to provide more context about the subscription:
<Checkbox checked={profile?.isSubscribedToTPMarketingEmails} customOnChange={onSubscribeToMarketingEmailsChange} + tooltip="Receive updates about new companies, job listings, and profile reminders" > Subscribe to marketing emails </Checkbox>This addition would improve clarity for users and align with the PR objective of informing users about the types of marketing emails they may receive.
Missing Unsubscribe Links in Email Templates
The search did not find any unsubscribe or opt-out links in the email templates. It's essential to include these to allow users to manage their email preferences effectively and comply with best practices.
Action Required:
- Implement unsubscribe functionality in all relevant email templates.
- Ensure that each marketing email contains a clear and accessible option for users to opt out of future communications.
🔗 Analysis chain
Line range hint
1-300: Overall implementation looks good and aligns with PR objectives.The changes to
JobseekerProfileForJobseekerEyes.tsxsuccessfully implement the functionality for managing marketing email subscriptions. The new checkbox and associated logic allow users to easily opt-in or opt-out of marketing emails, which directly addresses the objectives outlined in the PR summary and linked issue #883.Key points:
- The
onSubscribeToMarketingEmailsChangefunction correctly toggles the subscription status.- The new checkbox is logically placed and bound to the correct state and change handler.
- The implementation respects user preferences and allows for easy management of email settings.
To ensure full compliance with the PR objectives, please verify the following:
- Confirm that the sign-up process informs users about potential marketing emails.
- Verify that all marketing emails include an "Unsubscribe" button with a clear description.
Run the following script to check for the presence of unsubscribe links in email templates:
This will help ensure that all email templates include the necessary unsubscribe options as per the PR objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for email templates and check for unsubscribe links rg -i -g '*.html' -g '*.mjml' '(unsubscribe|opt.?out)'Length of output: 56
Script:
#!/bin/bash # Expanded search for unsubscribe links in various file types with additional keyword variations rg -i -g '*.html' -g '*.mjml' -g '*.tsx' -g '*.ts' -g '*.js' '(unsubscribe|opt\.?out|preferences|mailing\s?list)'Length of output: 16904
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
- apps/nestjs-api/src/tp-jobseeker-profile/dto/tp-jobseeker-profile-patch.entityinput.ts (1 hunks)
- apps/redi-talent-pool/src/components/organisms/jobseeker-profile-editables/EditableLanguages.tsx (0 hunks)
- apps/redi-talent-pool/src/components/templates/LoggedIn.tsx (1 hunks)
- apps/redi-talent-pool/src/pages/app/jobseeker-profile/JobseekerProfileForJobseekerEyes.tsx (2 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.entityprops.ts (1 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.mapper.ts (1 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.record.ts (1 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.recordprops.ts (1 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.entityprops.ts (1 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.mapper.ts (2 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.record.ts (1 hunks)
- libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.recordprops.ts (1 hunks)
- libs/data-access/src/lib/tp/jobseeker-directory-entries/tp-jobseeker-directory-entries-find-all-visible.generated.ts (1 hunks)
- libs/data-access/src/lib/tp/jobseeker-directory-entries/tp-jobseeker-directory-entries-find-one-visible.generated.ts (1 hunks)
- libs/data-access/src/lib/tp/jobseeker-profiles/tp-jobseeker-profile.fragment.generated.ts (2 hunks)
- libs/data-access/src/lib/tp/jobseeker-profiles/tp-jobseeker-profile.fragment.graphql (1 hunks)
- libs/data-access/src/lib/tp/my-tp-data/my-tp-data.generated.ts (1 hunks)
- libs/data-access/src/lib/types/types.ts (3 hunks)
- libs/shared-types/src/lib/TpJobseekerProfile.ts (1 hunks)
- schema.graphql (3 hunks)
💤 Files with no reviewable changes (1)
- apps/redi-talent-pool/src/components/organisms/jobseeker-profile-editables/EditableLanguages.tsx
🧰 Additional context used
🔇 Additional comments (21)
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.recordprops.ts (1)
31-31: New property added for marketing email subscription.The addition of
Subscribed_to_TP_Marketing_Emails__c: booleanaligns well with the PR objectives. It provides the necessary data structure to support the new feature allowing jobseekers to manage their marketing email preferences.A few observations:
- The property name follows the existing naming convention (PascalCase with underscores).
- The boolean type is appropriate for a subscription status.
- The property is not optional (no
?suffix), which implies it will always have a value. This is good for ensuring a default state.To ensure consistency, let's verify if this new field is used correctly in other parts of the codebase:
✅ Verification successful
To resolve the issue with the previous script, let's use the
--globoption to target.tsfiles directly:
Verification Successful: Marketing Email Subscription Property Confirmed
The
Subscribed_to_TP_Marketing_Emails__c: booleanproperty has been correctly added and is consistently utilized across the codebase. Its implementation aligns with the project's naming conventions and type requirements, ensuring seamless integration of the marketing email subscription feature.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of the new Subscribed_to_TP_Marketing_Emails__c field # Search for the new field name in TypeScript files echo "Searching for Subscribed_to_TP_Marketing_Emails__c usage:" rg --type typescript "Subscribed_to_TP_Marketing_Emails__c" -C 3 # Search for related UI components or form fields echo "\nSearching for related UI components:" rg --type typescript "isSubscribedToTPMarketingEmails|subscribedToTPMarketingEmails" -C 3Length of output: 438
Script:
#!/bin/bash # Description: Check for usage of the new Subscribed_to_TP_Marketing_Emails__c field # Search for the new field name in TypeScript files echo "Searching for Subscribed_to_TP_Marketing_Emails__c usage:" rg --glob '*.ts' "Subscribed_to_TP_Marketing_Emails__c" -C 3 # Search for related UI components or form fields echo "\nSearching for related UI components:" rg --glob '*.ts' "isSubscribedToTPMarketingEmails|subscribedToTPMarketingEmails" -C 3Length of output: 31296
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.record.ts (1)
38-38: LGTM! The new field has been correctly added.The addition of
Subscribed_to_TP_Marketing_Emails__cto theSALESFORCE_OBJECT_FIELDSarray is correct and aligns with the PR objectives. The field naming follows the established convention for Salesforce custom fields.Let's verify the consistency of this new field across the codebase:
✅ Verification successful
Verification Successful!
All occurrences of
Subscribed_to_TP_Marketing_Emails__cand its camelCase equivalentisSubscribedToTPMarketingEmailsare consistently used across the codebase. No inconsistencies or typos detected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of the new Salesforce field name # Test 1: Search for the exact field name echo "Occurrences of 'Subscribed_to_TP_Marketing_Emails__c':" rg --type typescript --type javascript "Subscribed_to_TP_Marketing_Emails__c" # Test 2: Search for similar field names (camelCase version often used in TypeScript) echo "\nOccurrences of 'isSubscribedToTPMarketingEmails':" rg --type typescript --type javascript "isSubscribedToTPMarketingEmails" # Test 3: Search for potential typos or inconsistencies echo "\nPotential typos or inconsistencies:" rg --type typescript --type javascript -i "subscrib.*marketing.*emails"Length of output: 658
Script:
#!/bin/bash # Description: Check for consistent usage of the new Salesforce field name # Test 1: Search for the exact field name in .ts and .js files echo "Occurrences of 'Subscribed_to_TP_Marketing_Emails__c':" rg "Subscribed_to_TP_Marketing_Emails__c" --glob "*.ts" --glob "*.js" # Test 2: Search for similar field names (camelCase version often used in TypeScript) echo "\nOccurrences of 'isSubscribedToTPMarketingEmails':" rg "isSubscribedToTPMarketingEmails" --glob "*.ts" --glob "*.js" # Test 3: Search for potential typos or inconsistencies in .ts and .js files echo "\nPotential typos or inconsistencies:" rg -i "subscrib.*marketing.*emails" --glob "*.ts" --glob "*.js"Length of output: 30271
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.entityprops.ts (1)
Line range hint
1-55: Verify related components for complete implementationThe addition of
isSubscribedToTPMarketingEmailsis well-placed and consistent with the existing code structure. To ensure complete implementation of this feature, please verify that corresponding updates have been made in related components such as:
- GraphQL resolvers (if the field is to be exposed in the API)
- Services handling user profile updates
- UI components for the profile page
- Any data migration scripts to set initial values for existing users
libs/shared-types/src/lib/TpJobseekerProfile.ts (1)
53-53: Verify usage of the new property across the codebaseThe addition of
isSubscribedToTPMarketingEmailsto theTpJobseekerProfiletype is correct. However, it's important to ensure that this new property is properly handled in all places whereTpJobseekerProfileis used.Let's verify the usage of this new property:
This script will help us identify:
- Where
TpJobseekerProfileis used, to ensure the new property is handled appropriately.- Where
isSubscribedToTPMarketingEmailsis explicitly used, to verify correct implementation.- Potential places where
TpJobseekerProfileobjects are created, to check if the new property is properly initialized.Please review the results to ensure the new property is consistently used and initialized throughout the codebase.
✅ Verification successful
Update shell script to target
.tsand.tsxfiles directlyThe previous script failed because
rgdoes not recognizetypescriptas a file type. Let's modify the script to search within.tsand.tsxfiles by specifying their extensions.[/run_scripts]
Correct the shell script to properly include
--globflagsThe previous script failed because the
--globpatterns were incorrectly quoted within theEXTENSIONSvariable. Let's update the script to pass each--globflag directly torg.[/run_scripts]
To further verify the usage of the new
isSubscribedToTPMarketingEmailsproperty, let's search for its direct usage in component files and any initializations of TpJobseekerProfile objects.
Let's modify our search approach to overcome the file type recognition issue. We'll use file glob patterns instead of the
--typeflag to target the relevant files.
Verification of
isSubscribedToTPMarketingEmailsimplementation is completeThe new
isSubscribedToTPMarketingEmailsproperty has been successfully implemented and integrated into the codebase. Key findings include:
- The property is correctly defined in the
TpJobseekerProfiletype and related interfaces.- It's included in GraphQL queries and mutations, ensuring proper data fetching and updating.
- The
JobseekerProfileForJobseekerEyescomponent directly uses the property for user interaction, allowing users to toggle their subscription status.- Mapper functions correctly handle the property when converting between different data representations.
- No issues were found with initialization or updates of this property.
The implementation appears to be thorough and consistent across the codebase. For future development, consider:
- Adding unit tests specifically for this property to ensure its behavior remains correct as the codebase evolves.
- Implementing analytics to track changes in subscription status, which could provide valuable insights for marketing efforts.
- Reviewing any email sending logic to ensure it respects this new property when determining whether to send marketing emails.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage and initialization of isSubscribedToTPMarketingEmails # Search for usage of TpJobseekerProfile echo "Searching for usage of TpJobseekerProfile:" rg --type typescript "TpJobseekerProfile" -C 5 # Search for usage of isSubscribedToTPMarketingEmails echo "\nSearching for usage of isSubscribedToTPMarketingEmails:" rg --type typescript "isSubscribedToTPMarketingEmails" -C 5 # Search for potential missing initializations in object literals echo "\nSearching for potential missing initializations:" rg --type typescript "TpJobseekerProfile\s*=\s*\{[^}]*\}" -C 5Length of output: 607
Script:
#!/bin/bash # Description: Check for usage and initialization of isSubscribedToTPMarketingEmails # Define file extensions to search EXTENSIONS="--glob '*.ts' --glob '*.tsx'" # Search for usage of TpJobseekerProfile echo "Searching for usage of TpJobseekerProfile:" rg "TpJobseekerProfile" $EXTENSIONS -C 5 # Search for usage of isSubscribedToTPMarketingEmails echo "\nSearching for usage of isSubscribedToTPMarketingEmails:" rg "isSubscribedToTPMarketingEmails" $EXTENSIONS -C 5 # Search for potential missing initializations in object literals echo "\nSearching for potential missing initializations:" rg "TpJobseekerProfile\s*=\s*\{[^}]*\}" $EXTENSIONS -C 5Length of output: 1086
Script:
#!/bin/bash # Description: Check for usage and initialization of isSubscribedToTPMarketingEmails # Search for usage of TpJobseekerProfile echo "Searching for usage of TpJobseekerProfile:" rg "TpJobseekerProfile" --glob '*.ts' --glob '*.tsx' -C 5 # Search for usage of isSubscribedToTPMarketingEmails echo "\nSearching for usage of isSubscribedToTPMarketingEmails:" rg "isSubscribedToTPMarketingEmails" --glob '*.ts' --glob '*.tsx' -C 5 # Search for potential missing initializations in object literals echo "\nSearching for potential missing initializations:" rg "TpJobseekerProfile\s*=\s*\{[^}]*\}" --glob '*.ts' --glob '*.tsx' -C 5Length of output: 441457
Script:
#!/bin/bash # Search for direct usage of isSubscribedToTPMarketingEmails in component files echo "Searching for direct usage of isSubscribedToTPMarketingEmails in component files:" rg "isSubscribedToTPMarketingEmails" --type tsx --type ts -C 3 # Search for TpJobseekerProfile initializations echo "\nSearching for TpJobseekerProfile initializations:" rg "TpJobseekerProfile\s*=\s*\{" --type tsx --type ts -C 5 # Search for profile patch or update functions echo "\nSearching for profile patch or update functions:" rg "patch.*TpJobseekerProfile|update.*TpJobseekerProfile" --type tsx --type ts -C 5Length of output: 690
Script:
#!/bin/bash # Search for direct usage of isSubscribedToTPMarketingEmails in component files echo "Searching for direct usage of isSubscribedToTPMarketingEmails in component files:" rg "isSubscribedToTPMarketingEmails" --glob '*.{ts,tsx,js,jsx}' -C 3 # Search for TpJobseekerProfile initializations echo "\nSearching for TpJobseekerProfile initializations:" rg "TpJobseekerProfile\s*=\s*\{" --glob '*.{ts,tsx,js,jsx}' -C 5 # Search for profile patch or update functions echo "\nSearching for profile patch or update functions:" rg "patch.*TpJobseekerProfile|update.*TpJobseekerProfile" --glob '*.{ts,tsx,js,jsx}' -C 5Length of output: 58305
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.entityprops.ts (2)
Line range hint
1-82: LGTM! The changes align well with the PR objectives.The addition of the
isSubscribedToTPMarketingEmailsproperty to theTpJobseekerDirectoryEntryEntityPropsclass is a good implementation of the subscription management feature described in the PR objectives. The change is minimal, focused, and doesn't introduce any disruptions to the existing code structure.Great job on implementing this feature! Once you've addressed the previous comments about adding the
@Field()decorator and verifying consistency across the codebase, this change will be ready for merging.
68-68: Verify consistency of the new field across the codebaseThe addition of
isSubscribedToTPMarketingEmailslooks good and is well-placed within the class. To ensure full implementation:
- Check if this field is properly handled in related components, especially those dealing with user profiles or email preferences.
- Verify if corresponding changes have been made to any database schemas or ORM configurations.
- Ensure that API endpoints or GraphQL resolvers are updated to handle this new field.
Run the following script to check for consistent implementation:
This script will help identify if the new field has been consistently implemented across different parts of the codebase.
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.record.ts (1)
103-103: LGTM! Verify consistency across the codebase.The addition of the
Subscribed_to_TP_Marketing_Emails__cfield to theJobseeker_Profiles__rchild object in theSALESFORCE_CHILD_OBJECTSmetadata is correct and aligns with the PR objectives. This change will allow tracking of the marketing email subscription status in Salesforce.To ensure consistency, let's verify if this new field is used appropriately in other parts of the codebase:
✅ Verification successful
Verified consistency across the codebase.
The
Subscribed_to_TP_Marketing_Emails__cfield has been successfully integrated and is consistently referenced across all relevant parts of the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of the new Subscribed_to_TP_Marketing_Emails__c field # Search for direct references to the new field echo "Searching for direct references to Subscribed_to_TP_Marketing_Emails__c:" rg --type typescript --type javascript "Subscribed_to_TP_Marketing_Emails__c" # Search for related terms that might indicate usage of the new feature echo "\nSearching for related terms (isSubscribedToTPMarketingEmails, subscribeToMarketingEmails, etc.):" rg --type typescript --type javascript -i "subscrib.*marketing.*emails"Length of output: 583
Script:
#!/bin/bash # Description: Check for usage of the new Subscribed_to_TP_Marketing_Emails__c field # Search for direct references to the new field echo "Searching for direct references to Subscribed_to_TP_Marketing_Emails__c:" rg --type ts --type js "Subscribed_to_TP_Marketing_Emails__c" # Search for related terms that might indicate usage of the new feature echo "\nSearching for related terms (isSubscribedToTPMarketingEmails, subscribeToMarketingEmails, etc.):" rg --type ts --type js -i "subscrib.*marketing.*emails"Length of output: 17281
libs/data-access/src/lib/tp/jobseeker-profiles/tp-jobseeker-profile.fragment.generated.ts (3)
Line range hint
1-1: Reminder: This is a generated file.The changes in this file correctly implement the new
isSubscribedToTPMarketingEmailsfield in both the type definition and the GraphQL fragment. These changes align with the PR objectives for introducing subscription management for marketing emails.Remember that this is a generated file, as indicated by the comment at the top. Any necessary changes should be made in the source of generation rather than directly in this file.
24-24: LGTM! Verify GraphQL schema on the backend.The addition of
isSubscribedToTPMarketingEmailsto the GraphQL fragment is correct and consistent with the type definition. This ensures that the field will be included in queries using this fragment.To ensure full compatibility, please verify that the GraphQL schema on the backend includes this field for the
TpJobseekerDirectoryEntrytype. Run the following command to check the schema:#!/bin/bash # Description: Check the GraphQL schema for the TpJobseekerDirectoryEntry type # Search for the TpJobseekerDirectoryEntry type definition in the schema echo "TpJobseekerDirectoryEntry type definition in the GraphQL schema:" rg --type graphql 'type TpJobseekerDirectoryEntry' -A 20If the field is missing in the backend schema, please update it accordingly.
7-7: LGTM! Verify consistency across the codebase.The addition of
isSubscribedToTPMarketingEmails: booleanto theAllTpJobseekerDirectoryEntryFieldsFragmenttype is correct and aligns with the PR objectives. The field name is clear and descriptive, and the boolean type is appropriate for a subscription status.To ensure consistency, let's verify that this field is used correctly in other parts of the codebase:
libs/data-access/src/lib/tp/jobseeker-directory-entries/tp-jobseeker-directory-entries-find-one-visible.generated.ts (1)
12-12: LGTM! Verify corresponding schema updates.The addition of the
isSubscribedToTPMarketingEmailsfield to theTpJobseekerDirectoryEntriesFindOneVisibleQuerytype is correct and aligns with the PR objectives. This boolean field will allow for managing user preferences regarding marketing emails.To ensure consistency, please verify that the corresponding GraphQL schema or data model has been updated to include this new field. Run the following command to check for the field in the schema:
This will help confirm that the change is reflected across the entire system.
✅ Verification successful
Verified: The
isSubscribedToTPMarketingEmailsfield is present in the GraphQL schema and TypeScript type definitions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the isSubscribedToTPMarketingEmails field is present in the GraphQL schema # Test: Search for the field in .graphql files rg --type graphql 'isSubscribedToTPMarketingEmails' # Test: Search for the field in TypeScript type definitions rg --type typescript 'isSubscribedToTPMarketingEmails.*:.*boolean'Length of output: 187
Script:
#!/bin/bash # Description: Check if the isSubscribedToTPMarketingEmails field is present in the GraphQL schema and TypeScript type definitions # Search for the field in .graphql files rg 'isSubscribedToTPMarketingEmails' --glob '*.graphql' # Search for the field in .ts and .tsx files rg 'isSubscribedToTPMarketingEmails.*:.*boolean' --glob '*.ts' --glob '*.tsx'Length of output: 13262
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-profile/tp-jobseeker-profile.mapper.ts (1)
95-96: LGTM! Verify property name consistency across the codebase.The addition of the
Subscribed_to_TP_Marketing_Emails__cproperty in thetoPersistencemethod is correct and maintains consistency with thefromPersistencemethod.To ensure consistency across the codebase, run the following script to verify the usage of the new property name:
This will help ensure that the property name is used consistently throughout the project.
✅ Verification successful
Update: Corrected shell script to verify property name consistency.
To address the previous error, please run the following revised script:
Property name consistency verified across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistent usage of the new property name across the codebase. # Test 1: Search for the entity property name echo "Searching for entity property 'isSubscribedToTPMarketingEmails':" rg --type typescript "isSubscribedToTPMarketingEmails" # Test 2: Search for the Salesforce field name echo "Searching for Salesforce field 'Subscribed_to_TP_Marketing_Emails__c':" rg --type typescript "Subscribed_to_TP_Marketing_Emails__c"Length of output: 486
Script:
#!/bin/bash # Description: Verify the consistent usage of the new property name across the codebase. # Test 1: Search for the entity property name in TypeScript files echo "Searching for entity property 'isSubscribedToTPMarketingEmails' in .ts and .tsx files:" rg "isSubscribedToTPMarketingEmails" --glob "*.ts" --glob "*.tsx" # Test 2: Search for the Salesforce field name in TypeScript files echo "Searching for Salesforce field 'Subscribed_to_TP_Marketing_Emails__c' in .ts and .tsx files:" rg "Subscribed_to_TP_Marketing_Emails__c" --glob "*.ts" --glob "*.tsx"Length of output: 15827
libs/data-access/src/lib/tp/my-tp-data/my-tp-data.generated.ts (1)
14-14: LGTM! Verify GraphQL schema update.The addition of the
isSubscribedToTPMarketingEmailsfield to thetpJobseekerDirectoryEntryobject is correct and aligns with the PR objectives. The boolean type is appropriate for representing the subscription status.As this is a generated file, please ensure that the corresponding GraphQL schema has been updated to include this new field. Run the following command to check the schema:
This command should return a match if the schema has been properly updated.
✅ Verification successful
GraphQL Schema Update Verified
The
isSubscribedToTPMarketingEmailsfield has been successfully added to the GraphQL schema inschema.graphql. This update aligns with the PR objectives and ensures that the TypeScript definitions are accurate.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify GraphQL schema update for isSubscribedToTPMarketingEmails field # Test: Search for the field in the GraphQL schema file rg --type graphql 'isSubscribedToTPMarketingEmails: Boolean!' schema.graphqlLength of output: 110
Script:
#!/bin/bash # Description: Verify GraphQL schema update for isSubscribedToTPMarketingEmails field # Find all .graphql files and search for the specific field fd -e graphql | xargs rg 'isSubscribedToTPMarketingEmails: Boolean!'Length of output: 188
libs/common-types/src/lib/tp-jobseeker/tp-jobseeker-directory-entries/tp-jobseeker-directory-entry.mapper.ts (1)
Line range hint
1-186: Overall implementation looks good.The changes to add the
isSubscribedToTPMarketingEmailsproperty are well-integrated into the existingTpJobseekerDirectoryEntryMapperclass. The implementation correctly maps the new Salesforce field to the entity property, aligning with the PR objectives.A few observations:
- The change is minimal and focused, reducing the risk of introducing bugs.
- The naming convention is consistent with the rest of the codebase.
- The new property is added in a logical place within the
fromPersistencemethod.To ensure consistency across the codebase, let's verify if this new field is properly handled in other related files:
This will help us identify if there are any other places in the codebase where this new field needs to be addressed.
✅ Verification successful
Overall implementation looks good.
The
isSubscribedToTPMarketingEmailsproperty is consistently integrated across the codebase. All relevant files, including entity definitions, mappers, GraphQL queries, and UI components, correctly handle the new subscription field. This ensures seamless functionality and maintains codebase integrity.
- The property is present in:
- Entity and record definitions (
TpJobseekerProfile.ts,types.ts)- Mapper files (
tp-jobseeker-profile.mapper.ts,tp-jobseeker-directory-entry.mapper.ts)- Generated GraphQL fragments and queries
- Frontend components (
JobseekerProfileForJobseekerEyes.tsx)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of isSubscribedToTPMarketingEmails or Subscribed_to_TP_Marketing_Emails__c rg -i -t typescript -t javascript "(isSubscribedToTPMarketingEmails|Subscribed_to_TP_Marketing_Emails__c)"Length of output: 143
Script:
#!/bin/bash # Search for other occurrences of isSubscribedToTPMarketingEmails or Subscribed_to_TP_Marketing_Emails__c rg -i -t ts -t js "(isSubscribedToTPMarketingEmails|Subscribed_to_TP_Marketing_Emails__c)"Length of output: 15393
schema.graphql (3)
1103-1103: LGTM: New field aligns with PR objectivesThe addition of the
isSubscribedToTPMarketingEmails: Boolean!field to theTpJobseekerDirectoryEntrytype is a good implementation. It directly supports the PR objective of allowing jobseekers to manage their marketing email preferences. The non-nullable nature ensures that every jobseeker has a defined subscription status.
1164-1164: LGTM: Consistent implementation in TpJobseekerProfileThe addition of the
isSubscribedToTPMarketingEmails: Boolean!field to theTpJobseekerProfiletype is consistent with the change made to theTpJobseekerDirectoryEntrytype. This ensures that the subscription status is available in both the directory entry and the jobseeker's profile, maintaining data consistency across the schema.
1103-1103: Verify if mutations/queries need updates for the new fieldThe addition of the
isSubscribedToTPMarketingEmailsfield to bothTpJobseekerDirectoryEntryandTpJobseekerProfiletypes is a good start. However, to fully implement the feature described in the PR objectives, we might need to update or add mutations to allow users to change this preference.Consider the following:
- Do we need a new mutation to update the
isSubscribedToTPMarketingEmailsfield?- Should existing profile update mutations be modified to include this field?
- Are there any queries that need to be updated to retrieve this information?
To check if any existing mutations or queries might need updates, you can run the following script:
This will help identify any areas of the schema that might need to be updated to fully support the new field.
Also applies to: 1164-1164
✅ Verification successful
Mutations and Queries Properly Updated for
isSubscribedToTPMarketingEmailsThe
isSubscribedToTPMarketingEmailsfield has been successfully integrated into relevant mutations and queries, ensuring full support and functionality for this new field.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing mutations or queries related to jobseeker profiles echo "Searching for relevant mutations and queries:" rg --type graphql -i '(mutation|query).*(?i)(jobseeker|profile|directory)' echo "\nSearching for fields that might need the new isSubscribedToTPMarketingEmails field:" rg --type graphql -i '(input|type).*(?i)(jobseeker|profile|directory).*\{[^}]+\}'Length of output: 506
Script:
#!/bin/bash # Search for existing mutations or queries related to jobseeker profiles without specifying file type echo "Searching for relevant mutations and queries:" rg -i '(mutation|query).*(?i)(jobseeker|profile|directory)' echo "\nSearching for fields that might need the new isSubscribedToTPMarketingEmails field:" rg -i '(input|type).*(?i)(jobseeker|profile|directory).*\{[^}]+\}'Length of output: 193221
libs/data-access/src/lib/types/types.ts (4)
1436-1436: LGTM! This change aligns with the PR objectives.The addition of the
isSubscribedToTPMarketingEmailsproperty to theTpJobseekerDirectoryEntrytype is a good implementation. It's a boolean type, which is appropriate for a subscription flag, and it's non-optional, ensuring that every jobseeker directory entry will have a defined subscription status. This change directly supports the feature of allowing jobseekers to unsubscribe from marketing emails.
1500-1500: LGTM! Consistent implementation across types.The addition of the
isSubscribedToTPMarketingEmailsproperty to theTpJobseekerProfiletype is consistent with the previous addition toTpJobseekerDirectoryEntry. This consistency is crucial for maintaining data integrity across different parts of the application. The boolean type and non-optional nature of the property ensure that every jobseeker profile will have a defined subscription status.
1657-1657: LGTM! Well-designed input type for profile updates.The addition of the optional
isSubscribedToTPMarketingEmailsproperty to theTpJobseekerProfilePatchInputtype is well-designed. Making this property optional (InputMaybe<Scalars['Boolean']>) in the patch input allows for partial updates to the profile, where the subscription status can be updated independently of other fields. This flexibility is beneficial for the frontend implementation and aligns well with GraphQL best practices for input types.
Line range hint
1436-1657: Overall, excellent implementation of the new feature.The additions to this file are well-structured and consistent, providing the necessary type definitions to support the new feature of allowing jobseekers to unsubscribe from marketing emails. The changes are focused and align perfectly with the PR objectives. The new
isSubscribedToTPMarketingEmailsproperty is appropriately added to bothTpJobseekerDirectoryEntryandTpJobseekerProfiletypes as non-optional boolean fields, ensuring data consistency. The optional inclusion in theTpJobseekerProfilePatchInputtype allows for flexible profile updates. These changes lay a solid foundation for implementing the unsubscribe functionality in the rest of the codebase.
libs/data-access/src/lib/tp/jobseeker-profiles/tp-jobseeker-profile.fragment.graphql
Show resolved
Hide resolved
|
Dear @katamatata , thanks a lot for the PR, looks awesome!
Thank you very much |
@astkhikatredi, since unsubscribing requires action from the user in our case, I wanted to make the message as straightforward as possible for that purpose. I went through many examples from other services in my email inbox and tailored this solution. If you don't like it, I can change it to the version from the comment (but without the button).
Oops, I forgot about that. I will push changes for that in a separate PR. |
No worries. let's keep as you proposed.
Lol, ok :) |
What Github issue does this PR relate to? Insert link.
Closes #883
What should the reviewer know?
This PR introduces the possibility to unsubscribe jobseekers from receiving ReDI Talent Pool marketing content (from receiving the "Update your profile", "New Companies", and "New Job Listings" reminders). By default, all jobseekers are subscribed to receive marketing content. To unsubscribe, the jobseeker can uncheck the "Subscribe to marketing emails" checkbox on their profile page on Talent Pool, or the student manager can uncheck the exact checkbox in the Jobseeker's profile in Salesforce.
Salesforce changes:
Subscribed_to_TP_Marketing_Emails__cto theJobseeker profileobject onpartial,contp&productionOrgspartialOrg, including the disclaimer about the possibility of unsubscribingSubscribed_to_TP_Marketing_Emails__ccondition onpartialOrgCodebase changes:
isSubscribedToTPMarketingEmailsfield to the related entities on the backendTo do after deployment:
productionOrgSubscribed_to_TP_Marketing_Emails__ccondition onproductionOrgScreenshots
Jobseeker's profile page:
New field in the Jobseeker's profile on Salesforce:
Updated email footer for three marketing reminders (Update your profile, New Companies, and New Job Listings):
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation