Skip to content

fix(contactsMenu): Attach user cloud to each contact entry #44954

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.

Sign up for GitHub

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

Closed
nfebe wants to merge 1 commit into from
Closed

fix(contactsMenu): Attach user cloud to each contact entry #44954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions core/src/components/UnifiedSearch/SearchableList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
:wide="true"
@click="itemSelected(element)">
<template #icon>
<NcAvatar :user="element.user" :show-user-status="false" :hide-favorite="false" />
<NcAvatar v-if="element.isUser" :user="element.user" :show-user-status="false" :hide-favorite="false" />
<NcAvatar v-else :url="element.avatar" :show-user-status="false" :hide-favorite="false" />
</template>
{{ element.displayName }}
</NcButton>
Expand Down Expand Up @@ -117,7 +118,6 @@ export default {
})
},
},

methods: {
clearSearch() {
this.searchTerm = ''
Expand All @@ -128,6 +128,7 @@ export default {
this.opened = false
},
searchTermChanged(term) {
console.debug('Users (search)', this.filteredList) // WIP, would remove
this.$emit('search-term-change', term)
},
},
Expand Down
1 change: 1 addition & 0 deletions core/src/services/UnifiedSearchService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export async function getContacts({ searchTerm }) {
id: authenticatedUser.uid,
fullName: authenticatedUser.displayName,
emailAddresses: [],
isUser: true,
}
contacts.unshift(authenticatedUser)
return contacts
Expand Down
3 changes: 3 additions & 0 deletions core/src/views/UnifiedSearchModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ export default {
},
mapContacts(contacts) {
return contacts.map(contact => {
console.debug('CONTACT VIEW', contact) // WIP would remove
return {
// id: contact.id,
// name: '',
Expand All @@ -391,6 +392,8 @@ export default {
subname: contact.emailAddresses[0] ? contact.emailAddresses[0] : '',
icon: '',
user: contact.id,
isUser: contact.isUser,
avatar: contact.avatar,
}
})
},
Expand Down
29 changes: 24 additions & 5 deletions lib/private/Contacts/ContactsMenu/ContactsStore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

namespace OC\Contacts\ContactsMenu;

use OC\Federation\CloudId;
use OC\KnownUser\KnownUserService;
use OC\Profile\ProfileManager;
use OCA\UserStatus\Db\UserStatus;
Expand All @@ -44,6 +45,7 @@
use OCP\IUser;
use OCP\IUserManager;
use OCP\L10N\IFactory as IL10NFactory;
use Psr\Log\LoggerInterface;
use function array_column;
use function array_fill_keys;
use function array_filter;
Expand All @@ -62,6 +64,7 @@ public function __construct(
private IGroupManager $groupManager,
private KnownUserService $knownUserService,
private IL10NFactory $l10nFactory,
private LoggerInterface $logger,
) {
}

Expand Down Expand Up @@ -351,19 +354,35 @@ public function findOne(IUser $user, int $shareType, string $shareWith): ?IEntry

private function contactArrayToEntry(array $contact): Entry {
$entry = new Entry();

if (!empty($contact['UID'])) {
$uid = $contact['UID'];
$entry->setId($uid);
$entry->setProperty('isUser', false);
$username = '';
$remoteServer = '';

if (isset($contact['CLOUD']) && is_array($contact['CLOUD']) && isset($contact['CLOUD'][0])) {
preg_match('/^(.*?)@(https?:\/\/.*?)$/', $contact['CLOUD'][0], $matches);
Copy link
Member

@nickvergessen nickvergessen Apr 22, 2024

Choose a reason for hiding this comment

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

Please use the ICloudManager::resolve() method instead?

if (count($matches) === 3) {
$username = $matches[1];
$remoteServer = $matches[2];
$cloud = new CloudId($entry->getId(), $username, $remoteServer);
$entry->setCloudId($cloud);
$this->logger->warning('Set address cloud: ' . json_encode(['username' => $username, 'server' => $remoteServer]));
} else {
$this->logger->warning('Unable to process contact remote server: ' . $contact['CLOUD'][0]);
}
} else {
$this->logger->warning('Invalid remote server data');
}
// overloaded usage so leaving as-is for now
if (isset($contact['isLocalSystemBook'])) {
$avatar = $this->urlGenerator->linkToRouteAbsolute('core.avatar.getAvatar', ['userId' => $uid, 'size' => 64]);
$entry->setProperty('isUser', true);
} elseif (!empty($contact['FN'])) {
$avatar = $this->urlGenerator->linkToRouteAbsolute('core.GuestAvatar.getAvatar', ['guestName' => str_replace('/', ' ', $contact['FN']), 'size' => 64]);
} elseif ($username != '') {
$avatar = $this->urlGenerator->linkToRemoteRouteAbsolute($remoteServer, 'core.avatar.getAvatar', ['userId' => str_replace('/', ' ', $username), 'size' => 64]);

Check failure

Code scanning / Psalm

UndefinedInterfaceMethod

Method OCP\IURLGenerator::linkToRemoteRouteAbsolute does not exist
Copy link
Member

@nickvergessen nickvergessen Apr 22, 2024

Choose a reason for hiding this comment

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

This assumes every cloud ID is a nextcloud? 🫣

} else {
$avatar = $this->urlGenerator->linkToRouteAbsolute('core.GuestAvatar.getAvatar', ['guestName' => str_replace('/', ' ', $uid), 'size' => 64]);
$avatar = $this->urlGenerator->linkToRemoteRouteAbsolute($remoteServer, 'core.avatar.getAvatar', ['userId' => str_replace('/', ' ', $uid), 'size' => 64]);

Check failure

Code scanning / Psalm

UndefinedInterfaceMethod

Method OCP\IURLGenerator::linkToRemoteRouteAbsolute does not exist
}
$entry->setAvatar($avatar);
}
Expand All @@ -374,7 +393,7 @@ private function contactArrayToEntry(array $contact): Entry {

$avatarPrefix = "VALUE=uri:";
if (!empty($contact['PHOTO']) && str_starts_with($contact['PHOTO'], $avatarPrefix)) {
$entry->setAvatar(substr($contact['PHOTO'], strlen($avatarPrefix)));
//$entry->setAvatar(substr($contact['PHOTO'], strlen($avatarPrefix)));
}

if (!empty($contact['EMAIL'])) {
Expand Down
71 changes: 40 additions & 31 deletions lib/private/Contacts/ContactsMenu/Entry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,41 +27,40 @@

namespace OC\Contacts\ContactsMenu;

use OC\Federation\CloudId;
use OCP\Contacts\ContactsMenu\IAction;
use OCP\Contacts\ContactsMenu\IEntry;
use function array_merge;

class Entry implements IEntry {
public const PROPERTY_STATUS_MESSAGE_TIMESTAMP = 'statusMessageTimestamp';

/** @var string|int|null */
private $id = null;

private string $fullName = '';

/** @var string[] */
private array $emailAddresses = [];

private ?string $avatar = null;

private ?string $profileTitle = null;

private ?string $profileUrl = null;

/** @var IAction[] */
private array $actions = [];

private array $properties = [];
public function __construct(
private ?string $id = null,
private string $fullName = '',
private array $emailAddresses = [],
private ?string $avatar = null,
private ?string $profileTitle = null,
private ?string $profileUrl = null,
private array $actions = [],
private array $properties = [],
private ?string $status = null,
private ?string $statusMessage = null,
private ?int $statusMessageTimestamp = null,
private ?string $statusIcon = null,
private ?CloudId $cloud = null
) {
}

private ?string $status = null;
private ?string $statusMessage = null;
private ?int $statusMessageTimestamp = null;
private ?string $statusIcon = null;

public function setId(string $id): void {
$this->id = $id;
}

public function getId(): string {

Check failure

Code scanning / Psalm

InvalidNullableReturnType

The declared return type 'string' for OC\Contacts\ContactsMenu\Entry::getId is not nullable, but 'null|string' contains null
return $this->id;

Check failure

Code scanning / Psalm

NullableReturnStatement

The declared return type 'string' for OC\Contacts\ContactsMenu\Entry::getId is not nullable, but the function returns 'null|string'
}

public function setFullName(string $displayName): void {
$this->fullName = $displayName;
}
Expand Down Expand Up @@ -163,8 +162,25 @@ public function getProperty(string $key): mixed {
return $this->properties[$key];
}


public function getStatusMessage(): ?string {
return $this->statusMessage;
}

public function getStatusMessageTimestamp(): ?int {
return $this->statusMessageTimestamp;
}

public function setCloudId(CloudId $cloudId) {
$this->cloud = $cloudId;
}

public function getCloud(): CloudId {

Check failure

Code scanning / Psalm

InvalidNullableReturnType

The declared return type 'OC\Federation\CloudId' for OC\Contacts\ContactsMenu\Entry::getCloud is not nullable, but 'OC\Federation\CloudId|null' contains null
return $this->cloud;

Check failure

Code scanning / Psalm

NullableReturnStatement

The declared return type 'OC\Federation\CloudId' for OC\Contacts\ContactsMenu\Entry::getCloud is not nullable, but the function returns 'OC\Federation\CloudId|null'
}

/**
* @return array{id: int|string|null, fullName: string, avatar: string|null, topAction: mixed, actions: array, lastMessage: '', emailAddresses: string[], profileTitle: string|null, profileUrl: string|null, status: string|null, statusMessage: null|string, statusMessageTimestamp: null|int, statusIcon: null|string, isUser: bool, uid: mixed}
* @return array{id: int|string|null, fullName: string, avatar: string|null, topAction: mixed, actions: array, lastMessage: '', emailAddresses: string[], profileTitle: string|null, profileUrl: string|null, status: string|null, statusMessage: null|string, statusMessageTimestamp: null|int, statusIcon: null|string, isUser: bool, uid: mixed, cloud: mixed}
Copy link
Member

@nickvergessen nickvergessen Apr 22, 2024

Choose a reason for hiding this comment

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

cloud should be called cloudId to be more clear and the type should be the array you define in the other file

*/
public function jsonSerialize(): array {
$topAction = !empty($this->actions) ? $this->actions[0]->jsonSerialize() : null;
Expand All @@ -188,14 +204,7 @@ public function jsonSerialize(): array {
'statusIcon' => $this->statusIcon,
'isUser' => $this->getProperty('isUser') === true,
'uid' => $this->getProperty('UID'),
'cloud' => $this->cloud,
];
}

public function getStatusMessage(): ?string {
return $this->statusMessage;
}

public function getStatusMessageTimestamp(): ?int {
return $this->statusMessageTimestamp;
}
}
12 changes: 12 additions & 0 deletions lib/private/Federation/CloudId.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,16 @@ public function getUser(): string {
public function getRemote(): string {
return $this->remote;
}

/**
* @return array{id: string, user: string, remote: string, displayName: string|null}
*/
public function jsonSerialize(): array {
Copy link
Member

@nickvergessen nickvergessen Apr 22, 2024

Choose a reason for hiding this comment

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

Should implement the interface JsonSerializable?

Copy link
Member

@nickvergessen nickvergessen Apr 22, 2024

Choose a reason for hiding this comment

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

That being said, should be on the public interface?

return [
'id' => $this->id,
'user' => $this->user,
'remote' => $this->remote,
'displayName' => $this->displayName,
];
}
}
13 changes: 13 additions & 0 deletions lib/private/URLGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ public function linkToRouteAbsolute(string $routeName, array $arguments = []): s
return $this->getAbsoluteURL($this->linkToRoute($routeName, $arguments));
}


public function linkToRemoteRouteAbsolute(string $remote, $routeName, array $arguments = []): string {

Check failure

Code scanning / Psalm

InvalidNullableReturnType

The declared return type 'string' for OC\URLGenerator::linkToRemoteRouteAbsolute is not nullable, but 'null|string' contains null
Copy link
Member

@nickvergessen nickvergessen Apr 22, 2024

Choose a reason for hiding this comment

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

This should not be here or on the public interface

return $this->formatAsUrl($remote, $this->linkToRoute($routeName, $arguments));

Check failure

Code scanning / Psalm

NullableReturnStatement

The declared return type 'string' for OC\URLGenerator::linkToRemoteRouteAbsolute is not nullable, but the function returns 'null|string'
}

private function formatAsUrl(string $baseUrl, string $restUrl): ?string {
$baseUrl = trim($baseUrl);
if (empty($baseUrl) || !filter_var(preg_match("~^(?:f|ht)tps?://~i", $baseUrl) ? $baseUrl : "http://$baseUrl", FILTER_VALIDATE_URL)) {
return null;
}
return filter_var($baseUrl . $restUrl, FILTER_VALIDATE_URL) ? $baseUrl . $restUrl : null;
}

public function linkToOCSRouteAbsolute(string $routeName, array $arguments = []): string {
// Returns `/subfolder/index.php/ocsapp/…` with `'htaccess.IgnoreFrontController' => false` in config.php
// And `/subfolder/ocsapp/…` with `'htaccess.IgnoreFrontController' => true` in config.php
Expand Down