Skip to content

fix: correct role removal and merge strategy in moveRoles#4082

Merged
skwowet merged 8 commits intomainfrom
bugfix/mergeRoles
May 7, 2026
Merged

fix: correct role removal and merge strategy in moveRoles#4082
skwowet merged 8 commits intomainfrom
bugfix/mergeRoles

Conversation

@skwowet
Copy link
Copy Markdown
Collaborator

@skwowet skwowet commented May 7, 2026

What

Fixes a FK violation (memberOrganizationAffiliationOverrides_memberId_fkey) that caused finishMemberMerging workflows to fail on the final member deletion step, and fixes several related bugs in mergeRoles that were silently producing wrong merge results.

Why it broke

mergeRoles was never removing secondary memberOrganizations rows 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 its memberOrganizationAffiliationOverrides still pointing at the secondary member. DELETE FROM members then hit the FK.

Two other bugs made things worse:

  • worthMerging was 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.
  • The dated overlap filter hardcoded mo.memberId instead of using mergeStrat.intersectBasedOn(), so overlapping dated roles were never detected during member merges.

Changes

  • Every secondary role is now queued for removal regardless of path. removeMemberRole deletes overrides first (in a transaction), so nothing dangles.
  • worthMerging fixed: organizationId for member merge, memberId for org merge.
  • Overlap detection replaced with standard interval test (sStart < pEnd && pStart < sEnd) via getOverlaps, which also skips already-removed primaries to avoid stale-snapshot issues.
  • originalRoleIdoriginalRoleIds[] so overrides from multiple source roles are all tracked and correctly recreated.
  • isSameRole (renamed from isSamePrimaryRole) no longer compares title — matches the DB unique key exactly.
  • directSecondaryRole workaround removed — redundant now that all secondaries are properly removed and tracked via originalRoleIds.
  • Helpers (areDatesEqual, isSameRole, toTargetEntity, getOverlaps, queueRoleRemoval) moved inside mergeRoles since they're only used there.

Note

Medium Risk
Touches core merge/delete logic for memberOrganizations and 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 by organizationId and org merges merge by memberId, restoring the intended behavior for current-role merging.

Reworks mergeRoles to plan removals/additions across undated, current, and dated roles, including proper overlap detection for historical ranges and tracking merged sources via originalRoleIds[]. 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/isPrimaryWorkExperience based 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.

Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@skwowet skwowet self-assigned this May 7, 2026
Copilot AI review requested due to automatic review settings May 7, 2026 06:45
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

2 similar comments
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) with mergeStrat.intersectBasedOn() and tracks multiple originalRoleIds to recreate overrides correctly; removes the prior directSecondaryRole workaround.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 7, 2026 07:02
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread services/libs/data-access-layer/src/members/organizations.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Copilot AI review requested due to automatic review settings May 7, 2026 09:48
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread services/libs/data-access-layer/src/members/organizations.ts Outdated
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

@skwowet skwowet merged commit b5551f0 into main May 7, 2026
15 checks passed
@skwowet skwowet deleted the bugfix/mergeRoles branch May 7, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants