Skip to content

[stable31] fix(systemtags): case-insensitive search & prevent duplicates #53888

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

Merged
merged 1 commit into from
Jul 10, 2025
Merged

[stable31] fix(systemtags): case-insensitive search & prevent duplicates #53888

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
54 changes: 54 additions & 0 deletions cypress/e2e/systemtags/files-bulk-action.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,4 +411,58 @@ describe('Systemtags: Files bulk action', { testIsolation: false }, () => {
cy.runOccCommand('config:app:set systemtags restrict_creation_to_admin --value 0')
})
})

it('Can search for tags with insensitive case', () => {
let tagId: string
resetTags()

cy.runOccCommand('tag:add TESTTAG public --output json').then(({ stdout }) => {
const tag = JSON.parse(stdout)
tagId = tag.id
})

cy.createRandomUser().then((user1) => {
files.forEach((file) => {
cy.uploadContent(user1, new Blob([]), 'text/plain', '/' + file)
})

cy.login(user1)
cy.visit('/apps/files')

files.forEach((file) => {
getRowForFile(file).should('be.visible')
})
selectAllFiles()

triggerTagManagementDialogAction()

cy.findByRole('textbox', { name: 'Search or create tag' }).should('be.visible')
cy.findByRole('textbox', { name: 'Search tag' }).should('not.exist')

cy.get('[data-cy-systemtags-picker-input]').type('testtag')

cy.get('[data-cy-systemtags-picker-tag]').should('have.length', 1)
cy.get(`[data-cy-systemtags-picker-tag="${tagId}"]`).should('be.visible')
.findByRole('checkbox').should('not.be.checked')

// Assign the tag
cy.intercept('PROPFIND', '/remote.php/dav/systemtags/*/files').as('getTagData')
cy.intercept('PROPPATCH', '/remote.php/dav/systemtags/*/files').as('assignTagData')

cy.get(`[data-cy-systemtags-picker-tag="${tagId}"]`).should('be.visible')
.findByRole('checkbox').click({ force: true })
cy.get('[data-cy-systemtags-picker-button-submit]').click()

cy.wait('@getTagData')
cy.wait('@assignTagData')

expectInlineTagForFile('file1.txt', ['TESTTAG'])
expectInlineTagForFile('file2.txt', ['TESTTAG'])
expectInlineTagForFile('file3.txt', ['TESTTAG'])
expectInlineTagForFile('file4.txt', ['TESTTAG'])
expectInlineTagForFile('file5.txt', ['TESTTAG'])

cy.get('[data-cy-systemtags-picker]').should('not.exist')
})
})
})
21 changes: 19 additions & 2 deletions lib/private/SystemTag/SystemTagManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function getAllTags($visibilityFilter = null, $nameSearchPattern = null):

