Skip to content

[NRT-897] Keep personally protected fields when resetting local changes - #1374

Merged
cacoze merged 4 commits into
mainfrom
NRT-897-reset-preserves-protected-fields
Aug 17, 2026
Merged

[NRT-897] Keep personally protected fields when resetting local changes#1374
cacoze merged 4 commits into
mainfrom
NRT-897-reset-preserves-protected-fields

Conversation

@cacoze

@cacoze cacoze commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Related issues

Proposed changes

Reset local changes was stripping AnkiHub_Protect::* tags before reimporting, so auto-protected edits (and the protect tag) were discarded. CS confirmed that protected fields should not be affected by Reset — same as a normal sync.

This only changes the two browser call sites (Reset local changes on notes, and reset-all-local-changes for a deck) to strip_personal_protect_tags=False. The strip flag stays so we can restore the old Reset behavior later without rewriting the reset path. No UI or copy changes.

How to reproduce

  1. Sync an AnkiHub deck. Confirm Automatically protect fields when edited is on.
  2. Edit a shared field, blur so auto-protect adds AnkiHub_Protect::<Field>.
  3. Right-click the note → Reset local changes.
  4. The edit and the protect tag should still be there.

Also check:

  • Unprotect a field, edit it, Reset → that field reverts; remaining protected fields stay.
  • Edit a globally protected field (e.g. Lecture Notes) → Reset leaves it.
  • Delete a note, Reset the deck → the note comes back.

To actually revert a protected edit: remove the protect tag, then Reset (or use Anki undo).

Screenshots and videos

N/A — no UI changes.

Further comments

NRT-769 made Reset strip personal protect tags so auto-protect wouldn't make Reset a no-op. CS uses Reset as a routine support step and confirmed protected fields should be left alone. Unprotected fields still reset.

Reset was stripping AnkiHub_Protect tags, so auto-protected edits were discarded. CS confirmed Reset should leave protected fields alone, matching sync.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cacoze
cacoze requested review from abdnh and ddevdan August 17, 2026 15:46
…otect_tags=False

Keep the strip flag so the old Reset behavior can be restored later without rewriting the reset path. Only the browser call sites change.

Co-authored-by: Cursor <cursoragent@cursor.com>

@ddevdan ddevdan 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.

2 findings

  • a (this PR) — no test pins False at either browser call site, though the db-check wiring is pinned (test_integration.py:6811-6829)
  • b (this PR) — test_reset_local_changes_to_notes:6724-6779 still passes True and asserts the old outcome

The previous tests still exercised the old True path, and nothing locked the two browser call sites this ticket actually changed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cacoze

cacoze commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both findings:

  • atest_browser_reset_does_not_strip_personal_protect_tags now pins strip_personal_protect_tags=False on both browser Reset entry points (selected notes and reset-all-to-a-deck).
  • btest_reset_local_changes_to_notes now calls with False and asserts the auto-protected Front edit (and its protect tag) survive.

@cacoze
cacoze requested a review from ddevdan August 17, 2026 17:22
abdnh
abdnh previously approved these changes Aug 17, 2026
Keep a test that still calls strip_personal_protect_tags=True, isolate globally protected Back from a personal tag, and cover AnkiHub_Protect::All.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cacoze

cacoze commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the latest review:

  • True coveragetest_reset_local_changes_to_notes_strips_personal_protect_tags (and ..._strips_protect_all_tag) call with True again, including that AnkiHub_Protect::All is not preserved.
  • Duplicate False tests — renamed to test_reset_local_changes_to_notes_keeps_personally_protected_fields. The main test now covers personal Front + global-only Back; the renamed one covers unprotected Front vs personal-only Back.
  • Global protection — main test no longer puts AnkiHub_Protect::Back on the note, so the surviving Back edit is from protected_fields only.
  • Protect::Alltest_reset_local_changes_to_notes_keeps_fields_when_protect_all_tag (False) and the strip variant (True).

@cacoze
cacoze merged commit 82f7cd0 into main Aug 17, 2026
6 of 7 checks passed
@cacoze
cacoze deleted the NRT-897-reset-preserves-protected-fields branch August 17, 2026 18:11
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.

3 participants