Migrate legacy Pairing workflow to Angular#74
Merged
Conversation
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
marked this pull request as ready for review
July 23, 2026 02:53
nllong
force-pushed
the
migrate-pairing-workflow
branch
from
July 23, 2026 02:56
542f2ad to
6cea8dc
Compare
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Migrates the legacy AngularJS Pairing workflow (
pairing_controller/pairing.html,/data/pairing/:importfile_id/{properties|taxlots}) to this app, perMIGRATION.md's"Not yet migrated" checklist.
Changes
PairingComponentat routedata/pairing/:id/:type:seed-page-inventory-tab), cycle selector (defaults to theimport file's cycle), and a Show All / Show Paired / Show Unpaired filter on the left grid.
ag-gridgrids side by side. Rows are dragged from the left grid and dropped onto a rowin the right grid to pair them, using ag-grid Community's
addRowDropZone/getRowDropZoneParams(no Enterprise license needed).counterpart, with a per-chip unpair button.
PairingService.pairInventoryadded (mirrors the existingunpairInventory), 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 ofconsole.log.datasets.routes.ts: addedpairing/:id/:type.ag-grid-modules.ts: registered the missingRowDragModule(required by the cross-grid dragAPIs used here; not previously needed by any other page).
Design decision
The legacy app has a separate Pairing Settings page with its own
localStorage-persistedcolumn 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.mdrecords this: Pairing workflow moved to "Alreadymigrated", Pairing settings moved to "Won't migrate" with the rationale above.
Validation
pnpm lintandpnpm buildpass.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:
RowDragModulenot registered, so drag-and-drop silently threw and did nothing.#<id>instead of an address, because the live API suffixesfield names with a unique column id (e.g.
address_line_1_11) that a literal field-name lookupdidn't account for.
page title, because Angular reuses the same component instance across the sibling
properties/taxlotsroutes and thetyperoute 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):
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):
"Show Unpaired" filter narrowing the left grid to only the remaining unpaired property: