Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Fixes #10737: Fix Fix editing notes in "Conflicts" causes them to temporarily vanish #10913

Conversation

personalizedrefrigerator
Copy link
Collaborator

Summary

This pull request fixes an issue where conflict notes would disappear from the note list when edited.

This was caused by NOTE_UPDATE_ONE being dispatched with a note entity missing is_conflict. As a result, oldNote.is_conflict && !newNote.is_conflict was true. This caused the note to be removed from the conflict note list:

if (n.is_conflict && !modNote.is_conflict) {
// Note was a conflict but was moved outside of
// the conflict folder
newNotes.splice(i, 1);
noteFolderHasChanged = true;

Fixes #10737.

Testing plan

  1. Open a conflict note in the Markdown editor.
  2. Make a small change.
  3. Verify that the conflict doesn't disappear.

This has been tested successfully on Fedora 40 (Linux).

Comment on lines +87 to +115
it('should preserve value of is_conflict on save', async () => {
const testNote = await Note.save({ title: 'Test Note!', is_conflict: 1 });

const makeFormNoteProps = (): HookDependencies => {
return {
...defaultFormNoteProps,
noteId: testNote.id,
};
};

const formNote = renderHook(props => useFormNote(props), {
initialProps: makeFormNoteProps(),
});
await formNote.waitFor(() => {
expect(formNote.result.current.formNote).toMatchObject({
is_conflict: 1,
title: testNote.title,
});
});

// Should preserve is_conflict after save.
expect(await formNoteToNote(formNote.result.current.formNote)).toMatchObject({
is_conflict: 1,
deleted_time: 0,
title: testNote.title,
});

formNote.unmount();
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure how useful this automated test is — the bug is related to the interaction of useFormNote with reducer.ts. As such, an integrated/end-to-end test would make more sense. However, creating conflict notes in Playwright end-to-end tests is more complicated than creating conflicts in Jest tests.

@laurent22 laurent22 merged commit 3359932 into laurent22:dev Aug 22, 2024
10 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.

Desktop: Editing notes in the "Conflicts" folder causes them to temporarily disappear
2 participants