Fix Deactivate, Active checkbox, and add Deactivate/Shorten buttons to edit page by jantman · Pull Request #28 · jantman/workshop-inventory-tracking

jantman · 2026-04-26T13:39:29Z

Summary

Fixes Deactivate API and edit-page Active checkbox bugs caused by non-deterministic row selection when a JA ID has multiple rows sharing the same date_added (common with imported history). get_item_any_status and update_item now order by (active DESC, date_added DESC, id DESC) so the active row always wins, with id as a deterministic tiebreaker.
Adds Deactivate/Activate and Shorten buttons to the edit-page header, wired to the existing PATCH /api/inventory/<ja_id>/status endpoint with confirm-then-reload behavior.
Adds real-DB tests reproducing the tied-date scenario for get_item_any_status, the toggle API, and the edit page render, plus tests for the new edit-page buttons.

Test plan

All 243 existing unit tests still pass (nox -s tests -- tests/unit/)
New tests in TestMultiRowJaIdHandling and TestEditPageActions pass
Manual browser verification on JA000018 (a real item with the tied-date bug):
- Edit page shows Active checkbox correctly checked
- Clicking Deactivate from edit page deactivates the active row; checkbox unchecks; button label flips to "Activate"
- Clicking Activate restores the active row
e2e suite

🤖 Generated with Claude Code

… Deactivate/Shorten buttons When a JA ID had multiple rows sharing the same date_added (common with imported history rows), the lookup query order_by(desc(date_added)).first() was non-deterministic and could return an inactive sibling instead of the active row. This broke the Deactivate API (it would set active=False on an already-inactive row) and the edit page (the Active checkbox reflected whichever sibling the DB happened to return). - get_item_any_status and update_item now order by (active DESC, date_added DESC, id DESC), so the active row always wins and ties are broken deterministically by id. - Add Deactivate/Activate and Shorten buttons to the edit page header, wired to the existing PATCH /api/inventory/<ja_id>/status endpoint with confirm-then-reload behavior. - Add real-DB tests covering the tied-date scenario for get_item_any_status, the toggle API, and the edit page render, plus tests asserting the new edit-page action buttons render correctly for active and inactive items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes non-deterministic “canonical row” selection for multi-row JA IDs (common in imported history) so edit/status-toggle operations consistently target the intended row, and it adds edit-page header actions for status toggling and shortening.

Changes:

Make get_item_any_status() and update_item() deterministically select the canonical row via (active DESC, date_added DESC, id DESC).
Add Shorten and Deactivate/Activate controls to the inventory edit page, with a client-side PATCH call to the existing status endpoint.
Add unit tests reproducing the tied-date_added scenario and verifying edit-page action button rendering.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`app/mariadb_inventory_service.py`	Deterministic canonical-row ordering for multi-row JA IDs in read/update flows.
`app/templates/inventory/edit.html`	Adds Shorten + Activate/Deactivate UI actions and JS wiring to the status API.
`tests/unit/test_routes.py`	Adds regression tests for tied-date selection + edit-page action visibility.
`.claude/settings.local.json`	Adds local Claude permissions/settings content (appears developer-specific).

github-actions · 2026-04-26T13:46:26Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

github-actions · 2026-04-26T13:47:04Z

📊 Code Coverage Report

Coverage: 42.1%

✅ Coverage meets the minimum threshold of 1%

📁 Detailed Report: Check the coverage-reports artifact for full HTML report

🔍 Generated by: Unit tests (nox -s coverage)

- Disable the edit-page Shorten button for inactive items, since shorten_item only operates on the active row and would otherwise immediately fail with "Active item ... not found". - Update update_item docstring to reflect the new canonical-row selection (prefer active, then most recent by date_added/id). - Add a test asserting the Shorten control is rendered as a disabled <button> (not a link) on inactive items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Regenerated via `nox -s screenshots_headless` after adding the Deactivate/Activate and Shorten action buttons to the edit-item header. The other screenshot diffs are minor rendering differences from the same regeneration run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-26T15:12:12Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

Copilot

Pull request overview

Copilot reviewed 5 out of 10 changed files in this pull request and generated 2 comments.

Replace the inlined fetch/confirm/reload logic in the edit-page toggle handler with a delegation to window.toggleItemStatus, exposed via a small <script type="module"> shim that imports the shared helper from app/static/js/components/item-actions.js (the same module the inventory list page uses). This keeps the API call, error handling, and confirmation flow consistent across pages and avoids drift. Verified end-to-end in browser: deactivation/activation round trip works on JA000018 with no console errors, and the disabled Shorten button on inactive items still renders correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-26T15:58:44Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

