From b8e555449fd22fc7dbec28c9d99b2f8e0c83b45d Mon Sep 17 00:00:00 2001 From: Marien Fressinaud Date: Thu, 31 Oct 2024 17:40:43 +0100 Subject: [PATCH] imp: Ignore case when searching for tags --- ...2410310002ChangeLinksTagsStorageFormat.php | 56 +++++++++++++++++++ src/models/Link.php | 18 ++++++ src/search_engine/LinksSearcher.php | 2 +- src/services/LinkTags.php | 6 +- tests/controllers/MessagesTest.php | 6 +- tests/controllers/links/CollectionsTest.php | 4 +- tests/controllers/links/MessagesTest.php | 4 +- tests/search_engine/LinksSearcherTest.php | 17 ++++++ 8 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 src/migrations/Migration202410310002ChangeLinksTagsStorageFormat.php diff --git a/src/migrations/Migration202410310002ChangeLinksTagsStorageFormat.php b/src/migrations/Migration202410310002ChangeLinksTagsStorageFormat.php new file mode 100644 index 00000000..12d2cd96 --- /dev/null +++ b/src/migrations/Migration202410310002ChangeLinksTagsStorageFormat.php @@ -0,0 +1,56 @@ +query(<<<'SQL' + SELECT l.* FROM links l + WHERE jsonb_typeof(l.tags) = 'array' + AND jsonb_array_length(l.tags) > 0; + SQL); + $db_links = $statement->fetchAll(); + $links = models\Link::fromDatabaseRows($db_links); + + $database->beginTransaction(); + + foreach ($links as $link) { + $tags = $link->tags; + $link->setTags($tags); + $link->save(); + } + + $database->commit(); + + return true; + } + + public function rollback(): bool + { + $database = \Minz\Database::get(); + + $statement = $database->query(<<<'SQL' + SELECT l.* FROM links l + WHERE jsonb_typeof(l.tags) = 'object'; + SQL); + $db_links = $statement->fetchAll(); + $links = models\Link::fromDatabaseRows($db_links); + + $database->beginTransaction(); + + foreach ($links as $link) { + $link->tags = array_values($link->tags); + $link->save(); + } + + $database->commit(); + + return true; + } +} diff --git a/src/models/Link.php b/src/models/Link.php index 99467492..8a80c0e9 100644 --- a/src/models/Link.php +++ b/src/models/Link.php @@ -121,6 +121,24 @@ public function __construct(string $url, string $user_id, bool $is_hidden) $this->user_id = $user_id; } + /** + * @param string[] $tags + */ + public function setTags(array $tags): void + { + $sanitized_tags = []; + + foreach ($tags as $tag) { + $lower_tag = mb_strtolower($tag); + + if (!isset($sanitized_tags[$lower_tag])) { + $sanitized_tags[$lower_tag] = $tag; + } + } + + $this->tags = $sanitized_tags; + } + /** * Copy a Link to the given user. */ diff --git a/src/search_engine/LinksSearcher.php b/src/search_engine/LinksSearcher.php index 64453a4e..c2c25385 100644 --- a/src/search_engine/LinksSearcher.php +++ b/src/search_engine/LinksSearcher.php @@ -180,7 +180,7 @@ private static function buildWhereQuery(Query $query): array $parameter_name = ':tag' . (count($parameters) + 1); - $parameters[$parameter_name] = $value; + $parameters[$parameter_name] = mb_strtolower($value); if ($condition->not()) { $not_tags_parameters[] = $parameter_name; diff --git a/src/services/LinkTags.php b/src/services/LinkTags.php index 32133cd3..9456e01d 100644 --- a/src/services/LinkTags.php +++ b/src/services/LinkTags.php @@ -23,11 +23,7 @@ public static function refresh(models\Link $link): void $tags = array_merge($tags, $message_tags); } - $tags = array_unique($tags); - $tags = array_values($tags); - - $link->tags = $tags; - + $link->setTags($tags); $link->save(); } diff --git a/tests/controllers/MessagesTest.php b/tests/controllers/MessagesTest.php index 8897033c..8d0856e4 100644 --- a/tests/controllers/MessagesTest.php +++ b/tests/controllers/MessagesTest.php @@ -116,10 +116,10 @@ public function testUpdateChangesLinkTags(): void { $user = $this->login(); $link = LinkFactory::create([ - 'tags' => ['foo'], + 'tags' => ['foo' => 'foo'], ]); $old_content = '#foo'; - $new_content = '#bar'; + $new_content = '#Bar'; $message = MessageFactory::create([ 'user_id' => $user->id, 'link_id' => $link->id, @@ -137,7 +137,7 @@ public function testUpdateChangesLinkTags(): void $message = $message->reload(); $this->assertSame($new_content, $message->content); $link = $link->reload(); - $this->assertEquals(['bar'], $link->tags); + $this->assertEquals(['bar' => 'Bar'], $link->tags); } public function testUpdateRedirectsToLoginIfNotConnected(): void diff --git a/tests/controllers/links/CollectionsTest.php b/tests/controllers/links/CollectionsTest.php index 3be6d426..ddebcadc 100644 --- a/tests/controllers/links/CollectionsTest.php +++ b/tests/controllers/links/CollectionsTest.php @@ -461,7 +461,7 @@ public function testUpdateChangesTags(): void 'user_id' => $user->id, 'type' => 'collection', ]); - $comment = '#foo #bar'; + $comment = '#foo #Bar'; $response = $this->appRun('POST', "/links/{$link->id}/collections", [ 'csrf' => $user->csrf, @@ -471,7 +471,7 @@ public function testUpdateChangesTags(): void ]); $link = $link->reload(); - $this->assertEquals(['foo', 'bar'], $link->tags); + $this->assertEquals(['foo' => 'foo', 'bar' => 'Bar'], $link->tags); } public function testUpdateCanMarkAsRead(): void diff --git a/tests/controllers/links/MessagesTest.php b/tests/controllers/links/MessagesTest.php index 5981c8b0..75bf5452 100644 --- a/tests/controllers/links/MessagesTest.php +++ b/tests/controllers/links/MessagesTest.php @@ -55,7 +55,7 @@ public function testCreateChangesLinkTags(): void 'user_id' => $user->id, 'tags' => [], ]); - $content = '#foo #bar'; + $content = '#foo #Bar'; $this->assertSame(0, models\Message::count()); @@ -66,7 +66,7 @@ public function testCreateChangesLinkTags(): void $this->assertResponseCode($response, 302, "/links/{$link->id}"); $link = $link->reload(); - $this->assertEquals(['foo', 'bar'], $link->tags); + $this->assertEquals(['foo' => 'foo', 'bar' => 'Bar'], $link->tags); } public function testCreateWorksIfLinkIsInCollectionSharedWithWriteAccess(): void diff --git a/tests/search_engine/LinksSearcherTest.php b/tests/search_engine/LinksSearcherTest.php index 5e4a0763..af997cc3 100644 --- a/tests/search_engine/LinksSearcherTest.php +++ b/tests/search_engine/LinksSearcherTest.php @@ -66,6 +66,23 @@ public function testGetLinksSearchesByTag(): void $this->assertSame($link->id, $links[0]->id); } + public function testGetLinksSearchesByTagIgnoringCase(): void + { + $tags = ['foo' => 'FOO', 'bar' => 'BAR']; + $tag = 'Foo'; + $user = UserFactory::create(); + $link = LinkFactory::create([ + 'user_id' => $user->id, + 'tags' => $tags, + ]); + $query = Query::fromString("#{$tag}"); + + $links = LinksSearcher::getLinks($user, $query); + + $this->assertSame(1, count($links)); + $this->assertSame($link->id, $links[0]->id); + } + public function testGetLinksCanExcludeByTag(): void { /** @var string[] */