Skip to content

Conversation

@Ayush-Patel-56
Copy link
Contributor

@Ayush-Patel-56 Ayush-Patel-56 commented Jan 15, 2026

Fix #19225: Refresh queue on note edit

Issue

Fixes #19225

Problem

Editing a note or template in the new reviewer desyncs the backend queue state. Attempting to Undo afterwards trigger a bug: card modified without updating queue error and messes up the review counts.

Solution

Force updateCurrentCard() when OpChanges.noteText fires. This ensures we refresh the queue state from the backend whenever the card content changes.

Testing

Added ReviewerViewModelTest which reproduces the issue by simulating a template edit and verifying that the currentCard instance is correctly refreshed from the backend.

  • Manual Test: Verified no crash on Undo after editing a note template.
  • Unit Test: ReviewerViewModelTest.editing note template updates queue state preventing undo error passes.

@BrayanDSO
Copy link
Member

Editing a note or template in the new reviewer desyncs the backend queue state.

what do you mean by that?

@Ayush-Patel-56
Copy link
Contributor Author

When we edit the note, the card's mod time changes in the DB, but the scheduler's queue cache is still holding the old state. If we don't force refresh it, backend detects this mismatch (DB vs Cache) when we try to undo, it triggers the "card modified without updating queue" error.

@david-allison
Copy link
Member

A test would make this easier to review

@Ayush-Patel-56 Ayush-Patel-56 force-pushed the fix/19225-update-queue-on-note-edit branch from 0d22112 to c13d904 Compare January 16, 2026 18:56
@Ayush-Patel-56
Copy link
Contributor Author

Added the test case in ReviewerViewModelTest.kt. It verifies that the card state refreshes correctly after a template modification


@Before
fun setup() {
Dispatchers.setMain(StandardTestDispatcher())
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was the boilerplate, but now base class already handles dispatcher setup, so it's removed now.

val viewModel = ReviewerViewModel(handle)

advanceUntilIdle()
advanceRobolectricLooper()
Copy link
Member

Choose a reason for hiding this comment

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

Are these necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's not necessary as test uses a dispatcher that executes tasks immediately here.

val notetype = note.notetype
val tmpls = notetype.templates
val tmpl = tmpls.first()
tmpl.qfmt = tmpl.qfmt + " Modified"
Copy link
Member

Choose a reason for hiding this comment

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

+=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated it as you suggested in next push.

val newCard = viewModel.currentCard.await()
assertThat("Card should be refreshed (new instance)", newCard, not(sameInstance(card)))

viewModel.answerCard(Rating.GOOD)
Copy link
Member

Choose a reason for hiding this comment

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

You should probably be asserting something. If not: assertDoesNotThrow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, i added assertDoesNotThrow.

Comment on lines 43 to 48
val card = viewModel.currentCard.await()
val note = card.note()
val notetype = note.notetype
val tmpls = notetype.templates
val tmpl = tmpls.first()
tmpl.qfmt = tmpl.qfmt + " Modified"
Copy link
Member

Choose a reason for hiding this comment

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

Very verbose, how about:

            viewModel.currentCard.await().noteType().update {
                templates.first().qfmt += " Modified"
            }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I used noteType().update approach which is much cleaner. I kept the initial val card = as I'm using it for not(sameInstance(card)) assertion later to check that card instance actually refreshes or not.

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Jan 16, 2026
@Ayush-Patel-56 Ayush-Patel-56 force-pushed the fix/19225-update-queue-on-note-edit branch 3 times, most recently from 7bedddd to 2e9f1ea Compare January 17, 2026 06:04
Prevent 'card modified without updating queue' error by refreshing state when note text changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Author Reply Waiting for a reply from the original author Needs Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editing the template then undoing a review causes an off by one error

3 participants