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

Nextcloud address book #1227

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

akhil1508
Copy link

@akhil1508 akhil1508 commented Aug 3, 2023

@akhil1508
Copy link
Author

akhil1508 commented Aug 3, 2023

@the-djmaze Perhaps we can have an option for admins like "Integrate nextcloud contacts fully"

  • If that option is ticked, we disable snappymail PDO contacts and suggestions
  • Along with that we disable the option in the right side menu

The UX will then be very similar to the attachments integration for nextcloud, where you can browse files in your cloud and attach them. Your contacts will be saved into your cloud and you can browse your contacts from your cloud app.

@cm-schl
Copy link

cm-schl commented Aug 4, 2023

Hi,
I'm very happy to see that you've intention to work on this as this would make SnappyMail a lot better when working together with Nextcloud 🙂.
I would be happy to help testing this - when you're at a good point just let me know.

@akhil1508
Copy link
Author

@the-djmaze What do you think about this change?

  • Suggestions come from the plugin for contact suggestions and are handled separately in the main-fabrica hook anyway so no changes there
  • Since IncFrec is the only function used to insert new contacts when an email is sent, I add logic only here to add/update into the cloud address book
  • I choose a uri 'webmail' by default; maybe in the future we can implement as an option for users the choice of address book to save too
  • To disable the contacts UI in snappymail, I add a new parameter called contactsExternal for frontend to understand if external provider or not and method called IsExternal for the address book to announce whether it is internal/external

@cm-schl

@akhil1508 akhil1508 changed the title Draft: Nextcloud address book Nextcloud address book Aug 31, 2023
@cm-schl
Copy link

cm-schl commented Aug 31, 2023

Hi,
sorry I tried to test you changes, but I don't get it to work on my test instance of Nextcloud. A question: what is the best way to get the changes to a working Nextcloud installation with SnappyMail installed? To patch the files manually seams wrong - there is surly a better way 🙂.

  • Disabling the default contacts UI seams ok to me, but somehow the administrator has to be informed about this in the Adminpanel -> Contacts. Otherwise we would create a situation where an Admin sets the flag to activate the contacts UI there and then the Nextcloud plugin overwrites this setting.

  • There should surly exist a option to let the user / admin choose where the contacts get saved. For example a company could use Thunderbird on their computers with for example the addon Cardbook. There you can choose an address book where new contacts should be saved. Therefore also SnappyMail in such a situation would have to use the same address book for this...

@the-djmaze
Copy link
Owner

I will come back later to this. 😉

@stepcellwolf
Copy link

I would love to have this as well. Any solution yet? Let me know if del is needed, testing, dev, anything.

@akhil1508
Copy link
Author

akhil1508 commented Sep 27, 2023

@stepcellwolf I do get back to you, got a bit busy in between.

I mostly need some dev to deal with the points raised by @cm-schl in #1227 (comment)

  • Mainly, let the admin know that this one overrides the main contacts UI and
  • Let the user choose the addressbook from the list of addressbooks in cloud

and ofc some testing..

@the-djmaze
Copy link
Owner

A good example of multiple address books is the people around you.
You ask a friend/colleague/etc. for someone his contact details, and they transfer the data to you and you put it in an address book.

Your phone only has "contacts" and you categorize them by any tag needed (family, friends, coworker, client, etc.)
The need for multiple address books is there, and there's not. Because some people are family & coworker or friend & client.

The biggest issue is GDPR where client data should not leave the building and yet, it is on your phone 😛

To support multiple address books, SnappyMail needs some big changes.
Currently i don't see the benefit vs all coding hours to get something nice.

Maybe someone already has a very bright idea?

@akhil1508
Copy link
Author

akhil1508 commented Oct 3, 2023

To support multiple address books, SnappyMail needs some big changes.
Currently i don't see the benefit vs all coding hours to get something nice.

@the-djmaze We can continue with a single address book but allow nextcloud admins to choose which one to activate(the SnappyMail one or full integration with nextcloud), wdyt?

I have already deployed a patched version if this PR to servers with a good amount of usage and this address book does over-ride the pdo address book of SnappyMail correctly.

@cm-schl
Copy link

cm-schl commented Oct 3, 2023

