Skip to content

Conversation

toby-griffiths
Copy link
Contributor

@toby-griffiths toby-griffiths commented Oct 23, 2020

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tickets n/a
License MIT
Doc PR api-platform/docs#1194

I've had need of a paginator for my custom data provider that returns a Generator, so I thought I'd submit this as an addition to the core, if that's so desired?

  • Add tests and ensure they pass.
  • Never break backward compatibility.
  • Features and deprecations must be submitted against master branch.
  • Update CHANGELOG.md file.
  • Submit PR for docs update.

@toby-griffiths
Copy link
Contributor Author

Not sure what to do about the failed build checks. The skipped tests that seems be be causing a failure are unrelated to my change, and now I'm seeing a Cannot update only a partial set of packages without a lock file present. error. On other projects I could click to re-run the build checks, but I don't seem to have that option here.

@soyuka soyuka added the Ready label Nov 8, 2020
@soyuka
Copy link
Member

soyuka commented Nov 8, 2020

we fixed our tests feel free to rebase to relaunch the CI

@toby-griffiths toby-griffiths force-pushed the add-traversable-paginator branch from 4b72c29 to e0f4ed6 Compare November 8, 2020 17:19
@toby-griffiths
Copy link
Contributor Author

@soyuka I've rebased & the build is still failing on SwaggerUiAction for both phpcs & phpunit checks. I can take a look at these in a fresh PR if you like?

There also looks to be a Github authentication issue?

@soyuka
Copy link
Member

soyuka commented Nov 8, 2020

Yes there's an issue with github authentification using composer 2 I haven't got the time to dig into that more :/

@toby-griffiths
Copy link
Contributor Author

I know that feeling. If I can find a little time I'll see if I can help out.

Would you be OK to add the hacktoberfest-accepted label in the meantime, as your approval doesn't seem to have registered on my Hacktober profile?

@toby-griffiths
Copy link
Contributor Author

toby-griffiths commented Nov 14, 2020

Thank you @soyuka . Looks like it needs to be added by a maintainer though, on reading the rules 😏


use Traversable;

class TraversablePaginator implements \IteratorAggregate, PaginatorInterface
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
class TraversablePaginator implements \IteratorAggregate, PaginatorInterface
final class TraversablePaginator implements \IteratorAggregate, PaginatorInterface

CHANGELOG.md Outdated
* IriConverter: Fix IRI url double encoding - may cause breaking change as some characters no longer encoded in output (#3552)
* OpenAPI: **BC** Replace all characters other than `[a-zA-Z0-9\.\-_]` to `.` in definition names to be compliant with OpenAPI 3.0 (#3669)
* Add stateless ApiResource attribute
* Add TraversablePaginator
Copy link
Member

Choose a reason for hiding this comment

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

Can you add it to a new 2.7.0 section please? We'll merge this new feature in the next version.

public function getLastPage(): float
{
if (0. === $this->itemsPerPage) {
return 1;
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
return 1;
return 1.;

private $itemsPerPage;
private $totalItems;

public function __construct(Traversable $iterator, float $currentPage, float $itemsPerPage, float $totalItems)
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
public function __construct(Traversable $iterator, float $currentPage, float $itemsPerPage, float $totalItems)
public function __construct(\Traversable $iterator, float $currentPage, float $itemsPerPage, float $totalItems)


namespace ApiPlatform\Core\DataProvider;

use Traversable;
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
use Traversable;

Base automatically changed from master to main January 23, 2021 21:59
@alanpoulain alanpoulain force-pushed the add-traversable-paginator branch from e0f4ed6 to 87fee54 Compare February 17, 2021 17:42
@alanpoulain alanpoulain merged commit 3557329 into api-platform:main Feb 17, 2021
@alanpoulain
Copy link
Member

Thank you @toby-griffiths!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants