Skip to content

Migrate legacy Pairing workflow to Angular#74

Merged
nllong merged 6 commits into
mainfrom
migrate-pairing-workflow
Jul 23, 2026
Merged

Migrate legacy Pairing workflow to Angular#74
nllong merged 6 commits into
mainfrom
migrate-pairing-workflow

Conversation

@nllong

@nllong nllong commented Jul 23, 2026

Copy link
Copy Markdown
Member

What

Migrates the legacy AngularJS Pairing workflow (pairing_controller/pairing.html,
/data/pairing/:importfile_id/{properties|taxlots}) to this app, per MIGRATION.md's
"Not yet migrated" checklist.

Changes

  • New PairingComponent at route data/pairing/:id/:type:
    • Property / Tax Lot tab switcher (seed-page-inventory-tab), cycle selector (defaults to the
      import file's cycle), and a Show All / Show Paired / Show Unpaired filter on the left grid.
    • Two ag-grid grids side by side. Rows are dragged from the left grid and dropped onto a row
      in the right grid to pair them, using ag-grid Community's addRowDropZone /
      getRowDropZoneParams (no Enterprise license needed).
    • Right grid has a "Paired" column (pinned right) rendering chips for each currently-paired
      counterpart, with a per-chip unpair button.
  • PairingService.pairInventory added (mirrors the existing unpairInventory), calling the same
    /api/v3/{properties|taxlots}/{id}/pair/ endpoints the legacy app already uses.
  • dataset.component.ts's "Data Pairing" action now navigates to the new route instead of
    console.log.
  • datasets.routes.ts: added pairing/:id/:type.
  • ag-grid-modules.ts: registered the missing RowDragModule (required by the cross-grid drag
    APIs used here; not previously needed by any other page).

Design decision

The legacy app has a separate Pairing Settings page with its own localStorage-persisted
column visibility/order config. Rather than porting that as a second, parallel column-config
mechanism for a single page, this PR reuses the List View Profile column selector already used on
the Properties/Tax Lots list pages (each grid's columns follow the user's currently selected list
profile for that type). MIGRATION.md records this: Pairing workflow moved to "Already
migrated", Pairing settings moved to "Won't migrate" with the rationale above.

Validation

pnpm lint and pnpm build pass.

Additionally did a full manual/live pass: spun up the Django backend locally (docker), seeded a
test org/cycle with paired and unpaired properties/tax lots, and drove the page end-to-end with a
browser (Playwright) — sign-in, tab switching, cycle/show filters, drag-and-drop pairing, and
unpair-by-click. That pass caught and fixed several bugs that lint/build alone couldn't have:

  • RowDragModule not registered, so drag-and-drop silently threw and did nothing.
  • Paired-chip labels always showed #<id> instead of an address, because the live API suffixes
    field names with a unique column id (e.g. address_line_1_11) that a literal field-name lookup
    didn't account for.
  • The "Paired" column, unpinned, could scroll far out of view for orgs with many columns.
  • Switching the Property/Tax Lot tab updated the URL and tab highlight but not the actual grids or
    page title, because Angular reuses the same component instance across the sibling
    properties/taxlots routes and the type route param was only read once at construction.

All four are fixed in this PR and re-verified live afterward.

Screenshots

Hosted as release assets (not committed to the repo — safe to delete after this PR merges/closes).

Pairing page overview (Show All, one paired and two unpaired tax lots):

Pairing page overview

Right after a drag-and-drop pairing action (dragged a property row onto TL-300; new "333 Pine Rd" chip appears, unpair (✕) buttons on each paired chip):

Drag-and-drop pairing in action

"Show Unpaired" filter narrowing the left grid to only the remaining unpaired property:

Show Unpaired filter

Nicholas Long and others added 5 commits July 22, 2026 21:49
Ports the AngularJS pairing_controller/pairing.html page (property/tax-lot
pairing after an import) to a new datasets/pairing route.

- Add PairingComponent: dual ag-grid layout (Property/Tax Lot tabs, cycle
  select, Show All/Paired/Unpaired filter) using ag-grid Community's
  addRowDropZone/getRowDropZoneParams to drag-and-drop pair rows between
  grids, and a Paired chip column with per-item unpair buttons.
- Add PairingService.pairInventory (mirrors existing unpairInventory).
- Wire dataset.component.ts Data Pairing action to navigate to the new
  route instead of console.log.
- Add route data/pairing/:id/:type.
- Reuse the existing List View Profile column selector for grid columns
  instead of porting the legacy standalone Pairing Settings page and its
  own localStorage config (decision recorded in MIGRATION.md).
- Update MIGRATION.md: move Pairing workflow to migrated, move Pairing
  settings to Won't migrate with rationale.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- docs/porting-forms.md: add a canonical example for interactive
  dual-grid workspaces (no form/save flow) pointing at the new
  PairingComponent, plus a short recipe for ag-grid Community's
  cross-grid drag-and-drop (addRowDropZone/getRowDropZoneParams) so
  future agents don't have to rediscover the API. Also note the
  InventoryTabComponent tab-switcher precedent.
- .github/copilot-instructions.md: note that not every legacy page is a
  reactive-form CRUD screen, and record the general principle (used for
  Pairing settings) of reusing an existing app mechanism instead of
  porting a legacy page's bespoke config/settings 1:1, with the
  "Won't migrate" MIGRATION.md entry as precedent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Spun up the Django backend (docker), seeded test properties/tax lots
(paired and unpaired), and drove the pairing page with Playwright.
Found and fixed:

- ag-grid's RowDragModule was never registered in the app-wide
  ag-grid-modules.ts, so api.addRowDropZone/getRowDropZoneParams threw
  and drag-and-drop pairing was completely non-functional.
- pairedLabel() matched literal field names (e.g. "address_line_1"),
  but the tax_lot_properties/filter API suffixes field names with a
  unique column id (e.g. "address_line_1_11"), so chips always fell
  back to "#<id>" instead of showing an address.
- The "Paired" column was appended unpinned, so for organizations with
  many columns (the common case with no saved List View Profile) it
  was scrolled far out of view. Pinned it to the right.
- The component read the `type` route param once at construction, but
  Angular reuses the same component instance across the sibling
  properties/taxlots routes, so switching tabs updated the URL/tab
  highlight but never actually swapped the grids or page title.
  Now watches route paramMap changes and reloads on type change.
- Added the missing "Show" Transloco translation key.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codifies what caught 4 real bugs in the Pairing migration that lint+build
alone missed (a missing ag-grid module registration, wrong field-name
assumptions about a live API response, an unpinned column, and a
route-param staleness bug).

- Add docs/local-testing.md: a concrete recipe for standing up a
  throwaway Django backend (docker) with real seeded test data, and
  driving it with the Playwright MCP tools, including the gotchas hit
  along the way (docker-compose bind-mount pulling in a host
  local_untracked.py, uv not on PATH in the runtime image, the sample
  data command's missing faker dependency, ImportRecord.super_organization).
- .github/copilot-instructions.md: "Definition of done" now requires
  live-testing interactive/backend-driven changes against seeded data,
  not just lint + build; reference the new doc from the migration docs
  list.
- MIGRATION.md: playbook step 8 (Validate) now includes the same
  requirement.
- docs/porting-forms.md: Definition of done and PR checklist updated
  to include live-testing as a required step, not optional polish.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extends the live-testing workflow: after driving a page with Playwright,
capture 2-3 screenshots (overall page + the notable new interaction) and
publish them in the PR description, since a reviewer shouldn't have to
stand up a backend themselves just to see what changed.

- docs/local-testing.md: new "Put screenshots in the PR" step —
  commit PNGs under .github/pr-screenshots/<feature-slug>/ and embed
  via raw.githubusercontent.com links pinned to the commit SHA (gh/the
  API have no direct "attach image" equivalent to the web UI's
  drag-and-drop); clarifies these committed screenshots are the one
  exception to "don't commit throwaway verification artifacts".
- .github/copilot-instructions.md: Definition of done now mentions
  publishing screenshots alongside live-testing.
- docs/porting-forms.md: checklist gains a screenshots item.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nllong
nllong marked this pull request as ready for review July 23, 2026 02:53
@nllong
nllong force-pushed the migrate-pairing-workflow branch from 542f2ad to 6cea8dc Compare July 23, 2026 02:56
Corrects the approach added in the previous commit: committing screenshot
PNGs to the branch under .github/pr-screenshots/ still puts binary review
artifacts in the repo's permanent git history, which isn't wanted.

Gists were the other candidate but don't work — `gh gist create`/the Gist
API only accept text files ("binary file not supported" on a .png).

Use a dedicated, clearly-labeled prerelease (`gh release create --prerelease`)
as the asset host instead: it's a real public API, and release assets live
outside any branch's tree entirely, so nothing lands in git history. Docs
updated accordingly (docs/local-testing.md step 4, docs/porting-forms.md
checklist).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nllong
nllong merged commit 4594675 into main Jul 23, 2026
2 checks passed
@nllong
nllong deleted the migrate-pairing-workflow branch July 23, 2026 10:49
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.

1 participant