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,Mobile: Resolves #10073, #10080: Fix conflicts notebook doesn't work with the trash feature #10104

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Mar 11, 2024

Summary

This pull request improves how conflicts are handled with the trash feature.

Fixes #10080 and #10073.

Notes

This PR does the following:

  • Fixes a crash when displaying the conflicts folder.
  • Fixes deleting items in the conflicts folder.
    • Previously, clicking "delete" would move an item to the trash, but it would still be visible in the conflicts folder. Additionally, clicking "restore" on such items did nothing.
  • Fixes an issue where opening an item in the conflicts folder would throw a Missing property "deleted_time" exception.
  • Refactors getConflictFolderId and getTrashFolderId into separate files to prevent a circular import issue.

When a conflict is deleted to trash, it now

  1. Keeps the conflict as a conflict (is_conflict = 1)
  2. Marks the conflict as deleted (deleted_time ≠ 0)
  3. Shows the conflict in trash, but not in the "conflicts" folder

When a conflict is restored from trash, it is moved back to the conflicts folder.

Remaining issues

I've noticed the following remaining issues that could be addressed in a follow-up pull request:

  • Currently, dragging a conflict note out of trash always moves it to "Conflicts".
  • Editing a note in "Conflicts" causes the conflict note to disappear until another notebook is opened, then "Conflicts" is opened again.

Testing

To manually test this change,

  1. Launch Joplin Server.
  2. Connect to two different Joplin Server accounts.
  3. Share a notebook from one account to the other.
  4. Verify that it's still possible to open and edit a shared note.
  5. Verify that it's still possible to delete and restore a shared note.
  6. Create a conflict and verify that it can be deleted to trash.
  7. Verify that a conflict can be restored from trash back to the conflicts notebook.
  8. Verify that a conflict can be moved out of the conflicts notebook.

This has been tested successfully on Ubuntu 23.10.

On Android:

  1. Create a conflict.
  2. Verify that the application doesn't crash.
  3. Delete a note from the conflicts notebook.
  4. Verify that it is moved to the trash folder.

This has been tested successfully on Android 13.

Comment on lines 86 to +88
if (!needsShareReadOnlyChecks(itemType, changeSource, shareState)) return false;

checkObjectHasProperties(item, ['share_id']);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This check was moved from the top of the function to after needsShareReadOnlyChecks. This matches the behavior of the function prior to this commit.

This seems to be necessary to allow the new resource-related test to pass (see readOnly.test.ts).

@personalizedrefrigerator personalizedrefrigerator changed the title Desktop: Resolves #10073, #10080: Improve handling of conflicts when deleting Desktop: Resolves #10073, #10080: Fix conflicts notebook doesn't work with the trash feature Mar 11, 2024
@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Mar 11, 2024

Marking as a draft until the test failures have been resolved (this should be done in the next few hours).

Edit: Fixed.

@personalizedrefrigerator personalizedrefrigerator marked this pull request as draft March 11, 2024 20:53
@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review March 11, 2024 21:29
@personalizedrefrigerator personalizedrefrigerator changed the title Desktop: Resolves #10073, #10080: Fix conflicts notebook doesn't work with the trash feature Desktop,Mobile: Resolves #10073, #10080: Fix conflicts notebook doesn't work with the trash feature Mar 11, 2024
@laurent22 laurent22 merged commit c16ce1c into laurent22:dev Mar 14, 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: Resource links broken
2 participants