Package search

[giggsey]

Just want a bit of feedback/advice before I continue on this:

• I'm thinking just an initial basic package name search for the time being, could be expanded to description (and maybe tags?) later
• Currently, the pagination is done as two different calls to the PackageQuery class - to avoid having to add search functionality for each, could a paginator bundle be used instead? (I come from a Laravel background where it's a core part of the framework)
• Design wise, I'm thinking that the search bar could be on the right of the title bar, to the left of 'Add package'
  
• Side note: could the hard-coded 20 per page be moved to a user setting?

[akondas]

Hey @giggsey, looks like a good idea.
When it comes to pagination with filters, I usually do it through an additional object. So you can create something like this:

final class Filter
{
    private int $offset;
    private int $limit;

    public function __construct()
    {
        $this->offset = 0;
        $this->limit = 20;
    }

    public function setOffset(int $offset): self
    {
        $this->offset = $offset;

        return $this;
    }

    public function setLimit(int $limit): self
    {
        $this->limit = $limit;

        return $this;
    }

    public function limit(): int
    {
        return $this->limit;
    }

    public function offset(): int
    {
        return $this->offset;
    }
}

(you can add $query param for serach reason)

And then add it to count and found methods in query:

interface PackageQuery
{
    public function findAll(Filter $filter): array;
    public function count(Filter $filter): int;
}

Side note about UI: here is mine proposition 😉

[giggsey]

@akondas Sounds good - I'll continue over the next few days.

[giggsey]

[giggsey]

@akondas Tests are failing because the test data is inserting NULL name & description. I don't use Postgres, and I'm not sure if there is a trick to include NULLs when the Filter searchTerm is empty.

[akondas]

I left some comments, the rest looks fine 👏

[codecov]

Codecov Report

Merging #259 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #259   +/-   ##
=========================================
  Coverage     99.81%   99.82%           
- Complexity     1522     1539   +17     
=========================================
  Files           251      252    +1     
  Lines          4434     4479   +45     
=========================================
+ Hits           4426     4471   +45     
  Misses            8        8           

Impacted Files	Coverage Δ	Complexity Δ
src/Controller/OrganizationController.php	100.00% <100.00%> (ø)	41.00 <3.00> (+2.00)
src/Query/Filter.php	100.00% <100.00%> (ø)	13.00 <13.00> (?)
src/Query/User/PackageQuery/DbalPackageQuery.php	100.00% <100.00%> (ø)	25.00 <4.00> (+2.00)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97de038...43ae73f. Read the comment docs.

[akondas]

Nice 👍
Do you want to try to write a functional test for this functionality? (you can check OrganizationControllerTest::testPackages should be very similar, also in test you can use $this->fixtures->syncPackageWithData to populate package with given name)

[giggsey]

Sure, I'll look at adding the tests.

[akondas]

Thanks 🍻 @giggsey, this will also fix error with negative offset 😉

[giggsey]

@akondas There are other places that use pagination, but are still doing it manually. Want me to convert them over to Filter (no need to add search to them)

[akondas]

Whenever you have the opportunity, I would like to. I'm just wondering if I should make a separate filter then so that this search term is not confusing. In the sense, if a particular query does not implement a search, let it use a filter without that capability.

If so, I would suggest to do a separate filter in the namespace of Package and leave this general filter only with the limit and offset attributes.

Buddy\Repman\Query\Filter
Buddy\Repman\Query\User\PackageQuery\Filter