Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Purchasing: supplier-alias-search (30/40 specs) - #66

Merged
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search
Aug 11, 2026
Merged

Purchasing: supplier-alias-search (30/40 specs)#66
corrin merged 3 commits into
mainfrom
purchasing-supplier-alias-search

Conversation

@corrin

@corrincorrin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • CompanyLookup gains a mode?: 'company' | 'supplier' prop, swapping its
    query source to the already-built, alias-aware
    purchasing_suppliers_search_retrieve endpoint. PoSummaryCard's
    create-mode supplier picker now uses it. SupplierSearchResult turned
    out to be an exact structural superset of CompanySearchResult, so no
    other prop or consumer needed to change.
  • CompanyDetailPage gains a third "Supplier Aliases" tab
    (SupplierAliasesPanel) for adding/listing/removing a company's search
    aliases, wired to the already-built alias CRUD endpoints.
  • Backend is untouched — SupplierSearchAlias, its CRUD API, and the
    alias-aware search endpoint already existed from an earlier session but
    were never wired to any UI. This slice is frontend-only.
  • New E2E spec frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts:
    quick-creates a supplier via live Xero push, adds an alias on the detail
    page, then confirms searching by the alias on PO create resolves to the
    canonical company name.
  • A pre-PR adversarial review (correctness + ADR-quality passes) found two
    low-severity gaps in SupplierAliasesPanel, fixed in the second commit:
    a double-click race on "remove alias" could show a false failure toast,
    and a failed alias-list fetch rendered nothing instead of an error/retry
    affordance.
  • docs/rewrite-status.md updated: specs-ported count regenerated via
    scripts/checks/status_table.py (29→30), "next in cluster" now points
    at pickup-address.

Verification

  • npm run type-check, pre-commit run --all-files --hook-stage pre-push
    (full expensive tier, whole repo): all green.
  • uv run pytest apps/company apps/purchasing: 356 passed, no regressions
    (backend code is unchanged).
  • ./scripts/ops/run_e2e.sh supplier-alias-search: passed once, cleanly,
    end to end
    (before the two review-driven fixes above).
  • Three subsequent local reruns (after the review fixes) all failed at the
    same pre-existing, unmodified-by-this-branch step — /api/companies/create/
    pushing a new contact to the live Xero demo tenant — with Xero-side auth
    errors (Xero ping reported connected=True in every run; the failure is
    specific to the contact-create call, not general connectivity). apps/xero
    and apps/company's push logic are untouched by this branch, and the very
    first run — before any of the review fixes — passed cleanly, so this looks
    like environmental flakiness on the live demo tenant (plausibly triggered by
    my own rapid repeated pushes) rather than a regression. CI does not run
    the Playwright/Xero E2E suite at all
    (checked .github/workflows/ci.yml
    — only lint/type/unit tests), so this doesn't block CI. Worth a clean rerun
    of ./scripts/ops/run_e2e.sh supplier-alias-search before merge, once the
    demo tenant has had time to settle.

