Improve coding standards

[norkunas]

Pull Request

Related issue

Fixes #<issue_number>

What does this PR do?

• Add missing phpdocs;
• Replace DateTime with DateTimeinterface to encourage usage of DateTimeImmutable;
• Add more tests for contracts;
• Make tests a little bit more consistent;
• Add missing php typehints;

PR checklist

Please check if your PR fulfills the following requirements:

• Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• Have you read the contributing guidelines?
• Have you made sure that the title is accurate and descriptive of the changes?

[norkunas]

I cannot understand if delete tasks query test was properly defined or not because https://www.meilisearch.com/docs/reference/api/tasks#delete-tasks does not indicate the cancelledBy query param type 😑

1) Tests\Contracts\DeleteTasksQueryTest::testSetAnyDateFilter
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
     'beforeEnqueuedAt' => '2024-09-25T04:08:50+00:00'
+    'canceledBy' => ''
 )

/home/runner/work/meilisearch-php/meilisearch-php/tests/Contracts/DeleteTasksQueryTest.php:24
phpvfscomposer:///home/runner/work/meilisearch-php/meilisearch-php/vendor/phpunit/phpunit/phpunit:106

But to me it does not make any sense $data = (new DeleteTasksQuery())->setCanceledBy([null])->setBeforeEnqueuedAt($date); to pass null as array item

[curquiza]

I don't understand @norkunas, why would you pass null as an array? What's the purpose of this PR?

Also, maybe the docs contains mistakes, but canceledBy is here: https://www.meilisearch.com/docs/reference/api/tasks#canceledby

[norkunas]

I don't understand @norkunas, why would you pass null as an array?

Not null as array, but null as item in array, see the previous code: (new DeleteTasksQuery())->setCanceledBy([null])

What's the purpose of this PR?

To make codebase consistent?

Also, maybe the docs contains mistakes, but canceledBy is here: https://www.meilisearch.com/docs/reference/api/tasks#canceledby

Ok so it accepts an integer and if multiple then it needs to be joined by comma. I've made the contract to explicitly say non-empty-array<int>

[curquiza]

Not null as array, but null as item in array, see the previous code: (new DeleteTasksQuery())->setCanceledBy([null])

Yes sorry, I got it but not said it correctly. But why do you need to pass null? This is not the purpose of canceling a task. the purpose is to pass the uid of the task(s) to cancel them

[norkunas]

Not null as array, but null as item in array, see the previous code: (new DeleteTasksQuery())->setCanceledBy([null])

Yes sorry, I got it but not said it correctly. But why do you need to pass null? This is not the purpose of canceling a task. the purpose is to pass the uid of the task(s) to cancel them

I don't. That was the existing test which is nonsense

[curquiza]

I don't. That was the existing test which is nonsense

Indeed, sounds weird to me, we can change it/remove it.

[codecov]

Codecov Report

Attention: Patch coverage is 98.15951% with 3 lines in your changes missing coverage. Please review.

Project coverage is 89.71%. Comparing base (b82d40e) to head (55dc98f).
Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Contracts/DeleteTasksQuery.php	66.66%	1 Missing ⚠️
src/Contracts/TasksQuery.php	85.71%	1 Missing ⚠️
src/Endpoints/Indexes.php	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #684      +/-   ##
==========================================
+ Coverage   83.34%   89.71%   +6.36%     
==========================================
  Files          51       52       +1     
  Lines        1303     1322      +19     
==========================================
+ Hits         1086     1186     +100     
+ Misses        217      136      -81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[curquiza]

@norkunas can you improve

• the title of the PR
• describes in some points (not a lot of details) what you are achieving in this PR?

There are a lot of changes

[norkunas]

@norkunas can you improve

• the title of the PR
• describes in some points (not a lot of details) what you are achieving in this PR?

There are a lot of changes

Done

[curquiza]

bors merge

[meili-bors]

Build succeeded:

• integration-tests (PHP 7.4) (Guzzle-7)
• integration-tests (PHP 7.4) (Guzzle-7-Adapter)
• integration-tests (PHP 7.4) (Kriswallsmith-Buzz)
• integration-tests (PHP 7.4) (PHP-HTTP-CurlClient)
• integration-tests (PHP 8.0) (Guzzle-7)
• integration-tests (PHP 8.0) (Guzzle-7-Adapter)
• integration-tests (PHP 8.0) (Kriswallsmith-Buzz)
• integration-tests (PHP 8.0) (PHP-HTTP-CurlClient)
• integration-tests (PHP 8.1) (Guzzle-7)
• integration-tests (PHP 8.1) (Guzzle-7-Adapter)
• integration-tests (PHP 8.1) (Kriswallsmith-Buzz)
• integration-tests (PHP 8.1) (PHP-HTTP-CurlClient)
• integration-tests (PHP 8.2) (Guzzle-7)
• integration-tests (PHP 8.2) (Guzzle-7-Adapter)
• integration-tests (PHP 8.2) (Kriswallsmith-Buzz)
• integration-tests (PHP 8.2) (PHP-HTTP-CurlClient)
• integration-tests (PHP 8.2) (Symfony-HttpClient)
• integration-tests (PHP 8.3) (Guzzle-7)
• integration-tests (PHP 8.3) (Guzzle-7-Adapter)
• integration-tests (PHP 8.3) (Kriswallsmith-Buzz)
• integration-tests (PHP 8.3) (PHP-HTTP-CurlClient)
• integration-tests (PHP 8.3) (Symfony-HttpClient)
• linter-check
• phpstan-tests