fix: correct role removal and merge strategy in moveRoles#4082
Conversation
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
2 similar comments
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
There was a problem hiding this comment.
Pull request overview
This PR fixes member/org role merge logic in mergeRoles() to prevent dangling memberOrganizationAffiliationOverrides from causing FK violations during member merge final deletion, and to correctly merge overlapping/current roles across merge strategies.
Changes:
- Fixes
worthMerging()comparisons for member-merge vs org-merge so current-role merges execute correctly. - Ensures every processed secondary role is queued for removal (and removes intersecting primaries when merging), preventing leftover rows/overrides.
- Replaces hardcoded overlap matching (
mo.memberId) withmergeStrat.intersectBasedOn()and tracks multipleoriginalRoleIdsto recreate overrides correctly; removes the priordirectSecondaryRoleworkaround.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dffefe4. Configure here.
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |

What
Fixes a FK violation (
memberOrganizationAffiliationOverrides_memberId_fkey) that causedfinishMemberMergingworkflows to fail on the final member deletion step, and fixes several related bugs inmergeRolesthat were silently producing wrong merge results.Why it broke
mergeRoleswas never removing secondarymemberOrganizationsrows in the fully-dated path. When the secondary had a matching role on the primary, the code merged the primary side but left the secondary row alive — along with itsmemberOrganizationAffiliationOverridesstill pointing at the secondary member.DELETE FROM membersthen hit the FK.Two other bugs made things worse:
worthMergingwas inverted in both strategies — comparing the entity field instead of the cross-entity field. This made the current-role merge path dead code for member merges; current roles were always duplicated instead of merged.mo.memberIdinstead of usingmergeStrat.intersectBasedOn(), so overlapping dated roles were never detected during member merges.Changes
removeMemberRoledeletes overrides first (in a transaction), so nothing dangles.worthMergingfixed:organizationIdfor member merge,memberIdfor org merge.sStart < pEnd && pStart < sEnd) viagetOverlaps, which also skips already-removed primaries to avoid stale-snapshot issues.originalRoleId→originalRoleIds[]so overrides from multiple source roles are all tracked and correctly recreated.isSameRole(renamed fromisSamePrimaryRole) no longer comparestitle— matches the DB unique key exactly.directSecondaryRoleworkaround removed — redundant now that all secondaries are properly removed and tracked viaoriginalRoleIds.areDatesEqual,isSameRole,toTargetEntity,getOverlaps,queueRoleRemoval) moved insidemergeRolessince they're only used there.Note
Medium Risk
Touches core merge/delete logic for
memberOrganizationsand affiliation overrides; mistakes could drop or mis-merge work experience history, but changes are localized and aimed at preventing FK violations and duplicate roles.Overview
Fixes merge strategy predicates (
worthMerging) so member merges merge byorganizationIdand org merges merge bymemberId, restoring the intended behavior for current-role merging.Reworks
mergeRolesto plan removals/additions across undated, current, and dated roles, including proper overlap detection for historical ranges and tracking merged sources viaoriginalRoleIds[]. Secondary roles (and any intersecting primaries being consolidated) are now consistently queued for deletion, preventing leftover rows/overrides that caused FK violations.Updates override recreation logic to reapply
allowAffiliation/isPrimaryWorkExperiencebased on all merged source roles, removes the prior direct-secondary workaround, and triggers affiliation recalculation when org-level blocks are dropped during the merge.Reviewed by Cursor Bugbot for commit e8094b6. Bugbot is set up for automated code reviews on this repo. Configure here.