Skip to content

Make lane count optional and stop displaying it (PP-4506)#283

Merged
dbernstein merged 4 commits into
mainfrom
feature/PP-4506-lane-count-optional
Jun 24, 2026
Merged

Make lane count optional and stop displaying it (PP-4506)#283
dbernstein merged 4 commits into
mainfrom
feature/PP-4506-lane-count-optional

Conversation

@dbernstein

@dbernstein dbernstein commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Description

The Palace Manager backend is removing lane-size maintenance (see ThePalaceProject/circulation#3424), so the admin GET .../lanes response no longer includes a per-lane count.

  • interfaces.ts: make LaneData.count optional (count?: number) for backward compatibility with older servers.
  • Lane.tsx: stop rendering the " (count)" suffix next to the lane name.

count was display-only — no logic depended on it — so this degrades gracefully and is safe to ship independently of the backend change.

Motivation and Context

JIRA: PP-4506

Companion to ThePalaceProject/circulation#3424, which removes the lane size/count from the backend.

How Has This Been Tested?

  • tsc type-check passes.
  • tests/jest/components/Lane.test.tsx passes (2/2); Prettier clean.

@claude

claude Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @dbernstein's task in 1m 17s —— View job

Code Review — PP-4506 (lane count optional)

  • Gather context (read diff + changed files)
  • Check for other usages of LaneData.count
  • Review for correctness / maintainability
  • Post review

Summary

Clean, well-scoped change. Making LaneData.count optional and dropping the " (count)" suffix in Lane.tsx is exactly right: count was display-only, and I confirmed via a tree-wide search that no code reads lane.count anywhere else in src/ — the only consumer was the removed line, so this degrades gracefully against both old and new backends. The optional type keeps existing test fixtures and any older-server responses type-valid. The deprecation comment plus the TODO referencing ThePalaceProject/circulation#3424 addresses @tdilauro's review note. The test edits correctly drop only the now-obsolete count assertions while leaving the drag/expand/collapse coverage intact. No correctness, maintainability, or backward-compatibility concerns — nothing blocking.

@dbernstein dbernstein marked this pull request as draft June 18, 2026 22:16
@dbernstein dbernstein marked this pull request as ready for review June 19, 2026 19:08
@dbernstein dbernstein force-pushed the feature/PP-4506-lane-count-optional branch from ea52344 to f5b64c9 Compare June 22, 2026 13:49
tdilauro
tdilauro approved these changes Jun 24, 2026
View reviewed changes

@tdilauro tdilauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! 🔥

One comment below for adding a TODO for future clean up.

Comment thread src/interfaces.ts
// Deprecated: the backend no longer returns a lane count (lane sizes were
// removed). Kept optional for backward compatibility with older servers; the
// UI no longer displays it.
count?: number;

@tdilauro tdilauro Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: Add a TODO: about when / under what circumstances this optional and the deprecation comment can be removed.

@dbernstein dbernstein Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 700b8af — added a TODO noting that count and the deprecation comment can be removed once the backend changes in ThePalaceProject/circulation#3424 have been released to production.

@dbernstein dbernstein Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also create a follow up JIRA.

@dbernstein
Make lane count optional and stop displaying it (PP-4506)
a55572a 
The Palace Manager backend no longer returns a lane `count` field — lane
size maintenance was removed there. Mark `LaneData.count` optional (for
backward compatibility with older servers) and stop rendering the
"(count)" suffix next to the lane name in the Lane component.
@dbernstein
Drop lane-count assertions from the Lanes drag test
cd30604 
The "(count)" suffix is no longer rendered (lane count was removed), so the
"drags a top-level lane" test no longer asserts it — it still verifies the
post-drag lane order via the lane names.
@dbernstein
Drop lane-count assertions from the LanesSidebar render test
c76e069
@dbernstein dbernstein force-pushed the feature/PP-4506-lane-count-optional branch from f5b64c9 to c76e069 Compare June 24, 2026 16:42
@dbernstein @claude
Add TODO for removing deprecated lane count field (PP-4506)
700b8af 
Addresses review feedback: note that LaneData.count and its deprecation
comment can be removed once ThePalaceProject/circulation#3424 has been
released to production.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dbernstein dbernstein merged commit 1fbe3e2 into main Jun 24, 2026
5 checks passed
@dbernstein dbernstein deleted the feature/PP-4506-lane-count-optional branch June 24, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdilauro tdilauro tdilauro approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dbernstein @tdilauro