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

Mobile: Support accepting Joplin shares #10300

Merged

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Apr 11, 2024

Summary

This pull request does two things:

  • Accepting Joplin server/cloud shares on mobile.
  • Adds tests for accepting server/cloud shares from the new mobile UI (commit 0c702e7).
    • This required a large amount of additional logic:
      • The share screen includes the ScreenHeader component, which is a connected Redux component. To allow testing this, a small amount of refactoring was required.
      • The mock initialization logic for ShareService was moved out of ShareService.test.tsx and into packages/lib/testing/share/mockShareService.ts to allow using it for tests for the new mobile UI.

Note

A large part of the changes made in this pull request are only related to tests. These changes are made in 0c702e7 and can be moved to a separate pull request or discarded on request.

Screenshots

iOS:

Android:

Testing plan

This pull request has automated tests, but they don't cover all cases. As such, this pull request has also been tested manually by:

  1. Setting up sync with Joplin server with two users (self@localhost and self2@localhost)
  2. Sharing two notebooks from self2@localhost to self@localhost
  3. On mobile, clicking on the "... would like to share" banner and accepting the shares
  4. Leaving one share from the same screen, cancelling the alert for the other
  5. Opening shared folder settings from "Configuration" > "Tools" > "Manage shared folders"
  6. Re-sharing one of the notebooks from self@localhost
  7. Refreshing on mobile by swiping from the top of the screen
  8. Rejecting the share on mobile.

This has been tested successfully on an Android 7 device. (More limited testing was done on iOS).

}, [invitation]);

const sharer = invitation.share.user;
if (!sharer) return <Text>Error: Share missing user</Text>; // Should not happen
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
if (!sharer) return <Text>Error: Share missing user</Text>; // Should not happen

This check might not be necessary. It was added because invitation.share.user is marked as optional in the type StateShare.

@@ -379,6 +380,7 @@ async function setupDatabase(id: number = null, options: any = null) {
await clearSettingFile(id);
await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy);

reg.setDb(databases_[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.

Without this change, reg.scheduleSync logs a warning related to a missing database during tests.

@laurent22 laurent22 merged commit ff86c25 into laurent22:dev Apr 15, 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.

2 participants