Copilot

Pull request overview

Copilot reviewed 5 out of 10 changed files in this pull request and generated 1 comment.

The Activate/Deactivate button delegates to toggleItemStatus, which reloads the page on success and would silently discard any pending edits in the form. Lift the existing originalFormData snapshot and checkForUnsavedChanges helper from the duplicate-item block to a shared scope, and prompt the user with an explicit unsaved-changes warning before invoking toggleItemStatus when the form is dirty. Verified in browser: - Clean form: only the standard "Are you sure you want to ... item ..." confirm fires. - Dirty form, dismiss warning: aborts before any API call; form state preserved. - Dirty form, accept warning: proceeds to the standard confirm; accepting that completes the deactivation and reloads as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-26T19:49:40Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

Copilot

Pull request overview

Copilot reviewed 5 out of 10 changed files in this pull request and generated 3 comments.

…page Two issues raised in code review of the edit-page Activate/Deactivate flow: 1. When the form was dirty, the user saw two consecutive confirm dialogs for the same click: the unsaved-changes warning, then the built-in confirm inside toggleItemStatus. Add a {skipConfirm} option to toggleItemStatus so callers that have already prompted the user can opt out of the built-in confirm. The edit page now shows a single combined warning when dirty (covering both the unsaved-edits side effect and the action), and falls through to the standard confirm when the form is clean. The inventory list page is unchanged - it still uses the default confirm. 2. checkForUnsavedChanges only iterated keys present in the current FormData, so a field that disappeared from the submission - most notably an unchecked checkbox like active or precision - was invisible and the warning would not fire. Walk the union of original and current keys so removals are detected too. Browser-verified (JA000018) for all four cases: - clean form: single standard "Are you sure ..." confirm. - dirty via text edit: single combined "unsaved changes ..." confirm; accepting completes the action; dismissing aborts. - dirty via checkbox toggle (the regression for issue 2): single combined warning fires. - accept path with skipConfirm: only one dialog total, action proceeds and reloads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The previous name implied the method returns "any row regardless of status", but after the canonical-row fix the method actually has a deterministic preference order: it returns the active row when one exists, falling back to the most recent inactive row. Rename to get_canonical_item to reflect that contract: from the (possibly many) rows for a JA ID, return the one canonical representative. Expand the docstring to spell out when to use this versus get_item (which still filters out deactivated items entirely). Updated both callers in app/main/routes.py and all references in the unit tests, including test method names. The historical feature-design doc at docs/features/complete/active-item-bug.md is left as-is since it is an archive of what was done at the time and renaming there would falsify the record. Verified by running the full unit (244 passed) and e2e (281 passed, 1 skipped) suites via nox. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-26T22:01:20Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

Copilot

Pull request overview

Copilot reviewed 8 out of 13 changed files in this pull request and generated 3 comments.

Three small follow-ups from code review of the canonical-row work: - Replace the stale "Get item (any status) for updating" comment in api_toggle_item_status with one that describes the actual canonical-row selection (prefer active, otherwise latest inactive with deterministic tiebreak). - Replace the loose "Deactivate"/"Activate" body substring checks in the edit-page action tests with assertions against the #toggle-status-btn element specifically. The edit page's inline JS embeds both words in template literals, so the old assertions passed regardless of the rendered button label and would not have caught a regression that flipped active-state semantics. - Update the docstring of test_get_canonical_item_returns_most_recent_when_multiple, which still claimed to test the old "order_by(desc(date_added))" behavior, to reflect the new (active DESC, date_added DESC, id DESC) ordering and the active-row preference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 13 changed files in this pull request and generated 1 comment.

github-actions · 2026-04-26T23:04:01Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

