Apply suggestions

Signed-off-by: Louis Chemineau <louis@chmn.me>

lib/Controller/PublicAlbumController.php

@@ -63,14 +63,13 @@ public function __construct(
 	 * @NoCSRFRequired
 	 */
 	public function get(): TemplateResponse {
-		$this->eventDispatcher->dispatch(LoadSidebar::class, new LoadSidebar());
 		$this->eventDispatcher->dispatch(LoadViewer::class, new LoadViewer());
 
 		$this->initialState->provideInitialState('image-mimes', Application::IMAGE_MIMES);
 		$this->initialState->provideInitialState('video-mimes', Application::VIDEO_MIMES);
-		$this->initialState->provideInitialState('maps', $this->appManager->isEnabledForUser('maps') === true);
-		$this->initialState->provideInitialState('recognize', $this->appManager->isEnabledForUser('recognize') === true);
-		$this->initialState->provideInitialState('systemtags', $this->appManager->isEnabledForUser('systemtags') === true);
+		$this->initialState->provideInitialState('maps', false);
+		$this->initialState->provideInitialState('recognize', false);
+		$this->initialState->provideInitialState('systemtags', false);
 
 		// Provide user config
 		foreach (array_keys(UserConfigService::DEFAULT_CONFIGS) as $key) {

lib/Sabre/Album/AlbumPhoto.php

@@ -152,11 +152,9 @@ public function setFavoriteState($favoriteState): bool {
 
 		switch ($favoriteState) {
 			case "0":
-				$tagger->removeFromFavorites($this->albumFile->getFileId());
-				return "0";
+				return $tagger->removeFromFavorites($this->albumFile->getFileId());
 			case "1":
-				$tagger->addToFavorites($this->albumFile->getFileId());
-				return "1";
+				return $tagger->addToFavorites($this->albumFile->getFileId());
 			default:
 				new \Exception('Favorite state is invalide, should be 0 or 1.');
 		}

lib/Sabre/Album/AlbumsHome.php

@@ -42,6 +42,8 @@ class AlbumsHome implements ICollection {
 	protected Folder $userFolder;
 	protected UserConfigService $userConfigService;
 
+	public const NAME = 'albums';
+
 	/**
 	 * @var AlbumRoot[]
 	 */
@@ -70,7 +72,7 @@ public function delete() {
 	}
 
 	public function getName(): string {
-		return 'albums';
+		return self::NAME;
 	}
 
 	/**

lib/Sabre/Album/PublicAlbumsHome.php

@@ -28,9 +28,7 @@
 use OCA\Photos\Album\AlbumMapper;
 
 class PublicAlbumsHome extends AlbumsHome {
-	public function getName(): string {
-		return 'public';
-	}
+	public const NAME = 'public';
 
 	/**
 	 * @return never

lib/Sabre/Album/SharedAlbumsHome.php

@@ -34,6 +34,8 @@
 class SharedAlbumsHome extends AlbumsHome {
 	private IGroupManager $groupManager;
 
+	public const NAME = 'sharedalbums';
+
 	public function __construct(
 		array $principalInfo,
 		AlbumMapper $albumMapper,
@@ -53,10 +55,6 @@ public function __construct(
 		$this->groupManager = $groupManager;
 	}
 
-	public function getName(): string {
-		return 'sharedalbums';
-	}
-
 	/**
 	 * @return never
 	 */

lib/Sabre/PhotosHome.php

@@ -91,11 +91,11 @@ public function createDirectory($name) {
 
 	public function getChild($name) {
 		switch ($name) {
-			case 'albums':
+			case AlbumsHome::NAME:
 				return new AlbumsHome($this->principalInfo, $this->albumMapper, $this->user, $this->rootFolder, $this->userConfigService);
-			case 'sharedalbums':
+			case SharedAlbumsHome::NAME:
 				return new SharedAlbumsHome($this->principalInfo, $this->albumMapper, $this->user, $this->rootFolder, $this->groupManager, $this->userConfigService);
-			case 'public':
+			case PublicAlbumsHome::NAME:
 				return new PublicAlbumsHome($this->principalInfo, $this->albumMapper, $this->user, $this->rootFolder, $this->userConfigService);
 		}
 
@@ -114,7 +114,7 @@ public function getChildren(): array {
 	}
 
 	public function childExists($name): bool {
-		return $name === 'albums' || $name === 'sharedalbums' || $name === 'public';
+		return $name === AlbumsHome::NAME || $name === SharedAlbumsHome::NAME || $name === PublicAlbumsHome::NAME;
 	}
 
 	public function getLastModified(): int {

src/Photos.vue

@@ -173,9 +173,9 @@ export default {
 				}), {
 					scope: generateUrl('/apps/photos'),
 				}).then(registration => {
-					logger.debug('SW registered: ', registration)
+					logger.debug('SW registered: ', { registration })
 				}).catch(registrationError => {
-					logger.error('SW registration failed: ', registrationError)
+					logger.error('SW registration failed: ', { registrationError })
 				})
 
 			})

src/PhotosPublic.vue

@@ -67,9 +67,9 @@ export default {
 				try {
 					const url = generateUrl('/apps/photos/service-worker.js', {}, { noRewrite: true })
 					const registration = await navigator.serviceWorker.register(url, { scope: generateUrl('/apps/photos') })
-					logger.debug('SW registered: ', registration)
+					logger.debug('SW registered: ', { registration })
 				} catch (error) {
-					logger.error('SW registration failed: ', error)
+					logger.error('SW registration failed: ', { error })
 				}
 			})
 		} else {

src/components/Albums/CollaboratorsSelectionForm.vue

@@ -358,7 +358,7 @@ export default {
 					this.errorFetchingAlbum = error
 				}
 
-				logger.error('[PublicAlbumContent] Error fetching album', error)
+				logger.error('[PublicAlbumContent] Error fetching album', {error})
 				showError(this.t('photos', 'Failed to fetch album.'))
 			} finally {
 				this.loadingAlbum = false
@@ -385,7 +385,7 @@ export default {
 					},
 				})
 			} catch (error) {
-				logger.error('[PublicAlbumContent] Error updating album', error)
+				logger.error('[PublicAlbumContent] Error updating album', {error})
 				showError(this.t('photos', 'Failed to update album.'))
 			} finally {
 				this.loadingAlbum = false

src/components/TiledLayout/TiledLayout.vue

@@ -65,7 +65,7 @@ export default {
 	computed: {
 		/** @return {import('../services/TiledLayout.js').TiledRow[]} */
 		rows() {
-			logger.debug('[TiledLayout] Computing rows', this.items)
+			logger.debug('[TiledLayout] Computing rows', { items: this.items })
 
 			return splitItemsInRows(this.items, this.containerWidth, this.baseHeight)
 		},

src/mixins/FetchFacesMixin.js

@@ -88,7 +88,7 @@ export default {
 						this.errorFetchingFaces = error
 					}
 				}
-				logger.error(t('photos', 'Failed to fetch faces list.'), error)
+				logger.error(t('photos', 'Failed to fetch faces list.'), { error })
 				showError(t('photos', 'Failed to fetch faces list.'))
 			} finally {
 				this.loadingFaces = false
@@ -140,7 +140,7 @@ export default {
 				}
 
 				// cancelled request, moving on...
-				logger.error('Error fetching face files', error)
+				logger.error('Error fetching face files', { error })
 			} finally {
 				this.loadingFiles = false
 			}

src/mixins/FetchFilesMixin.js

@@ -110,7 +110,7 @@ export default {
 				}
 
 				// cancelled request, moving on...
-				logger.error('Error fetching files', error)
+				logger.error('Error fetching files', { error })
 				console.error(error)
 			} finally {
 				this.loadingFiles = false

src/services/Albums.js

@@ -39,34 +39,43 @@ import { genFileInfo } from '../utils/fileUtils.js'
  * @property {number} cover - The cover of the album.
  */
 
+/**
+ * @param {string} extraProps - Extra properties to add to the DAV request.
+ * @return {string}
+ */
+function getDavRequest(extraProps = '') {
+	return `<?xml version="1.0"?>
+			<d:propfind xmlns:d="DAV:"
+				xmlns:oc="http://owncloud.org/ns"
+				xmlns:nc="http://nextcloud.org/ns"
+				xmlns:ocs="http://open-collaboration-services.org/ns">
+				<d:prop>
+					<nc:last-photo />
+					<nc:nbItems />
+					<nc:location />
+					<nc:dateRange />
+					<nc:collaborators />
+					${extraProps}
+				</d:prop>
+			</d:propfind>`
+}
+
 /**
  *
  * @param {string} path - Albums' root path.
  * @param {import('webdav').StatOptions} options - Options to forward to the webdav client.
+ * @param {string} extraProps - Extra properties to add to the DAV request.
  * @return {Promise<Album|null>}
  */
 export async function fetchAlbum(path, options, extraProps = '') {
 	try {
 		const response = await client.stat(path, {
-			data: `<?xml version="1.0"?>
-							<d:propfind xmlns:d="DAV:"
-								xmlns:oc="http://owncloud.org/ns"
-								xmlns:nc="http://nextcloud.org/ns"
-								xmlns:ocs="http://open-collaboration-services.org/ns">
-								<d:prop>
-									<nc:last-photo />
-									<nc:nbItems />
-									<nc:location />
-									<nc:dateRange />
-									<nc:collaborators />
-									${extraProps}
-								</d:prop>
-							</d:propfind>`,
+			data: getDavRequest(extraProps),
 			details: true,
 			...options,
 		})
 
-		logger.debug('[Albums] Fetched an album: ', response.data)
+		logger.debug('[Albums] Fetched an album: ', { data: response.data })
 
 		return formatAlbum(response.data)
 	} catch (error) {
@@ -87,24 +96,12 @@ export async function fetchAlbum(path, options, extraProps = '') {
 export async function fetchAlbums(path, options) {
 	try {
 		const response = await client.getDirectoryContents(path, {
-			data: `<?xml version="1.0"?>
-							<d:propfind xmlns:d="DAV:"
-								xmlns:oc="http://owncloud.org/ns"
-								xmlns:nc="http://nextcloud.org/ns"
-								xmlns:ocs="http://open-collaboration-services.org/ns">
-								<d:prop>
-									<nc:last-photo />
-									<nc:nbItems />
-									<nc:location />
-									<nc:dateRange />
-									<nc:collaborators />
-								</d:prop>
-							</d:propfind>`,
+			data: getDavRequest(),
 			details: true,
 			...options,
 		})
 
-		logger.debug(`[Albums] Fetched ${response.data.length} albums: `, response.data)
+		logger.debug(`[Albums] Fetched ${response.data.length} albums: `, { data: response.data })
 
 		return response.data
 			.filter(album => album.filename !== path)
@@ -183,7 +180,7 @@ export async function fetchAlbumContent(path, options) {
 			return []
 		}
 
-		logger.error('Error fetching album files', error)
+		logger.error('Error fetching album files', { error })
 		console.error(error)
 
 		throw error

src/store/albums.js

@@ -159,7 +159,7 @@ const actions = {
 					if (error.response.status !== 409) { // Already in the album.
 						context.commit('removeFilesFromAlbum', { albumName, fileIdsToRemove: [fileId] })
 
-						logger.error(t('photos', 'Failed to add {fileBaseName} to album {albumName}.', { fileBaseName: file.basename, albumName }), error)
+						logger.error(t('photos', 'Failed to add {fileBaseName} to album {albumName}.', { fileBaseName: file.basename, albumName }), { error })
 						showError(t('photos', 'Failed to add {fileBaseName} to album {albumName}.', { fileBaseName: file.basename, albumName }))
 					}
 				} finally {
@@ -193,7 +193,7 @@ const actions = {
 				} catch (error) {
 					context.commit('addFilesToAlbum', { albumName, fileIdsToAdd: [fileId] })
 
-					logger.error(t('photos', 'Failed to delete {fileBaseName}.', { fileBaseName: file.basename }), error)
+					logger.error(t('photos', 'Failed to delete {fileBaseName}.', { fileBaseName: file.basename }), { error })
 					showError(t('photos', 'Failed to delete {fileBaseName}.', { fileBaseName: file.basename }))
 				} finally {
 					semaphore.release(symbol)
@@ -216,7 +216,7 @@ const actions = {
 			context.commit('addAlbums', { albums: [album] })
 			return album
 		} catch (error) {
-			logger.error(t('photos', 'Failed to create {albumName}.', { albumName: album.basename }), error)
+			logger.error(t('photos', 'Failed to create {albumName}.', { albumName: album.basename }), { error })
 			showError(t('photos', 'Failed to create {albumName}.', { albumName: album.basename }))
 		}
 	},
@@ -244,7 +244,7 @@ const actions = {
 			return newAlbum
 		} catch (error) {
 			context.commit('removeAlbums', { albumNames: [newAlbumName] })
-			logger.error(t('photos', 'Failed to rename {currentAlbumName} to {newAlbumName}.', { currentAlbumName, newAlbumName }), error)
+			logger.error(t('photos', 'Failed to rename {currentAlbumName} to {newAlbumName}.', { currentAlbumName, newAlbumName }), { error })
 			showError(t('photos', 'Failed to rename {currentAlbumName} to {newAlbumName}.', { currentAlbumName, newAlbumName }))
 			return album
 		}
@@ -301,7 +301,7 @@ const actions = {
 			return updatedAlbum
 		} catch (error) {
 			context.commit('updateAlbum', { album })
-			logger.error(t('photos', 'Failed to update properties of {albumName} with {properties}.', { albumName, properties: JSON.stringify(properties) }), error)
+			logger.error(t('photos', 'Failed to update properties of {albumName} with {properties}.', { albumName, properties: JSON.stringify(properties) }), { error })
 			showError(t('photos', 'Failed to update properties of {albumName} with {properties}.', { albumName, properties: JSON.stringify(properties) }))
 			return album
 		}
@@ -320,7 +320,7 @@ const actions = {
 			await client.deleteFile(album.filename)
 			context.commit('removeAlbums', { albumNames: [albumName] })
 		} catch (error) {
-			logger.error(t('photos', 'Failed to delete {albumName}.', { albumName }), error)
+			logger.error(t('photos', 'Failed to delete {albumName}.', { albumName }), { error })
 			showError(t('photos', 'Failed to delete {albumName}.', { albumName }))
 		}
 	},

src/store/collectionStoreFactory.js

@@ -77,10 +77,7 @@ export default function collectionStoreFactory(collectionName) {
 			const collectionFiles = state[`${collectionName}sFiles`][collectionId] || []
 			state[`${collectionName}sFiles`] = {
 				...state[`${collectionName}sFiles`],
-				[collectionId]: [
-					...collectionFiles,
-					...fileIdsToAdd.filter(fileId => !collectionFiles.includes(fileId)), // Filter to prevent duplicate fileId.
-				],
+				[collectionId]: [...new Set([...collectionFiles, ...fileIdsToAdd])],
 			}
 			state[`${collectionName}s`][collectionId].nbItems += fileIdsToAdd.length
 		},
@@ -147,7 +144,7 @@ export default function collectionStoreFactory(collectionName) {
 						if (error.response.status !== 409) { // Already in the collection.
 							context.commit(`removeFilesFrom${capitalizedCollectionName}`, { collectionId, fileIdsToRemove: [fileId] })
 
-							logger.error(translate('photos', 'Failed to add {fileBaseName} to {collectionId}.', { fileBaseName: file.basename, collectionId }), error)
+							logger.error(translate('photos', 'Failed to add {fileBaseName} to {collectionId}.', { fileBaseName: file.basename, collectionId }), { error })
 							showError(translate('photos', 'Failed to add {fileBaseName} to {collectionId}.', { fileBaseName: file.basename, collectionId }))
 						}
 					} finally {
@@ -181,7 +178,7 @@ export default function collectionStoreFactory(collectionName) {
 					} catch (error) {
 						context.commit(`addFilesTo${capitalizedCollectionName}`, { collectionId, fileIdsToAdd: [fileId] })
 
-						logger.error(translate('photos', 'Failed to delete {fileBaseName}.', { fileBaseName: file.basename }), error)
+						logger.error(translate('photos', 'Failed to delete {fileBaseName}.', { fileBaseName: file.basename }), { error })
 						showError(translate('photos', 'Failed to delete {fileBaseName}.', { fileBaseName: file.basename }))
 					} finally {
 						semaphore.release(symbol)
@@ -204,7 +201,7 @@ export default function collectionStoreFactory(collectionName) {
 				await client.deleteFile(collection.filename)
 				context.commit(`remove${capitalizedCollectionName}s`, { collectionIds: [collectionId] })
 			} catch (error) {
-				logger.error(translate('photos', 'Failed to delete {collectionId}.', { collectionId }), error)
+				logger.error(translate('photos', 'Failed to delete {collectionId}.', { collectionId }), { error })
 				showError(translate('photos', 'Failed to delete {collectionId}.', { collectionId }))
 			}
 		},