Test plan

  • Full expensive pre-commit gate green
  • Scoped backend pytest green (no backend changes)
  • E2E spec confirmed green once end-to-end
  • Re-confirm E2E spec green after live-Xero flakiness clears (see note above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L

Summary by CodeRabbit

  • New Features
    • Added a Supplier Aliases tab to company details, allowing aliases to be added and removed.
    • Supplier searches now support alias matching during purchase order creation.
    • Improved supplier lookup suggestions and selection in purchasing workflows.
  • Bug Fixes
    • Added clearer loading, empty, error, retry, and notification states for supplier alias management.
  • Tests
    • Added end-to-end coverage for creating, managing, and searching suppliers by alias.

corrinand others added 2 commits August 11, 2026 16:32
CompanyLookup gains a supplier mode over purchasing_suppliers_search_retrieve
and CompanyDetailPage gains a Supplier Aliases tab, so staff-entered
nicknames let the PO-creation supplier picker find a company whose
paperwork/Xero name diverges from its canonical CRM name. The backend
(SupplierSearchAlias model, alias CRUD, the alias-aware search endpoint) was
already built and tested in an earlier session but never wired to any UI —
this slice is frontend-only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
… aliases
Adversarial pre-PR review found two low-severity gaps in SupplierAliasesPanel:
a double-click on remove could 404 on the second race and show a false
failure toast, and a failed alias-list fetch rendered nothing (no error
affordance, unlike the page's outer QueryState).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42a14941-9f0c-44a3-a78a-becd96b95cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 2213e31 and 5b12533.

📒 Files selected for processing (2)
  • docs/rewrite-status.md
  • frontend/src/features/crm/CompanyDetailPage.tsx
📝 Walkthrough

Walkthrough

The frontend now supports supplier aliases in CRM and supplier-mode lookup during purchase-order creation. It exports the required API helpers, adds alias CRUD states, validates the flow with an E2E test, and updates rewrite progress documentation.

Changes

Supplier alias search

Layer / File(s)Summary
Supplier and alias API exports
frontend/src/api/index.ts
The API barrel exports supplier search options and result types, plus supplier-alias query, mutation, query-key, and response helpers.
CRM supplier aliases
frontend/src/features/crm/CompanyDetailPage.tsx
The company detail page adds a Supplier Aliases tab. The panel lists aliases, adds trimmed values, removes aliases, invalidates lists after successful mutations, and renders loading, retry, empty, and error states.
Supplier-mode company lookup
frontend/src/features/shared/company/CompanyLookup.tsx, frontend/src/features/purchasing/PoSummaryCard.tsx
CompanyLookup supports company and supplier search modes. Purchase-order creation passes mode="supplier".
Alias search end-to-end validation
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts, docs/rewrite-status.md
The E2E test covers supplier creation, alias creation, synchronization, and canonical supplier selection. The rewrite status records completion and updates the green test count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompanyDetailPage
participant SupplierAliasAPI
participant CompanyLookup
User->>CompanyDetailPage: Add supplier alias
CompanyDetailPage->>SupplierAliasAPI: Create alias
SupplierAliasAPI-->>CompanyDetailPage: Return alias
User->>CompanyLookup: Search by alias in PO creation
CompanyLookup->>SupplierAliasAPI: Query supplier search
SupplierAliasAPI-->>CompanyLookup: Return canonical supplier
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the supplier-alias-search feature and indicates the related progress count.
Description check✅ PassedThe description explains the changes, verification results, test coverage, and known E2E limitation in sufficient detail.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch purchasing-supplier-alias-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove step narration from test comments.

Lines 27, 42, and 67 only restate the following test actions. Remove them.

If you retain the file-level comment, state the rejected alternative and the constraint. For example, state that direct API setup is rejected because this spec must validate the UI integration path.

As per coding guidelines, comments must document the rejected obvious alternative and factual constraint that rejected it; remove code narration and review-feedback echoes.

Also applies to: 27-27, 42-42, 67-67

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts` around lines 4 -
18, Remove the step-narrating comments near the test actions in the supplier
alias search spec, including the sections around the visible line references.
Retain or revise the file-level comment only to document that direct API setup
is intentionally rejected because this spec must validate the UI integration
path, including its required Xero-backed quick-create constraint; remove any
review-feedback echoes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rewrite-status.md`:
- Around line 17-28: Update the supplier-alias-search status in the
rewrite-status documentation so it is not listed as green while live Xero
contact creation remains unreliable. Mark it as blocked by the Xero
authentication dependency, and apply the same status correction to the
corresponding entry near the later referenced section.
In `@frontend/src/api/index.ts`:
- Line 159: Remove the code-narration comments at frontend/src/api/index.ts
lines 159-159 and frontend/src/features/crm/CompanyDetailPage.tsx lines 51-55;
do not replace them unless each comment documents a rejected obvious alternative
and the factual constraint that rules it out.
In `@frontend/src/features/crm/CompanyDetailPage.tsx`:
- Around line 68-92: Prevent duplicate synchronous submissions in handleAdd and
handleRemove by adding separate useRef in-flight locks, checking and setting
each lock before mutateAsync, and clearing it in finally blocks. Keep
createAlias.isPending and destroyAlias.isPending for button state, and add tests
covering duplicate add and remove events.
---
Nitpick comments:
In `@frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts`:
- Around line 4-18: Remove the step-narrating comments near the test actions in
the supplier alias search spec, including the sections around the visible line
references. Retain or revise the file-level comment only to document that direct
API setup is intentionally rejected because this spec must validate the UI
integration path, including its required Xero-backed quick-create constraint;
remove any review-feedback echoes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b2aa02-9704-4753-a65e-851f78f89086

📥 Commits

Reviewing files that changed from the base of the PR and between f7a8080 and 2213e31.

📒 Files selected for processing (6)
  • docs/rewrite-status.md
  • frontend/src/api/index.ts
  • frontend/src/features/crm/CompanyDetailPage.tsx
  • frontend/src/features/purchasing/PoSummaryCard.tsx
  • frontend/src/features/shared/company/CompanyLookup.tsx
  • frontend/tests/e2e/purchasing/supplier-alias-search.spec.ts

Comment threaddocs/rewrite-status.md Outdated
Comment threadfrontend/src/api/index.ts
Comment threadfrontend/src/features/crm/CompanyDetailPage.tsx
CompanyDetailPage: isPending is last-render state, so two clicks inside one
render frame could both dispatch a mutation before it flips true. Add
synchronous useRef in-flight locks for both alias handlers, checked and set
before mutateAsync and cleared in finally, keeping isPending for button
disabled-state only.
rewrite-status.md: CodeRabbit was right to flag the earlier "green" claim.
Root-caused via manage.py shell, outside the E2E harness entirely: this dev
environment's Xero connection 403s with AuthenticationUnsuccessful on the
first live API call in the create-company path (apps/xero/provider.py
search_contact_by_name -> get_contacts), despite a valid-looking token and
no rate-limit exhaustion. apps/xero is unmodified by this branch. Recorded
as an environment blocker, not code defect, and the spec's status corrected
to reflect it passed once but isn't yet confirmed reliable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
@corrin

Copy link
Copy Markdown
OwnerAuthor

Re the nitpick on supplier-alias-search.spec.ts (step-narration comments at lines 27/42/67/84-85): declining. The exact same step-comment style is the established convention across this project's existing E2E suite — e.g. create-purchase-order.spec.ts has // Create a job for PO line assignment testing, // Extract job number from the page, // Navigate to the created PO, // Open status dropdown, etc. ADR 0043 ("comments record the rejected alternative") targets application code, where the code itself is the whole behaviour and narrating it is pure noise; in a long sequential Playwright spec, these act as scannable step banners, matching how the rest of the test suite is already written. Rewriting just the new file would make it inconsistent with its own siblings, not more compliant.

Replied inline to the other three threads (rewrite-status.md, the two comment-narration findings, and the double-submit race) — summary in the latest commit message (5b12533).

@corrin
corrin merged commit be6f267 into mainAug 11, 2026
3 checks passed
Sign up for freeto 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

@corrin