Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

API client replacement #20

Closed
wants to merge 9 commits into from
Closed

API client replacement #20

wants to merge 9 commits into from

Conversation

mmenozzi
Copy link
Member

This PR will replace the API client using the official Akeneo PIM PHP SDK. This PR contains BC breaks!

@mmenozzi mmenozzi added this to the 2.0 milestone Nov 17, 2020
@mmenozzi
Copy link
Member Author

This PR makes #10 outdated and probably not needed.

@aleho
Copy link
Contributor

aleho commented Jan 16, 2021

I'd very much like to see something like that!

I've come up with the only solution of extending the API client with additional filters to getIdentifiersModifiedSince, so I can handle completeness.

$products = $this->apiClient->findProductsModifiedSince($sinceDate, [
    'enabled' => [
        ['operator' => '=', 'value' => true],
    ],
    'completeness' => [
        ['operator' => '>=', 'value' => 100, 'scope' => 'e4o'],
    ],
]);

The API client for this proof of concept was changed like this:

public function findProductsModifiedSince(\DateTime $date, array $filters = []): array
{
    $search = [
        'updated' => [
            ['operator' => '>', 'value' => $date->format('Y-m-d H:i:s')]
        ],
    ] + $filters;

    $endpoint = sprintf(
        '/api/rest/v1/products?search=%s&limit=20&page=1',
        json_encode($search)
    );

    return $this->traversePagination($this->authenticatedRequest($endpoint, 'GET', []));
}

I think this PR wouldn't change much, except for changing the signature to public function getIdentifiersModifiedSince(\DateTime $sinceDate, callable $queryExtension): array and passing the $searchBuilder to $queryExtension, if it exists.

For my use case that would be enough as a first step and I wouldn't even need to decorate the API client, as decorating the Importer and passing the extender callback would be enough.

What do you think?

Edit: Oh, and btw, can I help you with anything?

@mmenozzi
Copy link
Member Author

Hi @aleho,
decorating/replacing the API client is the only way for now. Changing public method signature with a new, required, argument is a BC break so we cannot release in 1.x.
Anyway if you look at a way to customize the way items are enqueued the right issue to work on is #5.
This PR regards the entire API client replacement which is a great improvement but will also introduce BC breaks so we cannot release in 1.x as well.

If you want to help with anything you can start by looking at the list of "help wanted" issues. Those with the "RFC" label require further discussion.

@aleho
Copy link
Contributor

aleho commented Jan 18, 2021

Yes, of course, I saw this PR as a BC break anyways.

I just wanted to let you know that a query extension point for the importer (for the time being) would be a nice addition to this PR.

lruozzi9 added a commit that referenced this pull request Mar 28, 2022
lruozzi9 added a commit that referenced this pull request Mar 28, 2022
@lruozzi9 lruozzi9 self-assigned this Mar 28, 2022
@lruozzi9 lruozzi9 mentioned this pull request Mar 28, 2022
@lruozzi9
Copy link
Member

Replaced by #125

@lruozzi9 lruozzi9 closed this Mar 28, 2022
lruozzi9 added a commit that referenced this pull request Apr 11, 2022
lruozzi9 added a commit that referenced this pull request Apr 11, 2022
lruozzi9 added a commit that referenced this pull request Apr 11, 2022
lruozzi9 added a commit that referenced this pull request Apr 11, 2022
@lruozzi9 lruozzi9 deleted the api-client-replacement branch August 9, 2023 08:13
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.

3 participants