Skip to content

Commit fd94986

Browse files
skjnldsvskjnldsv
authored
Merge pull request #1245 from nextcloud/feat/polish_collaboration
2 parents 1099a28 + 46d9982 commit fd94986

12 files changed

Lines changed: 101 additions & 58 deletions

‎cypress/e2e/shared_albums.cy.js‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ describe('Manage shared albums', () => {
4747
cy.visit(`${Cypress.env('baseUrl')}/index.php/apps/photos/albums`)
4848
cy.createAnAlbumFromAlbums('shared_album_test3')
4949
cy.addCollaborators([randUser2])
50+
cy.visit(`${Cypress.env('baseUrl')}/index.php/apps/photos/albums`)
51+
cy.createAnAlbumFromAlbums('shared_album_test4')
52+
cy.addCollaborators([randUser2])
5053
cy.logout()
5154

5255
cy.login(randUser2, 'password')
@@ -120,8 +123,23 @@ describe('Manage shared albums', () => {
120123
cy.get('[data-test="media"]').should('have.length', 0)
121124
})
122125

123-
xit('Remove collaborator from an album', () => {
126+
xit('Remove shared album', () => {
124127
cy.goToSharedAlbum('shared_album_test3')
125128
cy.removeSharedAlbums()
126129
})
130+
131+
xit('Remove collaborator from an album', () => {
132+
cy.get('[data-test="media"]').should('have.length', 4)
133+
134+
cy.logout()
135+
cy.login(randUser, 'password')
136+
cy.goToAlbum('shared_album_test4')
137+
cy.removeCollaborators([randUser2])
138+
cy.logout()
139+
140+
cy.login(randUser2, 'password')
141+
cy.visit(`${Cypress.env('baseUrl')}/index.php/apps/photos/sharedalbums`)
142+
143+
cy.get('ul.collections__list li').should('have.length', 3)
144+
})
127145
})

‎js/photos-main.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/photos-main.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/photos-src_mixins_FetchAlbumsMixin_js-node_modules_vue-material-design-icons_Plus_vue-src_components-f20774.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/photos-src_mixins_FetchAlbumsMixin_js-node_modules_vue-material-design-icons_Plus_vue-src_components-f20774.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/photos-src_views_AlbumContent_vue.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/photos-src_views_AlbumContent_vue.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/Album/AlbumMapper.php‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,11 @@ private function getLastAdded(int $albumId): int {
249249

250250
/**
251251
* @param int $albumId
252-
* @return array<array{'id': string, 'label': string, 'source': int}>
252+
* @return array<array{'id': string, 'label': string, 'type': int}>
253253
*/
254254
public function getCollaborators(int $albumId): array {
255255
$query = $this->connection->getQueryBuilder();
256-
$query->select("collaborator_id", "collaborator_source")
256+
$query->select("collaborator_id", "collaborator_type")
257257
->from("photos_collaborators")
258258
->where($query->expr()->eq('album_id', $query->createNamedParameter($albumId, IQueryBuilder::PARAM_INT)));
259259

@@ -263,15 +263,15 @@ public function getCollaborators(int $albumId): array {
263263
/** @var IUser|IGroup|null */
264264
$collaborator = null;
265265

266-
switch ($row['collaborator_source']) {
266+
switch ($row['collaborator_type']) {
267267
case self::TYPE_USER:
268268
$collaborator = $this->userManager->get($row['collaborator_id']);
269269
break;
270270
case self::TYPE_GROUP:
271271
$collaborator = $this->groupManager->get($row['collaborator_id']);
272272
break;
273273
default:
274-
throw new \Exception('Invalid collaborator source: ' . $row['collaborator_source']);
274+
throw new \Exception('Invalid collaborator type: ' . $row['collaborator_type']);
275275
}
276276

277277
if (is_null($collaborator)) {
@@ -281,7 +281,7 @@ public function getCollaborators(int $albumId): array {
281281
return [
282282
'id' => $row['collaborator_id'],
283283
'label' => $collaborator->getDisplayName(),
284-
'source' => (int)$row['collaborator_source'],
284+
'type' => $row['collaborator_type'],
285285
];
286286
}, $rows);
287287

@@ -290,16 +290,18 @@ public function getCollaborators(int $albumId): array {
290290

291291
/**
292292
* @param int $albumId
293-
* @param array{'id': string, 'label': string, 'source': int} $newCollaborators
293+
* @param array{'id': string, 'label': string, 'type': int} $collaborators
294294
*/
295-
public function setCollaborators(int $albumId, array $newCollaborators): void {
295+
public function setCollaborators(int $albumId, array $collaborators): void {
296296
$existingCollaborators = $this->getCollaborators($albumId);
297297

298-
$collaboratorsToAdd = array_udiff($newCollaborators, $existingCollaborators, fn ($a, $b) => strcmp($a['id'].$a['source'], $b['id'].$b['source']));
299-
$collaboratorsToRemove = array_udiff($existingCollaborators, $newCollaborators, fn ($a, $b) => strcmp($a['id'].$a['source'], $b['id'].$b['source']));
298+
$collaboratorsToAdd = array_udiff($collaborators, $existingCollaborators, fn ($a, $b) => strcmp($a['id'].$a['type'], $b['id'].$b['type']));
299+
$collaboratorsToRemove = array_udiff($existingCollaborators, $collaborators, fn ($a, $b) => strcmp($a['id'].$a['type'], $b['id'].$b['type']));
300+
301+
$this->connection->beginTransaction();
300302

301303
foreach ($collaboratorsToAdd as $collaborator) {
302-
switch ($collaborator['source']) {
304+
switch ($collaborator['type']) {
303305
case self::TYPE_USER:
304306
if (is_null($this->userManager->get($collaborator['id']))) {
305307
throw new \Exception('Unknown collaborator: ' . $collaborator['id']);
@@ -311,14 +313,14 @@ public function setCollaborators(int $albumId, array $newCollaborators): void {
311313
}
312314
break;
313315
default:
314-
throw new \Exception('Invalid collaborator source: ' . $collaborator['source']);
316+
throw new \Exception('Invalid collaborator type: ' . $collaborator['type']);
315317
}
316318

317319
$query = $this->connection->getQueryBuilder();
318320
$query->insert('photos_collaborators')
319321
->setValue('album_id', $query->createNamedParameter($albumId, IQueryBuilder::PARAM_INT))
320322
->setValue('collaborator_id', $query->createNamedParameter($collaborator['id']))
321-
->setValue('collaborator_source', $query->createNamedParameter($collaborator['source'], IQueryBuilder::PARAM_INT))
323+
->setValue('collaborator_type', $query->createNamedParameter($collaborator['type'], IQueryBuilder::PARAM_INT))
322324
->executeStatement();
323325
}
324326

@@ -327,16 +329,19 @@ public function setCollaborators(int $albumId, array $newCollaborators): void {
327329
$query->delete('photos_collaborators')
328330
->where($query->expr()->eq('album_id', $query->createNamedParameter($albumId, IQueryBuilder::PARAM_INT)))
329331
->andWhere($query->expr()->eq('collaborator_id', $query->createNamedParameter($collaborator['id'])))
330-
->andWhere($query->expr()->eq('collaborator_source', $query->createNamedParameter($collaborator['source'], IQueryBuilder::PARAM_INT)))
332+
->andWhere($query->expr()->eq('collaborator_type', $query->createNamedParameter($collaborator['type'], IQueryBuilder::PARAM_INT)))
331333
->executeStatement();
332334
}
335+
336+
$this->connection->commit();
333337
}
334338

335339
/**
336-
* @param string $userId
340+
* @param string $collaboratorId
341+
* @param string $collaboratorsType - The type of the collaborator, either a user or a group.
337342
* @return AlbumWithFiles[]
338343
*/
339-
public function getSharedAlbumsForCollaboratorWithFiles(string $collaboratorId, int $collaboratorSource): array {
344+
public function getSharedAlbumsForCollaboratorWithFiles(string $collaboratorId, int $collaboratorType): array {
340345
$query = $this->connection->getQueryBuilder();
341346
$rows = $query
342347
->select("fileid", "mimetype", "a.album_id", "size", "mtime", "etag", "location", "created", "last_added_photo", "added", 'owner')
@@ -348,7 +353,7 @@ public function getSharedAlbumsForCollaboratorWithFiles(string $collaboratorId,
348353
->leftJoin("a", "photos_albums_files", "p", $query->expr()->eq("a.album_id", "p.album_id"))
349354
->leftJoin("p", "filecache", "f", $query->expr()->eq("p.file_id", "f.fileid"))
350355
->where($query->expr()->eq('collaborator_id', $query->createNamedParameter($collaboratorId)))
351-
->andWhere($query->expr()->eq('collaborator_source', $query->createNamedParameter($collaboratorSource, IQueryBuilder::PARAM_INT)))
356+
->andWhere($query->expr()->eq('collaborator_type', $query->createNamedParameter($collaboratorType, IQueryBuilder::PARAM_INT)))
352357
->executeQuery()
353358
->fetchAll();
354359

@@ -379,31 +384,31 @@ public function getSharedAlbumsForCollaboratorWithFiles(string $collaboratorId,
379384
* @param int $albumId
380385
* @return void
381386
*/
382-
public function deleteCollaboratorFromAlbum(string $userId, int $albumId): void {
387+
public function deleteUserFromAlbumCollaboratorsList(string $userId, int $albumId): void {
383388
// TODO: only delete if this was not a group share
384389
$query = $this->connection->getQueryBuilder();
385390
$query->delete('photos_collaborators')
386391
->where($query->expr()->eq('album_id', $query->createNamedParameter($albumId, IQueryBuilder::PARAM_INT)))
387392
->andWhere($query->expr()->eq('collaborator_id', $query->createNamedParameter($userId)))
388-
->andWhere($query->expr()->eq('collaborator_source', $query->createNamedParameter(self::TYPE_USER, IQueryBuilder::PARAM_INT)))
393+
->andWhere($query->expr()->eq('collaborator_type', $query->createNamedParameter(self::TYPE_USER, IQueryBuilder::PARAM_INT)))
389394
->executeStatement();
390395
}
391396

392397
/**
393398
* @param string $collaboratorId
394-
* @param int $collaboratorSource
399+
* @param int $collaboratorType
395400
* @param int $fileId
396401
* @return AlbumInfo[]
397402
*/
398-
public function getAlbumForCollaboratorIdAndFileId(string $collaboratorId, int $collaboratorSource, int $fileId): array {
403+
public function getAlbumForCollaboratorIdAndFileId(string $collaboratorId, int $collaboratorType, int $fileId): array {
399404
$query = $this->connection->getQueryBuilder();
400405
$rows = $query
401406
->select("a.album_id", "name", "user", "location", "created", "last_added_photo")
402407
->from("photos_collaborators", "c")
403408
->leftJoin("c", "photos_albums", "a", $query->expr()->eq("a.album_id", "c.album_id"))
404409
->leftJoin("a", "photos_albums_files", "p", $query->expr()->eq("a.album_id", "p.album_id"))
405410
->where($query->expr()->eq('collaborator_id', $query->createNamedParameter($collaboratorId)))
406-
->andWhere($query->expr()->eq('collaborator_source', $query->createNamedParameter($collaboratorSource, IQueryBuilder::PARAM_INT)))
411+
->andWhere($query->expr()->eq('collaborator_type', $query->createNamedParameter($collaboratorType, IQueryBuilder::PARAM_INT)))
407412
->andWhere($query->expr()->eq('file_id', $query->createNamedParameter($fileId)))
408413
->groupBy('album_id')
409414
->executeQuery()

‎lib/Migration/Version20001Date20220830131446.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ class Version20001Date20220830131446 extends SimpleMigrationStep {
4545
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
4646
/** @var ISchemaWrapper $schema */
4747
$schema = $schemaClosure();
48+
$modified = false;
4849

4950
if (!$schema->hasTable("photos_collaborators")) {
51+
$modified = true;
5052
$table = $schema->createTable("photos_collaborators");
5153
$table->addColumn('album_id', Types::BIGINT, [
5254
'notnull' => true,
@@ -56,21 +58,26 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
5658
'notnull' => true,
5759
'length' => 64,
5860
]);
59-
$table->addColumn('collaborator_source', Types::INTEGER, [
61+
$table->addColumn('collaborator_type', Types::INTEGER, [
6062
'notnull' => true,
6163
]);
6264

63-
$table->addUniqueConstraint(['album_id', 'collaborator_id', 'collaborator_source'], 'collaborators_unique_idx');
65+
$table->addUniqueConstraint(['album_id', 'collaborator_id', 'collaborator_type'], 'collaborators_unique_idx');
6466
}
6567

6668
if (!$schema->getTable("photos_albums_files")->hasColumn("owner")) {
69+
$modified = true;
6770
$table = $schema->getTable("photos_albums_files");
6871
$table->addColumn('owner', Types::STRING, [
6972
'notnull' => true,
7073
'length' => 64,
7174
]);
7275
}
7376

74-
return $schema;
77+
if ($modified) {
78+
return $schema;
79+
} else {
80+
return null;
81+
}
7582
}
7683
}

‎lib/Sabre/Album/AlbumPhoto.php‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,27 +36,27 @@
3636
class AlbumPhoto implements IFile {
3737
private AlbumMapper $albumMapper;
3838
private AlbumInfo $album;
39-
private AlbumFile $file;
39+
private AlbumFile $albumFile;
4040
private IRootFolder $rootFolder;
4141

4242
public const TAG_FAVORITE = '_$!<Favorite>!$_';
4343

44-
public function __construct(AlbumMapper $albumMapper, AlbumInfo $album, AlbumFile $file, IRootFolder $rootFolder) {
44+
public function __construct(AlbumMapper $albumMapper, AlbumInfo $album, AlbumFile $albumFile, IRootFolder $rootFolder) {
4545
$this->albumMapper = $albumMapper;
4646
$this->album = $album;
47-
$this->file = $file;
47+
$this->albumFile = $albumFile;
4848
$this->rootFolder = $rootFolder;
4949
}
5050

5151
/**
5252
* @return void
5353
*/
5454
public function delete() {
55-
$this->albumMapper->removeFile($this->album->getId(), $this->file->getFileId());
55+
$this->albumMapper->removeFile($this->album->getId(), $this->albumFile->getFileId());
5656
}
5757

5858
public function getName() {
59-
return $this->file->getFileId() . "-" . $this->file->getName();
59+
return $this->albumFile->getFileId() . "-" . $this->albumFile->getName();
6060
}
6161

6262
/**
@@ -67,7 +67,7 @@ public function setName($name) {
6767
}
6868

6969
public function getLastModified() {
70-
return $this->file->getMTime();
70+
return $this->albumFile->getMTime();
7171
}
7272

7373
public function put($data) {
@@ -76,8 +76,8 @@ public function put($data) {
7676

7777
public function get() {
7878
$nodes = $this->rootFolder
79-
->getUserFolder($this->file->getOwner())
80-
->getById($this->file->getFileId());
79+
->getUserFolder($this->albumFile->getOwner() || $this->album->getUserId())
80+
->getById($this->albumFile->getFileId());
8181
$node = current($nodes);
8282
if ($node) {
8383
/** @var Node $node */
@@ -92,13 +92,13 @@ public function get() {
9292
}
9393

9494
public function getFileId(): int {
95-
return $this->file->getFileId();
95+
return $this->albumFile->getFileId();
9696
}
9797

9898
public function getFileInfo(): Node {
9999
$nodes = $this->rootFolder
100-
->getUserFolder($this->file->getOwner())
101-
->getById($this->file->getFileId());
100+
->getUserFolder($this->albumFile->getOwner() ?? $this->album->getUserId())
101+
->getById($this->albumFile->getFileId());
102102
$node = current($nodes);
103103
if ($node) {
104104
return $node;
@@ -108,19 +108,19 @@ public function getFileInfo(): Node {
108108
}
109109

110110
public function getContentType() {
111-
return $this->file->getMimeType();
111+
return $this->albumFile->getMimeType();
112112
}
113113

114114
public function getETag() {
115-
return $this->file->getEtag();
115+
return $this->albumFile->getEtag();
116116
}
117117

118118
public function getSize() {
119-
return $this->file->getSize();
119+
return $this->albumFile->getSize();
120120
}
121121

122122
public function getFile(): AlbumFile {
123-
return $this->file;
123+
return $this->albumFile;
124124
}
125125

126126
public function isFavorite(): bool {
@@ -141,9 +141,9 @@ public function setFavoriteState($favoriteState): bool {
141141

142142
switch ($favoriteState) {
143143
case "0":
144-
return $tagger->removeFromFavorites($this->file->getFileId());
144+
return $tagger->removeFromFavorites($this->albumFile->getFileId());
145145
case "1":
146-
return $tagger->addToFavorites($this->file->getFileId());
146+
return $tagger->addToFavorites($this->albumFile->getFileId());
147147
default:
148148
new \Exception('Favorite state is invalide, should be 0 or 1.');
149149
}

0 commit comments

Comments
 (0)