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

add simple product name and description search #564

Open
wants to merge 2 commits into
base: 1.5
Choose a base branch
from

Conversation

CSchulz
Copy link
Contributor

@CSchulz CSchulz commented Sep 22, 2019

Simple implementation for a product name and description search.

I don't like the always full blown output for a search f.e. if you have a search bar with result overlay, you don't need all details.

@CSchulz CSchulz requested a review from a team as a code owner September 22, 2019 15:41
doc/swagger.yml Show resolved Hide resolved
@CSchulz
Copy link
Contributor Author

CSchulz commented Sep 23, 2019

I would like also to integrate a grid (#49) but I have no clue how to connect it with an api action.

@mamazu
Copy link
Member

mamazu commented Sep 23, 2019

I would like also to integrate a grid (#49) but I have no clue how to connect it with an api action.

It should work out of the box if configured correctly. Have a look at how the Admin Api handles it.

@mezzat11
Copy link

@mamazu when it will be merged ?

@lchrusciel
Copy link
Member

It will be merged after 1.0 release (what means ~ the end of the next month)

@mathieudureisseix
Copy link

Hi,
Do you know when the feature will be merge ?
Best regards
Mathieu

@diimpp
Copy link
Member

diimpp commented Jul 23, 2020

If it will be merged, then it will be part of BC promise, which is not optimal as it really should be implemented via filters system, rather than separate endpoint.

Great PR though. 👍

@mamazu
Copy link
Member

mamazu commented Jul 23, 2020

Yeah this makes much more sense. We just need to copy over the route config from the admin bundle and we are basically done.

@diimpp
Copy link
Member

diimpp commented Jul 23, 2020

@mamazu

Personally I was thinking of ShopApiPlugin/Request filters system, where only goal is to convert querystring arguments into Command/Query variables. A sort of mini-library to handle querystring.

Here is pseudo-code to illustrate the idea.

class ShowProductsRequest
{
    use FiltersTrait;

    /** @var string */
   private $channelCode;

    /** @var string */
   private $localeCode;

   public __construct(Request $request)
   {
        // Should be handled via channelContext/localeContext and channel/locale aware CommandProvider/QueryProvider.
        $this->channelCode = $request->get('channel');        
        $this->localeCode = $request->get('locale');

        $this->filters->addDefinition(new BoolFilter($publicPropertyName, $privatePropertyName, $required, $whatever));
        $this->filters->addDefinition(new SearchFilter('search', ['name', 'description']));

        $this->filters->process($request);

        // At this point ShowProductsRequest->filters should be validateable as regular request property. It should be possible to provide set of predefined filter validators.
    }

    public function getQuery(): QueryInterface
    {
        return new ShowProductsQuery($this->channelCode, $this->localeCode, $this->filters->toArray())
    }
}

At ViewRepository/QueryHandler/CommandHandler it should be possible to write an answering part, that will understand $filters format and will do automatic filtering via QueryBuillder, if necessary.

So I think something along those lines will keep plugin true to CQS idea and at the same time will take some manual load from programmer.

Grid system is alright as well, though I've only escaped serialization hell and don't really want it too be back with ShopApiPlugin. Maybe two sets of endpoints? Casual with grids for people who don't care about those endpoints and Query/QueryHandler as alternative? For example, second set will be useful for people who is using denormalized data from Algolia/ElasticSearch.

@mamazu
Copy link
Member

mamazu commented Jul 25, 2020

Building another filter based action (despite the fact that we have all of the features in the resource bundle) doesn't really make sense, in my opinion. If we'd build something like that we should definitely only build an adapter for the ResourceProvider in the resource bundle. And we already have the endpoints that are generated by the resource bundle (eg. the address book).

Personalizing the output of it is indeed a problem. The projects that I use the resource bundle in we replaced the ViewFactory to also accept serialization data, which I think is the best solution.

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

Successfully merging this pull request may close these issues.

6 participants