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

Provide contactsmenu as ocs #29249

Closed

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen added this to the Nextcloud 23 milestone Oct 15, 2021
@nickvergessen nickvergessen requested review from skjnldsv, Pytal, a team, ArtificialOwl and CarlSchwan and removed request for a team October 15, 2021 07:58
@nickvergessen nickvergessen mentioned this pull request Oct 15, 2021
28 tasks
@tobiasKaminsky
Copy link
Member

I tested it against my local nc:

{
  "ocs": {
    "meta": {
      "status": "ok",
      "statuscode": 200,
      "message": "OK"
    },
    "data": [
      {
        "id": null,
        "fullName": "a b c",
        "avatar": "http:\/\/localhost\/nc\/remote.php\/dav\/addressbooks\/system\/system\/system\/Database:a b c.vcf?photo",
        "topAction": {
          "title": "Talk to a b c",
          "icon": "http:\/\/localhost\/nc\/apps\/spreed\/img\/app-dark.svg",
          "hyperlink": "http:\/\/localhost\/nc\/index.php\/apps\/spreed\/?callUser=a b c"
        },
        "actions": [],
        "lastMessage": "",
        "emailAddresses": []
      },
  • id is null?
  • avatar should be the public avatar

@nickvergessen
Copy link
Member Author

Oh, so I didn't know, but the contactmanager basically works on the (system)addressbook.
So there is no "id" because the systemaddressbook is not serving the user id or a contact id and that is also why the avatar is a dav resource.

Doesn't look fixable in short term. So either we just do it this way or we have to delay the PR I fear

@skjnldsv
Copy link
Member

  • id is null?

if (isset($contact['id'])) {
$entry->setId($contact['id']);
}
if (isset($contact['FN'])) {

We can fix that easily by making sure we set the id that is not set anymore $contact['UID'] works, but might not make sense for normal contacts (non system addressbook I mean)

  • avatar should be the public avatar

Two options:

  1. We fix the davAvatar provider and redirect to avatar endpoint if the addressbook is the system addressbook
  2. We properly fix/store the user Avatar in the cached system addressbook vcard
  3. We fix the contactsMenu method and if we encounter a contact from the system addressbook, we use the avatar endpoint
    $entry->setAvatar(substr($contact['PHOTO'], strlen($avatarPrefix)));
    }
    if (isset($contact['EMAIL'])) {

@tobiasKaminsky
Copy link
Member

For profile we do not need the avatar, I think. As this is only used when we show a sharee, so the user Avatar is already present.

@skjnldsv skjnldsv force-pushed the feature/28751/provide-contactsmenu-as-ocs branch from 5fde757 to 33128e6 Compare October 15, 2021 16:46
@skjnldsv
Copy link
Member

ID is fixed @tobiasKaminsky

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@skjnldsv skjnldsv force-pushed the feature/28751/provide-contactsmenu-as-ocs branch from 33128e6 to 660090d Compare October 15, 2021 16:52
@nickvergessen
Copy link
Member Author

I would instead go with #29269
Much more usable for the clients and a better way forward then populating whatever the current private api tried to do.

@nickvergessen nickvergessen deleted the feature/28751/provide-contactsmenu-as-ocs branch October 15, 2021 20:10
@skjnldsv skjnldsv restored the feature/28751/provide-contactsmenu-as-ocs branch October 18, 2021 10:14
@Pytal Pytal mentioned this pull request Oct 19, 2021
20 tasks
@skjnldsv skjnldsv deleted the feature/28751/provide-contactsmenu-as-ocs branch August 22, 2023 17:36
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.

4 participants