Skip to content

Commit 994f629

Browse files
committed
Review fixes
Signed-off-by: Gary Kim <gary@garykim.dev>
1 parent d30a073 commit 994f629

7 files changed

Lines changed: 61 additions & 55 deletions

File tree

appinfo/routes.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,15 +520,15 @@
520520

521521
[
522522
'name' => 'Federation#acceptShare',
523-
'url' => 'api/{apiVersion}/federation/pending/{id}',
523+
'url' => 'api/{apiVersion}/federation/invitation/{id}',
524524
'verb' => 'POST',
525525
'requirements' => [
526526
'apiVersion' => 'v1',
527527
],
528528
],
529529
[
530530
'name' => 'Federation#rejectShare',
531-
'url' => 'api/{apiVersion}/federation/pending/{id}',
531+
'url' => 'api/{apiVersion}/federation/invitation/{id}',
532532
'verb' => 'DELETE',
533533
'requirements' => [
534534
'apiVersion' => 'v1',

lib/Federation/CloudFederationProviderTalk.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@
2626
namespace OCA\Talk\Federation;
2727

2828
use Exception;
29-
use OC\HintException;
3029
use OCA\FederatedFileSharing\AddressHandler;
3130
use OCA\Talk\AppInfo\Application;
3231
use OCA\Talk\Manager;
3332
use OCA\Talk\Model\Attendee;
3433
use OCA\Talk\Model\AttendeeMapper;
3534
use OCA\Talk\Participant;
35+
use OCA\Talk\Room;
3636
use OCA\Talk\Service\ParticipantService;
3737
use OCP\AppFramework\Http;
3838
use OCP\DB\Exception as DBException;
@@ -42,6 +42,7 @@
4242
use OCP\Federation\Exceptions\ProviderCouldNotAddShareException;
4343
use OCP\Federation\ICloudFederationProvider;
4444
use OCP\Federation\ICloudFederationShare;
45+
use OCP\HintException;
4546
use OCP\IURLGenerator;
4647
use OCP\IUser;
4748
use OCP\IUserManager;
@@ -110,8 +111,8 @@ public function shareReceived(ICloudFederationShare $share): string {
110111
if (!$this->federationManager->isEnabled()) {
111112
throw new ProviderCouldNotAddShareException('Server does not support talk federation', '', Http::STATUS_SERVICE_UNAVAILABLE);
112113
}
113-
if ($share->getShareType() !== 'user') {
114-
throw new ProviderCouldNotAddShareException('support for sharing with non-users not implemented yet', '', Http::STATUS_NOT_IMPLEMENTED);
114+
if (!in_array($share->getShareType(), $this->getSupportedShareTypes(), true)) {
115+
throw new ProviderCouldNotAddShareException('Support for sharing with non-users not implemented yet', '', Http::STATUS_NOT_IMPLEMENTED);
115116
// TODO: Implement group shares
116117
}
117118

@@ -199,7 +200,7 @@ private function shareDeclined(int $id, array $notification): array {
199200

200201
$room = $this->manager->getRoomById($attendee->getRoomId());
201202
$participant = new Participant($room, $attendee, null);
202-
$this->participantService->removeAttendee($room, $participant, 'Left Room');
203+
$this->participantService->removeAttendee($room, $participant, Room::PARTICIPANT_LEFT);
203204
return [];
204205
}
205206

@@ -218,7 +219,7 @@ private function getAttendeeAndValidate(int $id, string $sharedSecret): Attendee
218219
} catch (Exception $ex) {
219220
throw new ShareNotFound();
220221
}
221-
if ($attendee->getActorType() !== Attendee::ACTOR_FEDERATED_REMOTE_USER) {
222+
if ($attendee->getActorType() !== Attendee::ACTOR_FEDERATED_USERS) {
222223
throw new ShareNotFound();
223224
}
224225
if ($attendee->getAccessToken() !== $sharedSecret) {

lib/Federation/FederationManager.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
namespace OCA\Talk\Federation;
2727

28+
use OCA\Talk\AppInfo\Application;
2829
use OCA\Talk\Exceptions\RoomNotFoundException;
2930
use OCA\Talk\Exceptions\UnauthorizedException;
3031
use OCA\Talk\Manager;
@@ -76,7 +77,7 @@ public function __construct(
7677
*/
7778
public function isEnabled(): bool {
7879
// TODO: Set to default true once implementation is complete
79-
return $this->config->getSystemValueBool('talk_federation_enabled', false);
80+
return $this->config->getAppValue(Application::APP_ID, 'federation_enabled', "false") === "true";
8081
}
8182

8283
/**

lib/Manager.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ public function getRoomById(int $roomId): Room {
630630
* @return Room
631631
* @throws RoomNotFoundException
632632
*/
633-
public function getRoomByActor(string $token, string $actorType, string $actorId, ?string $sessionId = null, ?string $server_url = null): Room {
633+
public function getRoomByActor(string $token, string $actorType, string $actorId, ?string $sessionId = null, ?string $serverUrl = null): Room {
634634
$query = $this->db->getQueryBuilder();
635635
$helper = new SelectHelper();
636636
$helper->selectRoomsTable($query);
@@ -643,10 +643,10 @@ public function getRoomByActor(string $token, string $actorType, string $actorId
643643
))
644644
->where($query->expr()->eq('r.token', $query->createNamedParameter($token)));
645645

646-
if ($server_url === null) {
646+
if ($serverUrl === null) {
647647
$query->andWhere($query->expr()->isNull('r.server_url'));
648648
} else {
649-
$query->andWhere($query->expr()->eq('r.server_url', $query->createNamedParameter($server_url)));
649+
$query->andWhere($query->expr()->eq('r.server_url', $query->createNamedParameter($serverUrl)));
650650
}
651651

652652
if ($sessionId !== null) {

lib/Migration/Version13000Date20210625232111.php

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -44,44 +44,45 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
4444
/** @var ISchemaWrapper $schema */
4545
$schema = $schemaClosure();
4646

47-
if ($schema->hasTable('talk_attendees')) {
48-
$table = $schema->getTable('talk_attendees');
49-
if (!$table->hasColumn('access_token')) {
50-
$table->addColumn('access_token', Types::STRING, [
51-
'notnull' => false,
52-
'default' => null,
53-
]);
54-
}
47+
$table = $schema->getTable('talk_attendees');
48+
if (!$table->hasColumn('access_token')) {
49+
$table->addColumn('access_token', Types::STRING, [
50+
'notnull' => false,
51+
'default' => null,
52+
]);
5553
}
5654

57-
if ($schema->hasTable('talk_rooms')) {
58-
$table = $schema->getTable('talk_rooms');
59-
if (!$table->hasColumn('server_url')) {
60-
$table->addColumn('server_url', Types::STRING, [
61-
'notnull' => false,
62-
'default' => null,
63-
]);
64-
}
55+
$table = $schema->getTable('talk_rooms');
56+
if (!$table->hasColumn('server_url')) {
57+
$table->addColumn('server_url', Types::STRING, [
58+
'notnull' => false,
59+
'default' => null,
60+
]);
6561
}
6662

67-
$table = $schema->createTable('talk_invitations');
68-
$table->addColumn('id', Types::BIGINT, [
69-
'autoincrement' => true,
70-
'notnull' => true,
71-
]);
72-
$table->addColumn('room_id', Types::BIGINT, [
73-
'notnull' => true,
74-
'unsigned' => true,
75-
]);
76-
$table->addColumn('user_id', Types::STRING, [
77-
'notnull' => true,
78-
'length' => 255,
79-
]);
80-
$table->addColumn('access_token', Types::STRING, [
81-
'notnull' => true,
82-
]);
63+
if (!$schema->hasTable('talk_invitations')) {
64+
$table = $schema->createTable('talk_invitations');
65+
$table->addColumn('id', Types::BIGINT, [
66+
'autoincrement' => true,
67+
'notnull' => true,
68+
]);
69+
$table->addColumn('room_id', Types::BIGINT, [
70+
'notnull' => true,
71+
'unsigned' => true,
72+
]);
73+
$table->addColumn('user_id', Types::STRING, [
74+
'notnull' => true,
75+
'length' => 255,
76+
]);
77+
$table->addColumn('access_token', Types::STRING, [
78+
'notnull' => true,
79+
]);
80+
81+
$table->setPrimaryKey(['id']);
82+
83+
$table->addIndex(['room_id']);
84+
}
8385

84-
$table->setPrimaryKey(['id']);
8586

8687
return $schema;
8788
}

lib/Model/Attendee.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class Attendee extends Entity {
6060
public const ACTOR_GUESTS = 'guests';
6161
public const ACTOR_EMAILS = 'emails';
6262
public const ACTOR_CIRCLES = 'circles';
63-
public const ACTOR_FEDERATED_REMOTE_USER = 'federated_remote';
63+
public const ACTOR_FEDERATED_USERS = 'federated_users';
6464

6565
public const PUBLISHING_PERMISSIONS_NONE = 0;
6666
public const PUBLISHING_PERMISSIONS_AUDIO = 1;

lib/Notification/Notifier.php

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323

2424
namespace OCA\Talk\Notification;
2525

26-
use OC\HintException;
2726
use OCA\FederatedFileSharing\AddressHandler;
2827
use OCA\Talk\Chat\CommentsManager;
2928
use OCA\Talk\Chat\MessageParser;
@@ -38,6 +37,7 @@
3837
use OCA\Talk\Service\ParticipantService;
3938
use OCP\Comments\ICommentsManager;
4039
use OCP\Comments\NotFoundException;
40+
use OCP\HintException;
4141
use OCP\IL10N;
4242
use OCP\IURLGenerator;
4343
use OCP\IUser;
@@ -289,14 +289,8 @@ protected function parseRemoteInvitationMessage(INotification $notification, IL1
289289
[$sharedById, $sharedByServer] = $this->addressHandler->splitUserRemote($subjectParameters['sharedByFederatedId']);
290290

291291
$message = $l->t('{user1} shared room {roomName} on {remoteServer} with you');
292-
$parsedMessage = $l->t('{user1} shared room {roomName} on {remoteServer} with you', [
293-
'user1' => $subjectParameters['sharedByFederatedId'],
294-
'roomName' => $subjectParameters['roomName'],
295-
'remoteServer' => $subjectParameters['serverUrl'],
296-
]);
297-
298-
$notification->setParsedMessage($parsedMessage);
299-
$notification->setRichMessage($message, [
292+
293+
$rosParameters = [
300294
'user1' => [
301295
'type' => 'user',
302296
'id' => $sharedById,
@@ -313,7 +307,16 @@ protected function parseRemoteInvitationMessage(INotification $notification, IL1
313307
'id' => $subjectParameters['serverUrl'],
314308
'name' => $subjectParameters['serverUrl'],
315309
]
316-
]);
310+
];
311+
312+
$placeholders = $replacements = [];
313+
foreach ($rosParameters as $placeholder => $parameter) {
314+
$placeholders[] = '{' . $placeholder .'}';
315+
$replacements[] = $parameter['name'];
316+
}
317+
318+
$notification->setParsedMessage(str_replace($placeholders, $replacements, $message));
319+
$notification->setRichMessage($message, $rosParameters);
317320

318321
return $notification;
319322
}

0 commit comments

Comments
 (0)