Merge pull request #49295 from nextcloud/feat/tags-colors

apps/dav/lib/SystemTag/SystemTagNode.php

@@ -13,6 +13,7 @@
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\TagAlreadyExistsException;
 use OCP\SystemTag\TagNotFoundException;
+use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\Conflict;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\MethodNotAllowed;
@@ -86,12 +87,13 @@ public function setName($name) {
 	 * @param string $name new tag name
 	 * @param bool $userVisible user visible
 	 * @param bool $userAssignable user assignable
+	 * @param string $color color
 	 *
 	 * @throws NotFound whenever the given tag id does not exist
 	 * @throws Forbidden whenever there is no permission to update said tag
 	 * @throws Conflict whenever a tag already exists with the given attributes
 	 */
-	public function update($name, $userVisible, $userAssignable): void {
+	public function update($name, $userVisible, $userAssignable, $color): void {
 		try {
 			if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) {
 				throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist');
@@ -110,7 +112,12 @@ public function update($name, $userVisible, $userAssignable): void {
 				}
 			}
 
-			$this->tagManager->updateTag($this->tag->getId(), $name, $userVisible, $userAssignable);
+			// Make sure color is a proper hex
+			if ($color !== null && (strlen($color) !== 6 || !ctype_xdigit($color))) {
+				throw new BadRequest('Color must be a 6-digit hexadecimal value');
+			}
+
+			$this->tagManager->updateTag($this->tag->getId(), $name, $userVisible, $userAssignable, $color);
 		} catch (TagNotFoundException $e) {
 			throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist');
 		} catch (TagAlreadyExistsException $e) {

apps/dav/lib/SystemTag/SystemTagPlugin.php

@@ -49,6 +49,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 	public const NUM_FILES_PROPERTYNAME = '{http://nextcloud.org/ns}files-assigned';
 	public const REFERENCE_FILEID_PROPERTYNAME = '{http://nextcloud.org/ns}reference-fileid';
 	public const OBJECTIDS_PROPERTYNAME = '{http://nextcloud.org/ns}object-ids';
+	public const COLOR_PROPERTYNAME = '{http://nextcloud.org/ns}color';
 
 	/**
 	 * @var \Sabre\DAV\Server $server
@@ -243,6 +244,10 @@ public function handleGetProperties(
 			return $this->tagManager->canUserAssignTag($node->getSystemTag(), $this->userSession->getUser()) ? 'true' : 'false';
 		});
 
+		$propFind->handle(self::COLOR_PROPERTYNAME, function () use ($node) {
+			return $node->getSystemTag()->getColor() ?? '';
+		});
+
 		$propFind->handle(self::GROUPS_PROPERTYNAME, function () use ($node) {
 			if (!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) {
 				// property only available for admins
@@ -406,6 +411,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			self::GROUPS_PROPERTYNAME,
 			self::NUM_FILES_PROPERTYNAME,
 			self::REFERENCE_FILEID_PROPERTYNAME,
+			self::COLOR_PROPERTYNAME,
 		], function ($props) use ($node) {
 			if (!($node instanceof SystemTagNode)) {
 				return false;
@@ -415,6 +421,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			$name = $tag->getName();
 			$userVisible = $tag->isUserVisible();
 			$userAssignable = $tag->isUserAssignable();
+			$color = $tag->getColor();
 
 			$updateTag = false;
 
@@ -435,6 +442,15 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 				$updateTag = true;
 			}
 
+			if (isset($props[self::COLOR_PROPERTYNAME])) {
+				$propValue = $props[self::COLOR_PROPERTYNAME];
+				if ($propValue === '' || $propValue === 'null') {
+					$propValue = null;
+				}
+				$color = $propValue;
+				$updateTag = true;
+			}
+
 			if (isset($props[self::GROUPS_PROPERTYNAME])) {
 				if (!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) {
 					// property only available for admins
@@ -452,7 +468,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			}
 
 			if ($updateTag) {
-				$node->update($name, $userVisible, $userAssignable);
+				$node->update($name, $userVisible, $userAssignable, $color);
 			}
 
 			return true;

apps/dav/lib/SystemTag/SystemTagsInUseCollection.php

@@ -73,7 +73,7 @@ public function getChildren(): array {
 		$result = $this->systemTagsInFilesDetector->detectAssignedSystemTagsIn($userFolder, $this->mediaType);
 		$children = [];
 		foreach ($result as $tagData) {
-			$tag = new SystemTag((string)$tagData['id'], $tagData['name'], (bool)$tagData['visibility'], (bool)$tagData['editable'], $tagData['etag']);
+			$tag = new SystemTag((string)$tagData['id'], $tagData['name'], (bool)$tagData['visibility'], (bool)$tagData['editable'], $tagData['etag'], $tagData['color']);
 			// read only, so we can submit the isAdmin parameter as false generally
 			$node = new SystemTagNode($tag, $user, false, $this->systemTagManager, $this->tagMapper);
 			$node->setNumberOfFiles((int)$tagData['number_files']);

apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php

@@ -85,19 +85,19 @@ public function tagNodeProvider() {
 			[
 				true,
 				new SystemTag(1, 'Original', true, true),
-				['Renamed', true, true]
+				['Renamed', true, true, null]
 			],
 			[
 				true,
 				new SystemTag(1, 'Original', true, true),
-				['Original', false, false]
+				['Original', false, false, null]
 			],
 			// non-admin
 			[
 				// renaming allowed
 				false,
 				new SystemTag(1, 'Original', true, true),
-				['Rename', true, true]
+				['Rename', true, true, '0082c9']
 			],
 		];
 	}
@@ -116,47 +116,47 @@ public function testUpdateTag($isAdmin, ISystemTag $originalTag, $changedArgs):
 			->willReturn($originalTag->isUserAssignable() || $isAdmin);
 		$this->tagManager->expects($this->once())
 			->method('updateTag')
-			->with(1, $changedArgs[0], $changedArgs[1], $changedArgs[2]);
+			->with(1, $changedArgs[0], $changedArgs[1], $changedArgs[2], $changedArgs[3]);
 		$this->getTagNode($isAdmin, $originalTag)
-			->update($changedArgs[0], $changedArgs[1], $changedArgs[2]);
+			->update($changedArgs[0], $changedArgs[1], $changedArgs[2], $changedArgs[3]);
 	}
 
 	public function tagNodeProviderPermissionException() {
 		return [
 			[
 				// changing permissions not allowed
 				new SystemTag(1, 'Original', true, true),
-				['Original', false, true],
+				['Original', false, true, ''],
 				'Sabre\DAV\Exception\Forbidden',
 			],
 			[
 				// changing permissions not allowed
 				new SystemTag(1, 'Original', true, true),
-				['Original', true, false],
+				['Original', true, false, ''],
 				'Sabre\DAV\Exception\Forbidden',
 			],
 			[
 				// changing permissions not allowed
 				new SystemTag(1, 'Original', true, true),
-				['Original', false, false],
+				['Original', false, false, ''],
 				'Sabre\DAV\Exception\Forbidden',
 			],
 			[
 				// changing non-assignable not allowed
 				new SystemTag(1, 'Original', true, false),
-				['Rename', true, false],
+				['Rename', true, false, ''],
 				'Sabre\DAV\Exception\Forbidden',
 			],
 			[
 				// changing non-assignable not allowed
 				new SystemTag(1, 'Original', true, false),
-				['Original', true, true],
+				['Original', true, true, ''],
 				'Sabre\DAV\Exception\Forbidden',
 			],
 			[
 				// invisible tag does not exist
 				new SystemTag(1, 'Original', false, false),
-				['Rename', false, false],
+				['Rename', false, false, ''],
 				'Sabre\DAV\Exception\NotFound',
 			],
 		];
@@ -181,7 +181,7 @@ public function testUpdateTagPermissionException(ISystemTag $originalTag, $chang
 
 		try {
 			$this->getTagNode(false, $originalTag)
-				->update($changedArgs[0], $changedArgs[1], $changedArgs[2]);
+				->update($changedArgs[0], $changedArgs[1], $changedArgs[2], $changedArgs[3]);
 		} catch (\Exception $e) {
 			$thrown = $e;
 		}
@@ -206,7 +206,7 @@ public function testUpdateTagAlreadyExists(): void {
 			->method('updateTag')
 			->with(1, 'Renamed', true, true)
 			->will($this->throwException(new TagAlreadyExistsException()));
-		$this->getTagNode(false, $tag)->update('Renamed', true, true);
+		$this->getTagNode(false, $tag)->update('Renamed', true, true, null);
 	}
 
 
@@ -226,7 +226,7 @@ public function testUpdateTagNotFound(): void {
 			->method('updateTag')
 			->with(1, 'Renamed', true, true)
 			->will($this->throwException(new TagNotFoundException()));
-		$this->getTagNode(false, $tag)->update('Renamed', true, true);
+		$this->getTagNode(false, $tag)->update('Renamed', true, true, null);
 	}
 
 	/**

apps/systemtags/appinfo/info.xml

@@ -11,7 +11,7 @@
 	<summary>Collaborative tagging functionality which shares tags among people.</summary>
 	<description>Collaborative tagging functionality which shares tags among people. Great for teams.
 	(If you are a provider with a multi-tenancy installation, it is advised to deactivate this app as tags are shared.)</description>
-	<version>1.21.0</version>
+	<version>1.21.1</version>
 	<licence>agpl</licence>
 	<author>Vincent Petry</author>
 	<author>Joas Schilling</author>

apps/systemtags/composer/composer/autoload_classmap.php

@@ -16,6 +16,8 @@
     'OCA\\SystemTags\\Listeners\\BeforeSabrePubliclyLoadedListener' => $baseDir . '/../lib/Listeners/BeforeSabrePubliclyLoadedListener.php',
     'OCA\\SystemTags\\Listeners\\BeforeTemplateRenderedListener' => $baseDir . '/../lib/Listeners/BeforeTemplateRenderedListener.php',
     'OCA\\SystemTags\\Listeners\\LoadAdditionalScriptsListener' => $baseDir . '/../lib/Listeners/LoadAdditionalScriptsListener.php',
+    'OCA\\SystemTags\\Migration\\Version31000Date20241018063111' => $baseDir . '/../lib/Migration/Version31000Date20241018063111.php',
+    'OCA\\SystemTags\\Migration\\Version31000Date20241114171300' => $baseDir . '/../lib/Migration/Version31000Date20241114171300.php',
     'OCA\\SystemTags\\Search\\TagSearchProvider' => $baseDir . '/../lib/Search/TagSearchProvider.php',
     'OCA\\SystemTags\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
 );

apps/systemtags/composer/composer/autoload_static.php

@@ -31,6 +31,8 @@ class ComposerStaticInitSystemTags
         'OCA\\SystemTags\\Listeners\\BeforeSabrePubliclyLoadedListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeSabrePubliclyLoadedListener.php',
         'OCA\\SystemTags\\Listeners\\BeforeTemplateRenderedListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeTemplateRenderedListener.php',
         'OCA\\SystemTags\\Listeners\\LoadAdditionalScriptsListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadAdditionalScriptsListener.php',
+        'OCA\\SystemTags\\Migration\\Version31000Date20241018063111' => __DIR__ . '/..' . '/../lib/Migration/Version31000Date20241018063111.php',
+        'OCA\\SystemTags\\Migration\\Version31000Date20241114171300' => __DIR__ . '/..' . '/../lib/Migration/Version31000Date20241114171300.php',
         'OCA\\SystemTags\\Search\\TagSearchProvider' => __DIR__ . '/..' . '/../lib/Search/TagSearchProvider.php',
         'OCA\\SystemTags\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
     );

core/Migrations/Version31000Date20241018063111.php → apps/systemtags/lib/Migration/Version31000Date20241018063111.php

@@ -7,7 +7,7 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-namespace OC\Core\Migrations;
+namespace OCA\SystemTags\Migration;
 
 use Closure;
 use Doctrine\DBAL\Types\Types;
@@ -26,12 +26,6 @@
 #[AddIndex(table: 'systemtag_object_mapping', type: IndexType::INDEX, description: 'Adding objecttype index to systemtag_object_mapping')]
 class Version31000Date20241018063111 extends SimpleMigrationStep {
 
-	/**
-	 * @param IOutput $output
-	 * @param Closure(): ISchemaWrapper $schemaClosure
-	 * @param array $options
-	 * @return null|ISchemaWrapper
-	 */
 	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
 		/** @var ISchemaWrapper $schema */
 		$schema = $schemaClosure();

apps/systemtags/lib/Migration/Version31000Date20241114171300.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\SystemTags\Migration;
+
+use Closure;
+use Doctrine\DBAL\Types\Types;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\Attributes\AddColumn;
+use OCP\Migration\Attributes\ColumnType;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * Add objecttype index to systemtag_object_mapping
+ */
+#[AddColumn(table: 'systemtag', name: 'color', type: ColumnType::STRING, description: 'Adding color for systemtag table')]
+class Version31000Date20241114171300 extends SimpleMigrationStep {
+
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if ($schema->hasTable('systemtag')) {
+			$table = $schema->getTable('systemtag');
+
+			if (!$table->hasColumn('color')) {
+				$table->addColumn('color', Types::STRING, [
+					'notnull' => false,
+					'length' => 6,
+				]);
+			}
+		}
+
+		return $schema;
+	}
+}