if (!empty($nameSearchPattern)) {
$query->andWhere(
$query->expr()->like(
$query->expr()->iLike(
'name',
$query->createNamedParameter('%' . $this->connection->escapeLikeParameter($nameSearchPattern) . '%')
)
Expand All @@ -120,7 +120,7 @@ public function getAllTags($visibilityFilter = null, $nameSearchPattern = null):
->addOrderBy('visibility', 'ASC')
->addOrderBy('editable', 'ASC');

$result = $query->execute();
$result = $query->executeQuery();
while ($row = $result->fetch()) {
$tags[$row['id']] = $this->createSystemTagFromRow($row);
}
Expand Down Expand Up @@ -156,6 +156,14 @@ public function createTag(string $tagName, bool $userVisible, bool $userAssignab
throw new TagCreationForbiddenException();
}

// Check if tag already exists (case-insensitive)
$existingTags = $this->getAllTags(null, $tagName);
foreach ($existingTags as $existingTag) {
if (mb_strtolower($existingTag->getName()) === mb_strtolower($tagName)) {
throw new TagAlreadyExistsException('Tag ' . $tagName . ' already exists');
}
}

// Length of name column is 64
$truncatedTagName = substr($tagName, 0, 64);
$query = $this->connection->getQueryBuilder();
Expand Down Expand Up @@ -226,6 +234,15 @@ public function updateTag(
$color
);

// Check if tag already exists (case-insensitive)
$existingTags = $this->getAllTags(null, $truncatedNewName);
foreach ($existingTags as $existingTag) {
if (mb_strtolower($existingTag->getName()) === mb_strtolower($truncatedNewName)
&& $existingTag->getId() !== $tagId) {
throw new TagAlreadyExistsException('Tag ' . $truncatedNewName . ' already exists');
}
}

$query = $this->connection->getQueryBuilder();
$query->update(self::TAG_TABLE)
->set('name', $query->createParameter('name'))
Expand Down
61 changes: 24 additions & 37 deletions tests/lib/SystemTag/SystemTagManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use OCP\IUserSession;
use OCP\SystemTag\ISystemTag;
use OCP\SystemTag\ISystemTagManager;
use OCP\SystemTag\TagAlreadyExistsException;
use Test\TestCase;

/**
Expand Down Expand Up @@ -79,17 +80,6 @@ public static function getAllTagsDataProvider() {
['two', false, false],
]
],
[
// duplicate names, different flags
[
['one', false, false],
['one', true, false],
['one', false, true],
['one', true, true],
['two', false, false],
['two', false, true],
]
]
];
}

Expand Down Expand Up @@ -162,14 +152,14 @@ public static function getAllTagsFilteredDataProvider() {
[
[
['one', true, false],
['one', false, false],
['one_different', false, false],
['two', true, false],
],
null,
'on',
[
['one', true, false],
['one', false, false],
['one_different', false, false],
]
],
// filter by name pattern and visibility
Expand All @@ -178,7 +168,7 @@ public static function getAllTagsFilteredDataProvider() {
[
['one', true, false],
['two', true, false],
['one', false, false],
['one_different', false, false],
],
true,
'on',
Expand Down Expand Up @@ -239,7 +229,7 @@ public static function oneTagMultipleFlagsProvider() {
* @dataProvider oneTagMultipleFlagsProvider
*/
public function testCreateDuplicate($name, $userVisible, $userAssignable): void {
$this->expectException(\OCP\SystemTag\TagAlreadyExistsException::class);
$this->expectException(TagAlreadyExistsException::class);

try {
$this->tagManager->createTag($name, $userVisible, $userAssignable);
Expand All @@ -249,6 +239,15 @@ public function testCreateDuplicate($name, $userVisible, $userAssignable): void
$this->tagManager->createTag($name, $userVisible, $userAssignable);
}

public function testCreateDuplicateWithDifferentFlags(): void {
$this->expectException(TagAlreadyExistsException::class);

// Create a tag with specific flags
$this->tagManager->createTag('duplicate', true, false);
// Try to create a tag with the same name but different flags - should fail
$this->tagManager->createTag('duplicate', false, true);
}

public function testCreateOverlongName(): void {
$tag = $this->tagManager->createTag('Zona circundante do Palácio Nacional da Ajuda (Jardim das Damas, Salão de Física, Torre Sineira, Paço Velho e Jardim Botânico)', true, true);
$this->assertSame('Zona circundante do Palácio Nacional da Ajuda (Jardim das Damas', $tag->getName()); // 63 characters but 64 bytes due to "á"
Expand Down Expand Up @@ -356,32 +355,20 @@ public function testUpdateTag($tagCreate, $tagUpdated): void {

}

/**
* @dataProvider updateTagProvider
*/
public function testUpdateTagDuplicate($tagCreate, $tagUpdated): void {
$this->expectException(\OCP\SystemTag\TagAlreadyExistsException::class);
public function testUpdateTagToExistingName(): void {
$this->expectException(TagAlreadyExistsException::class);

$this->tagManager->createTag(
$tagCreate[0],
$tagCreate[1],
$tagCreate[2],
$tagCreate[3],
);
$tag2 = $this->tagManager->createTag(
$tagUpdated[0],
$tagUpdated[1],
$tagUpdated[2],
$tagUpdated[3],
);
// Create two different tags
$tag1 = $this->tagManager->createTag('first', true, true);
$tag2 = $this->tagManager->createTag('second', false, false);

// update to match the first tag
// Try to update tag2 to have the same name as tag1 - should fail
$this->tagManager->updateTag(
$tag2->getId(),
$tagCreate[0],
$tagCreate[1],
$tagCreate[2],
$tagCreate[3],
'first',
false,
false,
null
);
}

Expand Down
Loading