Companion fix to the earlier checkForUnsavedChanges improvement. After that fix the unsaved-changes warning correctly fires when a user unchecks active or precision on the edit form, but the "Save changes and duplicate" code path then silently dropped the unchecked checkbox: the duplicate-save loop iterated only over new FormData(form).entries(), and unchecked checkboxes are omitted from FormData entirely. Client (app/templates/inventory/edit.html): Lift the form snapshot logic into a single buildFormSnapshot() helper that overlays explicit boolean values for every named checkbox on top of the FormData entries. checkForUnsavedChanges now uses this snapshot via a small diffFormSnapshots() helper, and the duplicate-save payload builder uses the same diff so the outgoing updated_fields includes a checkbox going off as {"precision": false} rather than nothing at all. Server (app/main/routes.py): setattr(source_item, "precision", "on") would store the literal string "on" on the in-memory ORM attribute - SQLAlchemy was coercing it to True at commit time, but only by luck. Add _normalize_duplicate_updated_fields() and _coerce_bool() so any known-boolean field in the duplicate-with-save payload (active, precision) is converted to a real Python bool before setattr. This handles JSON true/false from the new client, the legacy "on"/"off" form-encoded strings, and arbitrary truthy/falsy variants. Tests: - tests/unit/test_routes.py::TestDuplicateUpdatedFieldsBoolean - 4 cases covering the regression (off->on, on->off, the legacy "on" string, and the empty / "false" / "off" / "0" / "no" family) directly against the API endpoint. - tests/e2e/test_duplicate_item.py::test_duplicate_with_unchecked _checkbox_save_option - end-to-end browser test that loads the edit page, unchecks precision, opens the duplicate modal, asserts the unsaved-changes warning fires, accepts "save changes", clicks Create, and verifies both the source item and the new duplicate row come back with precision=False. Verified: 248 unit tests pass, 16 duplicate e2e tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-26T23:19:56Z

📸 Screenshot Update Reminder

This PR modifies UI files. Screenshot differences were detected when regenerating in CI.

Next Steps

If you haven't already, please regenerate and commit screenshots:

nox -s screenshots
git add docs/images/screenshots/
git commit -m "Update screenshots for UI changes"

Review

✅ Updated screenshots are available in the workflow artifacts for comparison

💡 Note: Minor differences in screenshots can occur due to rendering variations. The pre-commit hook will also remind you about screenshot updates.

This is an informational message and does not block merging.

Copilot

Pull request overview

Copilot reviewed 9 out of 14 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

app/main/routes.py:550

In the duplicate-with-save flow, updated_fields can now include active: false (because the edit form snapshot includes unchecked checkboxes). If the source item is deactivated here, the subsequent reload via service.get_item(ja_id) will return None (since get_item is active-only), and the rest of the duplication logic will crash when it tries to copy fields from source_item. Reload using get_canonical_item (or avoid reloading / reload conditionally based on whether active was changed) so deactivating via save-changes doesn’t break duplication.

                if hasattr(source_item, field):
                    setattr(source_item, field, value)
            service.update_item(source_item)
            # Reload the item to get fresh data
            source_item = service.get_item(ja_id)

jantman requested a review from Copilot April 26, 2026 13:40

Copilot started reviewing on behalf of jantman April 26, 2026 13:41 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread app/templates/inventory/edit.html

Comment thread app/mariadb_inventory_service.py

Comment thread .claude/settings.local.json

jantman and others added 2 commits April 26, 2026 10:59

jantman requested a review from Copilot April 26, 2026 15:10

Copilot started reviewing on behalf of jantman April 26, 2026 15:11 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread .claude/settings.local.json

Comment thread app/templates/inventory/edit.html

jantman requested a review from Copilot April 26, 2026 15:57

Copilot started reviewing on behalf of jantman April 26, 2026 15:57 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread app/templates/inventory/edit.html

jantman requested a review from Copilot April 26, 2026 19:53

Copilot started reviewing on behalf of jantman April 26, 2026 19:53 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread app/templates/inventory/edit.html Outdated

Comment thread app/mariadb_inventory_service.py Outdated

Comment thread app/templates/inventory/edit.html Outdated

jantman and others added 2 commits April 26, 2026 16:01

jantman requested a review from Copilot April 26, 2026 21:59

Copilot started reviewing on behalf of jantman April 26, 2026 21:59 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread app/main/routes.py Outdated

Comment thread tests/unit/test_routes.py

Comment thread tests/unit/test_mariadb_inventory_service.py Outdated

jantman requested a review from Copilot April 26, 2026 22:40

Copilot started reviewing on behalf of jantman April 26, 2026 22:40 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread app/templates/inventory/edit.html Outdated

jantman requested a review from Copilot April 26, 2026 23:17

Copilot started reviewing on behalf of jantman April 26, 2026 23:18 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

jantman merged commit e2446ac into main Apr 26, 2026
8 checks passed

jantman deleted the fix-deactivate-and-edit-page-buttons branch April 26, 2026 23:23

Conversation

jantman commented Apr 26, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

Review

Uh oh!

github-actions Bot commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Code Coverage Report

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

Review

Uh oh!

github-actions Bot commented Apr 26, 2026

📸 Screenshot Update Reminder

Next Steps

github-actions Bot commented Apr 26, 2026 •

edited

Loading