Skip to content

fix(categories): enable Save after switching category set (closes #955) - #965

Merged
ErikBjare merged 1 commit into
ActivityWatch:masterfrom
TimeToBuildBob:fix/category-set-dropdown-dirty-state
Sep 9, 2026
Merged

ErikBjare merged 1 commit into
ActivityWatch:masterfrom
TimeToBuildBob:fix/category-set-dropdown-dirty-state

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Problem

When switching between category sets in Categorization Settings, the Save button stays disabled and the selection change is never persisted. A page refresh silently reverts to the previous set.

Root cause: switchToSet() always set classes_unsaved_changes = false after switching, even though the new active_set_ids value hadn't been written to storage yet. The Save button is gated on classes_unsaved_changes, so it stayed disabled.

Fix

In src/stores/categories.ts, switchToSet() now:

  • Detects whether the active set actually changed (avoids spurious dirty state when re-selecting the same set or initializing)
  • Sets classes_unsaved_changes = true when the set changed, activating the Save button

Two regression tests added to test/unit/store/categories.test.node.ts:

  • Switching to a different set marks dirty ✓
  • Re-selecting the already-active set does not mark dirty ✓

Testing

npx jest test/unit/store/categories.test.node.ts  # 12/12 pass
npx jest test/unit/presetCategories.test.node.ts  # 34/34 pass
npx tsc --noEmit                                   # clean

Closes #955

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR fixes category-set persistence by marking the category store dirty when the active set actually changes, while leaving clean same-set selections unchanged.

  • Computes whether the selected category set differs from the current single active set.
  • Enables Save when the active-set selection changes.
  • Adds regression coverage for changed and unchanged selections.

Confidence Score: 5/5

The PR appears safe to merge, with the active-set persistence fix aligned with existing category-store save and discard flows.

The changed dirty-state assignment makes genuine set switches saveable, while initialization and current same-set caller paths do not expose a concrete regression.

Important Files Changed

Filename Overview
src/stores/categories.ts Updates switchToSet to preserve clean state for same-set selection and mark actual active-set changes for persistence.
test/unit/store/categories.test.node.ts Adds focused regression tests for switching to a different set and re-selecting the current set.

Reviews (1): Last reviewed commit: "fix(categories): switchToSet marks dirty..." | Re-trigger Greptile

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.69%. Comparing base (2809ee2) to head (e95fc56).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #965      +/-   ##
==========================================
+ Coverage   51.47%   51.69%   +0.22%     
==========================================
  Files          48       48              
  Lines        2889     2890       +1     
  Branches      669      637      -32     
==========================================
+ Hits         1487     1494       +7     
+ Misses       1382     1376       -6     
  Partials       20       20              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

The failed aw-server-rust master job was a CI cleanup failure, not a test failure: all 244 unit tests and 7 e2e tests passed, then both log steps errored because no files matched ~/.cache/activitywatch/log/*/*.log.

I pushed b8bb1d3, making those cleanup steps no-ops when logs are absent. The replacement run is now fully green (including all three test-matrix jobs, builds, lint, and CodeQL).

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Rebased onto current master and force-pushed 6bb8ff0. The only conflict was the PR's temporary CI log-cleanup commit versus the now-merged upstream fix in #962; I dropped the redundant PR commit and kept upstream's broader activitywatch* cache-path handling.

The category fix itself rebased cleanly. Verified locally: category-store tests pass (12/12), TypeScript checks clean, and git diff --check is clean. Fresh CI is starting on the rebased head.

@ErikBjare

Copy link
Copy Markdown
Member

@TimeToBuildBob Resolve conflict

…bling Save

Before this fix, selecting a different category set in the dropdown left
classes_unsaved_changes=false, so the Save button stayed disabled and the
active_set_ids change was never persisted to storage. A page refresh would
silently revert to the previous set.

Root cause: switchToSet() unconditionally set classes_unsaved_changes=false
even though the new active_set_ids hadn't been saved yet.

Fix: compute whether the set actually changed before syncing state and set
classes_unsaved_changes=true only in that case. Re-selecting the already-
active set does not mark dirty (no spurious save prompts on init).

Fixes: ActivityWatch#955
Git-Session-Id: fbbd
@TimeToBuildBob
TimeToBuildBob force-pushed the fix/category-set-dropdown-dirty-state branch from 6bb8ff0 to e95fc56 Compare September 3, 2026 09:27
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Conflict resolved — rebased onto master (which added the import syncs classes test in #956). Kept both test additions. CI running on new head e95fc56.

@ErikBjare
ErikBjare merged commit 2ddc69f into ActivityWatch:master Sep 9, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Category set dropdown doesn't apply selection or prompt save

2 participants