@the-djmaze We can continue with a single address book but allow nextcloud admins to choose which one to activate(the SnappyMail one or full integration with nextcloud), wdyt?

I too don't see it necessary to support multiple address books 👍.
Having the possibility to choose the destination of "collected addresses" would perfectly integrate into Nextcloud because there is also a automatically generated address book called "last contacted". As said for company use the option to choose the destination would be crucial when having different mail clients following the same logic (saving the new addresses into a specific address book).

Talking from a company perspective I would have to add something: for single users a simple dialog to choose the destination for new contacts would be enough. For an administrator it would be nice to have the possibility to preset this destination address book for all users.
Example: in Thunderbird you can preset this by MCD by a string like "https://<SERVER>/remote.php/dav/addressbooks/users/" + getenv("USERNAME") + "/<ADDRESS_BOOK_NAME>/".
It may be to much at the moment, but i definitely see a use case for larger installs 🙂. Maybe some provider would like to add SnappyMail to its Nextcloud for all users. Then a definition for a default destination would prevent to mess up the "normal" address book of the users with collected addresses...

Signed-off-by: Akhil <akhil@e.email>
@fahim44
Copy link

fahim44 commented Sep 3, 2024

@the-djmaze @akhil1508 this is ready to be reviewed.

@akhil1508
Copy link
Author

@the-djmaze So we have the following functionality now:

  1. Setting for user to choose their addressbook
  2. Settings for admins to set:
  • Default addressbook uri
  • Default Addressbook description
  • Disable inbuilt contacts UI so users can use nextcloud contacts
  • Enable NC addressbook so users' contacts can save to NC contacts
  1. Save contact to NC addressbook on send if it is not a new contact (no duplicates)

cc @fahim44


## Nextcloud Addressbook for recipients

This plugin can let user to choose which nextcloud addressbook to use save recipients. This is opt-in feature (enabled by admin). After admin enable this, user will find a dropdown in his/her SnappyMail's `Contacts` section, containing all his/her addressbook.
Copy link
Author

Choose a reason for hiding this comment

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

@fahim44 Could be nice to mention that it works best with Nextcloud Contacts where contacts app is used as the frontend for contacts(browsing, editing etc) and SnappyMail seamlessly integrates with it to provide email autocomplete suggestions when drafting emails (not in this PR) and save emails to Nextcloud Contacts upon sending.

Copy link

Choose a reason for hiding this comment

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

@akhil1508 updated the README

NC contacts app is just frontend for contacts & addressbook of NC
server. Nextcloud plugin `auto save recipients on addressbook` feature used to depends on contacts app directly. This commit update that logic.
@fahim44 fahim44 force-pushed the dev/sync-nextcloud-addressbook branch from 7223a19 to cf870df Compare September 5, 2024 10:11
@mstolf
Copy link

mstolf commented Sep 17, 2024

I'm looking forward to this feature

It will actually be very good, I would like to ask how it will be in the case of two or more accounts authenticated in Snappymail, will this create the contacts in the same addressbook?

Or will it save in each user's addressbook?

@fahim44
Copy link

fahim44 commented Sep 23, 2024

I'm looking forward to this feature

It will actually be very good, I would like to ask how it will be in the case of two or more accounts authenticated in Snappymail, will this create the contacts in the same addressbook?

Or will it save in each user's addressbook?

@mstolf the contacts will be saved in the nextcloud addressbook. By default, all contacts will be saved in the default addressbook defined by nextcloud admin. But, this feature support addressbook switching for each snappyMail account. So, if you have nextcloud account with multiple addressbooks, & multiple snappyMail account setup, you can pick separate addressbooks for each snappyMail account

@fahim44
Copy link

fahim44 commented Sep 23, 2024

screenshot:

aa

@cm-schl
Copy link

cm-schl commented Sep 23, 2024

Hi,
first of all thanks for your work on this!

