Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

IBX-7419: [autcomplete] Add the total number of results to the autocomplete/suggestions endpoint #38

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

kisztof
Copy link
Contributor

@kisztof kisztof commented Dec 19, 2023

Question Answer
Tickets IBX-7419
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? yes
License GPL-2.0

This PR adds new two properties in the suggestion endpoint, suggestionResults and totalCount.

http://127.0.0.1:8000/suggestion?query=ala&limit=1

{
  "suggestionResults": [
    {
      "contentId": 66,
      "locationId": 67,
      "contentTypeIdentifier": "article",
      "name": "abra ala kadabra",
      "pathString": "2/67",
      "type": "content",
      "parentLocations": [
        {
          "id": 52,
          "locationId": 2,
          "name": "Ibexa Digital Experience Platform"
        },
        { "id": 66, "locationId": 67, "name": "abra ala kadabra" }
      ]
    }
  ],
  "resultsCount": 3
}

The capability to adjust the totalCount via an extension point can be achieved through the use of \Ibexa\Contracts\Search\Model\Suggestion\SuggestionCollection::increaseTotalCount or
\Ibexa\Contracts\Search\Model\Suggestion\SuggestionCollection::decreaseTotalCount

Checklist:

  • Implement tests
  • Coding standards ($ composer fix-cs)

@kisztof kisztof requested a review from a team December 19, 2023 10:12
@@ -47,4 +56,19 @@ public function truncate(int $count): void
{
$this->items = array_slice($this->items, 0, $count);
}

public function increaseTotalCount(int $totalCount): void
Copy link
Contributor

@ViniTou ViniTou Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't increasing totalCount when using append() be less error prone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is needed because of multiple data sources. When the limit value (for example, 5) is passed, only 5 records will be inserted using the append method. However, it is necessary to include the total number of records that the query finds.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I realised that this is probably the case later in the day, wonder how it will work with pagination tho (offset), but I guess this is not something we need (or at all if this works properly), now.

Comment on lines +25 to +27
parent::__construct($items);
$this->items = $items;
$this->totalCount = $totalCount;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parent::__construct($items);
$this->items = $items;
$this->totalCount = $totalCount;
parent::__construct($items);
$this->items = $items;
$this->totalCount = $totalCount;


public function decreaseTotalCount(int $totalCount): void
{
$this->totalCount -= $totalCount;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps this will not be the case but maybe something should happen when $this->totalCount will be lower than 0

@bogusez
Copy link

bogusez commented Jan 4, 2024

@kisztof the backend endpoint (suggestion) does not return searched contents - frontend does.

Screenshot 2024-01-04 at 13 11 10 Screenshot 2024-01-04 at 13 19 13

kisztof and others added 2 commits January 8, 2024 14:50
Copy link

sonarqubecloud bot commented Jan 8, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@bogusez
Copy link

bogusez commented Jan 8, 2024

@kisztof the backend endpoint (suggestion) does not return searched contents - frontend does.

Screenshot 2024-01-04 at 13 11 10 Screenshot 2024-01-04 at 13 19 13

As discussed with @webhdx missing authentication causes that problem with empty table. It's not an issue.

Copy link

@bogusez bogusez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regression tests passed.

@webhdx webhdx merged commit c33343b into main Jan 8, 2024
9 checks passed
@webhdx webhdx deleted the IBX-7419 branch January 8, 2024 14:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants