fix: affiliations when work experiences are deleted #2675
Conversation
WalkthroughThe changes in the pull request modify the Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 0
🧹 Outside diff range and nitpick comments (2)
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts (2)
70-78: Consider more descriptive variable names.While the logic is correct, consider renaming variables for better clarity:
allMemberOrganizations→memberOrganizationsWithDeleteddeletedOrganizationIds→uniqueDeletedOrganizationIds- const allMemberOrganizations = await qx.select( + const memberOrganizationsWithDeleted = await qx.select( // ... ) - const validMemberOrganizations = allMemberOrganizations.filter(org => !org.deletedAt) + const validMemberOrganizations = memberOrganizationsWithDeleted.filter(org => !org.deletedAt) - const deletedOrganizationIds = _.uniq( + const uniqueDeletedOrganizationIds = _.uniq( - allMemberOrganizations + memberOrganizationsWithDeleted .filter(org => org.deletedAt) .map(org => org.organizationId) )🧰 Tools
🪛 GitHub Check: lint-format-services
[failure] 71-71:
Replaceorgwith(org)
[failure] 75-75:
Replace⏎······.filter(org·=>·org.deletedAt)⏎······.map(org·=>·org.organizationId)with.filter((org)·=>·org.deletedAt).map((org)·=>·org.organizationId),
81-89: LGTM: Proper handling of unaffiliation for deleted organizations.The implementation correctly creates conditions to unaffiliate activities associated with deleted organizations. Consider applying the suggested formatting for consistency:
..._.chain(deletedOrganizationIds) - .map(orgId => ({ + .map((orgId) => ({ when: [`'organizationId'::text = '${orgId}'`], - matches: (activity) => activity.organizationId === orgId, + matches: (activity) => activity.organizationId === orgId, orgId: null, })) .value(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts(5 hunks)
🧰 Additional context used
🪛 GitHub Check: lint-format-services
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts
[failure] 71-71:
Replace org with (org)
[failure] 75-75:
Replace ⏎······.filter(org·=>·org.deletedAt)⏎······.map(org·=>·org.organizationId) with .filter((org)·=>·org.deletedAt).map((org)·=>·org.organizationId),
🔇 Additional comments (3)
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts (3)
43-43: LGTM: Type modification enables proper handling of deleted organizations.
The change to allow null values for orgId in the Condition type is appropriate for supporting the unaffiliation of activities when organizations are deleted.
55-69: LGTM: Query modification properly retrieves deletion status.
The addition of deletedAt and createdAt columns enables proper filtering of organizations based on their deletion status while maintaining the existing ordering.
Line range hint 145-150: LGTM: Fallback organization logic properly handles valid organizations.
The change correctly uses filtered valid organizations for fallback selection. Let's verify that this change doesn't affect existing fallback behavior.
✅ Verification successful
Based on the search results, I can see that the fallback organization logic is unique to this specific file and there are no other implementations or direct dependencies on this logic in the codebase. The change from memberOrganizations to validMemberOrganizations is localized to this function and doesn't affect other parts of the codebase.
Let me verify if there are any other references to this fallback logic:
LGTM: Fallback organization logic is self-contained and correctly implemented.
The change from memberOrganizations to validMemberOrganizations is localized to this function, and the fallback selection logic is only used within this file. The implementation maintains the same filtering, sorting, and selection behavior, ensuring backward compatibility. The search results confirm that there are no other similar patterns or dependencies on this logic elsewhere in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the fallback organization selection behavior
# Test: Check for any direct usage of memberOrganizations for fallback logic
rg -A 5 "memberOrganizations.*fallback"
# Test: Check for any other implementations of fallback organization selection
ast-grep --pattern 'const fallbackOrganizationId = $_'
Length of output: 49292
Script:
#!/bin/bash
# Check for any references to fallbackOrganizationId
rg -A 3 "fallbackOrganizationId"
# Check for any similar patterns of organization selection
ast-grep --pattern 'chain($_)
.filter((row) => !row.dateStart && !row.dateEnd)
.sortBy($$$)
.map($$$)
.head()'
Length of output: 1407
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts
Outdated
Show resolved
Hide resolved
sausage-todd
left a comment
sausage-todd
left a comment
There was a problem hiding this comment.
Other than that, lgtm
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts(3 hunks)
🔇 Additional comments (1)
services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts (1)
43-43: Ensure all usages of Condition handle orgId as nullable
Changing the orgId property in the Condition type from string to string | null means that orgId can now be null. Verify that all parts of the code that use Condition are updated to handle a null orgId to prevent potential runtime errors.
Run the following script to identify all usages of Condition:
✅ Verification successful
Let me search for the actual usage of the Condition type and its implementation to verify how orgId is handled.
Let me modify the search to handle the special characters properly.
After analyzing the code, I can now provide the final response:
The nullable orgId is already properly handled throughout the codebase
The Condition type is only used locally within the runMemberAffiliationsUpdate function, and all its usages properly handle the nullable orgId:
- The
conditionfunction usesnullableOrghelper which explicitly handles null cases - The
insertIfMatchesfunction initializesactivity.organizationIdas null by default - The SQL query uses
COALESCEto handle null organization IDs - The
orgCasesarray construction properly passes the potentially nullorganizationIdvalues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of 'Condition' type to ensure they handle 'orgId' being nullable.
# Search for usages of 'Condition' type in the codebase.
rg 'Condition'
Length of output: 6438
Script:
#!/bin/bash
# Search for the implementation and usage of the Condition type specifically in the profiles_worker
rg -A 10 'type Condition = {' services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts
Length of output: 201
Script:
#!/bin/bash
# Search for the implementation and usage of the Condition type in profiles_worker
cat services/libs/data-access-layer/src/old/apps/profiles_worker/index.ts
Length of output: 7776
Changes proposed ✍️
What
copilot:summary
copilot:poem
Why
How
copilot:walkthrough
Checklist ✅
Feature,Improvement, orBug.Summary by CodeRabbit
New Features
Bug Fixes
Refactor