As promised before some feedback from my first tests:

  1. I first did not understood that you have to activate the "contacts" in the SnappyMail configuration. Without this no contact gets saved even if inside the configuration of the Nextcloud plugin something is configured. I don't know if other users would fail in the same way or if its only me 🙂.
  2. defaultNCAddressbookUri and defaultNCAddressbookName: I was able to use the plugin, but to me first it wasn't really clear what to put there. Why defaultNCAddressbookUri and defaultNCAddressbookName is necessary? Wouldn't only defaultNCAddressbookUri be sufficient to identify the address book?
  3. Where does defaultNCAddressbookDescription gets written to when a contact is saved?
  4. disableSnappymailContactsUI: In my tests i did not saw a difference between activating and deactivating this... Only the original setting (see point 1) did had an impact on the ui...
  5. As admin I would prefer to set the address book where all addresses should get saved without giving the user to change my preset. At the moment, when I set defaultNCAddressbookUri and defaultNCAddressbookName and disable enableNcAddressbook the contact does not get saved... So at the moment it seams that enableNcAddressbook does not only disables the user from choosing the address book but disables the whole functionality. This wasn't clear to me because i didn't saw a connection between the various options. It would be nice if this settings would be grouped together to enable the user to understand that these are connected (and not usable independently)... But this maybe is only my preference 🙂.
    Example from my plugin (https://github.com/the-djmaze/snappymail/tree/master/plugins/ldap-mail-accounts):

grafik

@fahim44
Copy link

fahim44 commented Sep 24, 2024

Hi @cm-schl , let me try to answer your questions

  1. We want to save contacts into nextcloud addressbook. I think this make more sense to save contacts, we need to enable the contacts first. Else it makes more confusion & will feel like snappyMail not working.

2 & 3. This patch will try to create new nextcloud addressbook / reuse if already present (search against addressbook uri). This is because we need addressbook to save our contacts & admin can't be sure user has any specific / any addressbook present or not by default. User can change it later as per his preference. To create a nextcloud addressbook, we need uri, name, & description. defaultNCAddressbookUri, defaultNCAddressbookName, defaultNCAddressbookDescription serves those perposes.

aa

  1. disableSnappymailContactsUI disable snappyMail specific contact UI elements. This is present so admin can hide snappyMail contacts entry option to avoid confusion (as contacts will be automatically saved to nextcloud addressbook).
disableSnappymailContactsUI = false disableSnappymailContactsUI = true
aa1 bb1
aa2 bb2
  1. enableNcAddressbook is the main switch which will enable this feature. We want this feature to be optional, so admin can choose whatever he wants it or not. If you enable enableNcAddressbook, then defaultNCAddressbookUri & defaultNCAddressbookName, defaultNCAddressbookDescription will be taken as consideration to create/use nextcloud addressbook. disableSnappymailContactsUI is used to disable snappyMail inhouse contacts UI elements to avoid confusion.

@cm-schl
Copy link

cm-schl commented Sep 24, 2024

Hi @fahim44 and thanks for your response!
1 & 2 & 3 & 4: makes completely sense like you described it. Maybe this information could get added to the readme? (I like documentation 😉🙂).

5: like you and I said enableNcAddressbook disables everything. It may only be me but I first thought that the description Enable User to choose Nextcloud addressbook for recipients would let the admin choose if the user can choose the address book by himself or only the admin can do this by setting the fields below. Therefore a "Enable" checkbox or some grouping came into my mind. I thought that this maybe makes it clear that disabling the first option would disable everything and not only the possibility to let the user choose.

@fahim44
Copy link

fahim44 commented Sep 24, 2024

Hi @fahim44 and thanks for your response! 1 & 2 & 3 & 4: makes completely sense like you described it. Maybe this information could get added to the readme? (I like documentation 😉🙂).

5: like you and I said enableNcAddressbook disables everything. It may only be me but I first thought that the description Enable User to choose Nextcloud addressbook for recipients would let the admin choose if the user can choose the address book by himself or only the admin can do this by setting the fields below. Therefore a "Enable" checkbox or some grouping came into my mind. I thought that this maybe makes it clear that disabling the first option would disable everything and not only the possibility to let the user choose.

updated README. cc @cm-schl

@fahim44
Copy link

fahim44 commented Sep 26, 2024

Hi @the-djmaze ! Can you give a look? TIA :)

@botsarenthuman
Copy link

This work looks FANTASTIC

@the-djmaze
Copy link
Owner

Hi @the-djmaze ! Can you give a look? TIA :)

Will look at it. Just had some other things to do ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants