qualogy-solutions/sales-insights-dashboard

Read-only snapshot · generated 2026-09-30T06:49:10.884Z · act on PRs directly in Bitbucket · ← all repos · auto-merge is off for this repo — reviews still post automatically, merging is manual

🔺 Needs your review (1)

feat: replace Dupliceren with manual duplicate sets (QRAD-291)
#6 by Prijanka Ramcharan · QRA-291 → main · Open in Bitbucket →
medium

Replaces the "Dupliceren" (clone) feature with a manual duplicate-set linking mechanism, changing business logic and data flow for assignments (archiving/unlinking via is_active, superseded_by, alert_new_duplicate columns). Affects the Aanvragen list, detail pane, and pipeline view across the whole assignment workflow used by Sales, and reuses shared columns that also drive existing supersession/alert UI, so a bug could mis-archive or mis-link real aanvraag records.

  • Removes an existing feature (duplicateAssignment) and replaces with new bulk mutation logic touching shared DB columns (is_active, superseded_by, archived_at) used elsewhere in the app
  • Moderate-large diff (627+/329-) spanning 10 files including services, hooks, and multiple UI components
  • Business logic change affecting core Aanvragen workflow (archiving, unlinking, duplicate detection) with direct writes to production data via Supabase
  • Good test coverage included for new service functions, which reduces but doesn't eliminate risk
  • No schema migration or auth changes, keeping it out of 'high' territory, but affects a widely-used feature across list, detail, and pipeline views

Solid, well-documented change that replaces the "Dupliceren" clone feature with a manual duplicate-set linking mechanism, reusing existing supersession columns. Logic looks consistent with tests covering the new service functions. A few edge cases worth flagging, none blocking.

src/services/assignmentsService.ts:1534 · warning
markAssignmentsAsDuplicateSet does not validate that duplicateIds is non-empty. If called with an empty array, .in("id", []) will run an update with an empty match set; depending on the Supabase/PostgREST behavior this could be a no-op or could error, but it's worth guarding explicitly (and DuplicateSetDialog already prevents this via UI, but the service function itself is a public API used elsewhere).
src/services/assignmentsService.ts:1534 · warning
No transaction/atomicity between reading the primary's email_id and updating the duplicate rows. If another process changes the primary's email_id or deletes it between the two calls, the update could link duplicates to a stale/incorrect email_id. Low risk given single-user admin usage, but worth noting since this directly affects data integrity of the duplicate-set relationship.
src/components/aanvragen/AanvraagDuplicaten.tsx:68 · info
useLinkedDuplicateAssignments is called unconditionally on every aanvraag row rendered (e.g. in list contexts via useHasAanvraagDuplicaten), each with its own query key. This is fine functionally (react-query dedupes/caches), but for large lists this could mean many parallel network requests fired on mount when many rows want to know if they have duplicates. Worth confirming this scales for typical list sizes in Aanvragen.tsx and AssignmentPipelineView.tsx.
src/pages/Aanvragen.tsx:559 · info
selectedAssignments is recomputed via a filter on every render without memoization; fine given typical selection sizes, but could be wrapped in useMemo for consistency with other derived selectors in this file.
src/components/aanvragen/DuplicateSetDialog.tsx:63 · info
matches.length === 1 auto-selects a preferred broker as primary, but if the preferred-brokers query is still loading when the dialog opens (matches computed from empty preferredNames), primaryId will briefly be null then update once data resolves via the effect — this is handled by the effect re-running on `matches` change, but there's a brief flash where no broker looks preselected even when the query will eventually resolve to exactly one match. Not a bug, just a minor UX note.