Skip to content

Commit

Permalink
fix: Exclude links only in the never list from the search
Browse files Browse the repository at this point in the history
  • Loading branch information
marienfressinaud committed Oct 7, 2022
1 parent 649c0f3 commit b80eb73
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 4 deletions.
11 changes: 9 additions & 2 deletions src/controllers/Links.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,14 @@ public function index($request)
$pagination_page = $request->paramInteger('page', 1);

if ($query) {
$number_links = models\Link::daoCall('countByQueryAndUserId', $query, $user->id);
$number_links = models\Link::daoCall(
'countByQueryAndUserId',
$query,
$user->id,
[
'exclude_never_only' => true,
]
);
$number_per_page = 30;
$pagination = new utils\Pagination($number_links, $number_per_page, $pagination_page);
if ($pagination_page !== $pagination->currentPage()) {
Expand All @@ -55,9 +62,9 @@ public function index($request)
$user->id,
['published_at', 'number_comments'],
[
'exclude_never_only' => true,
'offset' => $pagination->currentOffset(),
'limit' => $pagination->numberPerPage(),
'context_user_id' => $user->id,
]
);

Expand Down
48 changes: 47 additions & 1 deletion src/models/dao/links/SearchQueries.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ trait SearchQueries
* properties.
* @param array $options
* Custom options to filter links. Possible options are:
* - exclude_never_only (boolean, default to false), whether we should
* exclude links that are only in a "never" collection
* - offset (integer, default to 0), the offset for pagination
* - limit (integer|string, default to 'ALL') the limit for pagination
*
Expand All @@ -38,6 +40,7 @@ trait SearchQueries
public function listComputedByQueryAndUserId($query, $user_id, $selected_computed_props, $options = [])
{
$default_options = [
'exclude_never_only' => false,
'offset' => 0,
'limit' => 'ALL',
];
Expand Down Expand Up @@ -66,6 +69,22 @@ public function listComputedByQueryAndUserId($query, $user_id, $selected_compute
SQL;
}

$exclude_clause = '';
if ($options['exclude_never_only']) {
$exclude_clause = <<<'SQL'
AND NOT EXISTS (
SELECT 1
FROM links_to_collections lc, collections c
WHERE lc.link_id = l.id
AND lc.collection_id = c.id
HAVING COUNT(CASE WHEN c.type='never' THEN 1 END) = 1
AND COUNT(c.*) = 1
)
SQL;
}

$limit_clause = '';
if ($options['limit'] !== 'ALL') {
$limit_clause = 'LIMIT :limit';
Expand All @@ -81,6 +100,7 @@ public function listComputedByQueryAndUserId($query, $user_id, $selected_compute
WHERE l.user_id = :user_id
AND search_index @@ query
{$exclude_clause}
{$order_by_clause}
OFFSET :offset
Expand All @@ -99,22 +119,48 @@ public function listComputedByQueryAndUserId($query, $user_id, $selected_compute
* The query to search for.
* @param string $user_id
* The user id the links must match.
* @param array $options
* Custom options to filter links. Possible option is:
* - exclude_never_only (boolean, default to false), whether we should
* exclude links that are only in a "never" collection
*
* @return integer
*/
public function countByQueryAndUserId($query, $user_id)
public function countByQueryAndUserId($query, $user_id, $options = [])
{
$default_options = [
'exclude_never_only' => false,
];
$options = array_merge($default_options, $options);

$parameters = [
':user_id' => $user_id,
':query' => $query,
];

$exclude_clause = '';
if ($options['exclude_never_only']) {
$exclude_clause = <<<'SQL'
AND NOT EXISTS (
SELECT 1
FROM links_to_collections lc, collections c
WHERE lc.link_id = l.id
AND lc.collection_id = c.id
HAVING COUNT(CASE WHEN c.type='never' THEN 1 END) = 1
AND COUNT(c.*) = 1
)
SQL;
}

$sql = <<<SQL
SELECT COUNT(l.id)
FROM links l, plainto_tsquery('french', :query) AS query
WHERE l.user_id = :user_id
AND search_index @@ query
{$exclude_clause}
SQL;

$statement = $this->prepare($sql);
Expand Down
50 changes: 49 additions & 1 deletion tests/models/dao/links/SearchQueriesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,31 @@ public function testSearchComputedByUserIdCanOffsetResults()
$this->assertSame($link_id_1, $db_links[1]['id']);
}

public function testCountByUserId()
public function testSearchComputedByUserIdCanExcludeLinksOnlyInNeverCollection()
{
$dao = new dao\Link();
$title = $this->fake('sentence', 10, false);
$query = $title;
$user_id = $this->create('user');
$user = models\User::find($user_id);
$never_list = $user->neverList();
$link_id = $this->create('link', [
'user_id' => $user_id,
'title' => $title,
]);
$this->create('link_to_collection', [
'collection_id' => $never_list->id,
'link_id' => $link_id,
]);

$db_links = $dao->listComputedByQueryAndUserId($query, $user_id, [], [
'exclude_never_only' => true,
]);

$this->assertSame(0, count($db_links));
}

public function testCountByQueryAndUserId()
{
$dao = new dao\Link();
$title = $this->fake('sentence', 10, false);
Expand All @@ -197,4 +221,28 @@ public function testCountByUserId()

$this->assertSame(1, $count);
}

public function testCountByQueryAndUserIdCanExcludeLinksOnlyInNeverCollection()
{
$dao = new dao\Link();
$title = $this->fake('sentence', 10, false);
$query = $title;
$user_id = $this->create('user');
$user = models\User::find($user_id);
$never_list = $user->neverList();
$link_id = $this->create('link', [
'user_id' => $user_id,
'title' => $title,
]);
$this->create('link_to_collection', [
'collection_id' => $never_list->id,
'link_id' => $link_id,
]);

$count = $dao->countByQueryAndUserId($query, $user_id, [
'exclude_never_only' => true,
]);

$this->assertSame(0, $count);
}
}

0 comments on commit b80eb73

Please sign in to comment.