Skip to content

Commit

Permalink
imp: Ignore case when searching for tags
Browse files Browse the repository at this point in the history
  • Loading branch information
marienfressinaud committed Oct 31, 2024
1 parent 4faef23 commit b8e5554
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

namespace App\migrations;

use App\models;

class Migration202410310002ChangeLinksTagsStorageFormat
{
public function migrate(): bool
{
$database = \Minz\Database::get();

$statement = $database->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;
}
}
18 changes: 18 additions & 0 deletions src/models/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/search_engine/LinksSearcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions src/services/LinkTags.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
6 changes: 3 additions & 3 deletions tests/controllers/MessagesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/controllers/links/CollectionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/controllers/links/MessagesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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
Expand Down
17 changes: 17 additions & 0 deletions tests/search_engine/LinksSearcherTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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[] */
Expand Down

0 comments on commit b8e5554

Please sign in to comment.