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

Allow applying custom filters on base combobox element #699

Conversation

svenmuennich
Copy link
Contributor

Description

Why is it necessary?

Currently it is not possible to apply a filter on a select element, when adding one to the plugin config using the short store syntax (e.g. 'store' => 'Shopware.apps.Base.store.Dispatch'). However, since e.g. the dispatch base store applies a filter on the active field by default, it is desirable to be able to override those default filters in a plugin config.

What does it improve?

This PR changes the Shopware.apps.Base.view.element.Select element to check for an optional filter option and, if set, clears all filters from the store and applies the passed filters. Furthermore the remoteFilter property is set to true in all ExtJS base stores, whose corresponding controller actions allow passing a filter parameter. Finally, the default filter on dispatches.active added in Shopware_Controllers_Backend_Base::getDispatchesAction() is removed and instead a filter on active is added to Shopware.apps.Base.store.Dispatch. This allows to override the filter and keeps backwards compatibility of the Shopware.apps.Base.store.Dispatch at the same time. However, it changes the default behaviour Shopware_Controllers_Backend_Base::getDispatchesAction() to respond all dispatch methods instead of only active ones.

Does it have side effects?

The following base stores will use remote filtering by default:

  • Shopware.apps.Base.store.Country
  • Shopware.apps.Base.store.CountryArea
  • Shopware.apps.Base.store.CountryState
  • Shopware.apps.Base.store.Currency
  • Shopware.apps.Base.store.CustomerGroup
  • Shopware.apps.Base.store.Dispatch
  • Shopware.apps.Base.store.Locale
  • Shopware.apps.Base.store.OrderStatus
  • Shopware.apps.Base.store.Payment
  • Shopware.apps.Base.store.PaymentStatus
  • Shopware.apps.Base.store.PositionStatus
  • Shopware.apps.Base.store.Tax

Furthermore Shopware_Controllers_Backend_Base::getDispatchesAction() will now respond all dispatch methods, instead of only active ones.

Questions Answers
BC breaks? no
Tests pass? yes
Related tickets?
How to test? Create a plugin containing a config form select element with e.g. the store Shopware.apps.Base.store.Language and a filter of [['property' => 'name', 'value' => 'English']]. The select element will only show the store named English.

Currently it is not possible to apply a filter on a `select` element, when adding one to the plugin config using the short store syntax (e.g. `'store' => 'Shopware.apps.Base.store.Dispatch'`). However, since e.g. the dispatch base store applies a filter on the `active` field by default, it is desirable to be able to override those default filters in a plugin config.
This commit changes the `Shopware.apps.Base.view.element.Select` element to check for an optional `filter` option and, if set, clears all filters from the store and applies the passed filters. Furthermore the `remoteFilter` property is set to `true` in all ExtJS base stores, whose corresponding controller actions allow passing a `filter` parameter. Finally, the  default filter on `dispatches.active` added in `Shopware_Controllers_Backend_Base::getDispatchesAction()` is removed and instead a filter on `active` is added to `Shopware.apps.Base.store.Dispatch`. This allows to override the filter and keeps backwards compatibility of the `Shopware.apps.Base.store.Dispatch` at the same time. However, it changes the default behaviour `Shopware_Controllers_Backend_Base::getDispatchesAction()` to respond all dispatch methods instead of only active ones.
@shopwareBot
Copy link

Hello,

thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/#/issues/SW-16346.

Please use this issue to track the state of your pull request.

@@ -52,6 +52,7 @@ Ext.define('Shopware.apps.Base.store.PositionStatus', {
alternateClassName: 'Shopware.store.PositionStatus',
storeId: 'base.PositionStatus',
pageSize: 1000,
remoteFilter: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

getDetailStatus doesn't support filter or sort parameter

$query = $repository->getDispatchesQuery(
$filters,
$this->Request()->getParam('sort', array()),
$this->prefixKeys($this->Request()->getParam('filter', array()), 'dispatches'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this prefix function. It is not necessary to prefix each filter field if only one table is selected.
Otherwise you have to prefix all filters in all actions which you marked now as remoteFilter in the ext js stores.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for replying so late. I saw that my PR was already merged into 5.2 via 39e84ad (this PR is still open due to a broken URL in the commit message). However, since you changed this particular part to not prefix the filter params, Shopware_Controllers_Backend_Base ::getDispatchesAction() throws an exception when loading the base.Dispatch store:

[Semantical Error] line 0, col 171 near 'active = :ac': Error: 'active' is not defined. in vendor/doctrine/orm/lib/Doctrine/ORM/Query/QueryException.php on line 63 Stack trace:
#0 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(483): Doctrine\ORM\Query\QueryException::semanticalError('line 0, col 171...', Object(Doctrine\ORM\Query\QueryException))
#1 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(586): Doctrine\ORM\Query\Parser->semanticalError(''active' is not...', Array)
#2 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(272): Doctrine\ORM\Query\Parser->processDeferredIdentificationVariables()
#3 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(351): Doctrine\ORM\Query\Parser->getAST()
#4 vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php(263): Doctrine\ORM\Query\Parser->parse()
#5 vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php(123): Doctrine\ORM\Tools\Pagination\Paginator->getCountQuery()
#6 engine/Shopware/Components/Model/ModelManager.php(191): Doctrine\ORM\Tools\Pagination\Paginator->count()
#7 engine/Shopware/Controllers/Backend/Base.php(279): Shopware\Components\Model\ModelManager->getQueryCount(Object(Doctrine\ORM\Query))
#8 engine/Library/Enlight/Controller/Action.php(159): Shopware_Controllers_Backend_Base->getDispatchesAction()
#9 engine/Library/Enlight/Controller/Dispatcher/Default.php(523): Enlight_Controller_Action->dispatch('getDispatchesAc...')
#10 engine/Library/Enlight/Controller/Front.php(223): Enlight_Controller_Dispatcher_Default->dispatch(Object(Enlight_Controller_Request_RequestHttp), Object(Enlight_Controller_Response_ResponseHttp))
#11 engine/Shopware/Kernel.php(177): Enlight_Controller_Front->dispatch()
#12 vendor/symfony/http-kernel/HttpCache/HttpCache.php(487): Shopware\Kernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#13 engine/Shopware/Components/HttpCache/AppCache.php(255): Symfony\Component\HttpKernel\HttpCache\HttpCache->forward(Object(Symfony\Component\HttpFoundation\Request), true, NULL)
#14 vendor/symfony/http-kernel/HttpCache/HttpCache.php(258): Shopware\Components\HttpCache\AppCache->forward(Object(Symfony\Component\HttpFoundation\Request), true)
#15 engine/Shopware/Components/HttpCache/AppCache.php(103): Symfony\Component\HttpKernel\HttpCache\HttpCache->pass(Object(Symfony\Component\HttpFoundation\Request), true)
#16 shopware.php(113): Shopware\Components\HttpCache\AppCache->handle(Object(Symfony\Component\HttpFoundation\Request))
#17 {main}

The problem is, that Shopware\Models\Dispatch\Repository::getDispatchesQueryBuilder() uses the dispatches. prefix for the SELECTed fields, so any filters must use the same prefix. I only added the prefixKeys() method to keep that prefix out of the ExtJS store and because I didn't want to change Shopware\Models\Dispatch\Repository::getDispatchesQueryBuilder() to avoid breaking changes.

Since the current 5.2 HEAD throws the above exception the merged changes should be reverted before release. I propose to merge the PR as I sent it including the prefix method to fix the error and avoid breaking changes.

Pinging @bcremer for a second opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, i deployed a fix for the issue:
shopware/shopware@4614988

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the fix 👍 I didn't know setAlias existed. This is of course much more elegant than my prefixing solution 😉

Copy link
Contributor

@ahmad-saad ahmad-saad Dec 2, 2016

Choose a reason for hiding this comment

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

Hallo,
This is a good option.

but had anyone thing about the other stores.

I use now Shopware.apps.Base.store.Country and I have this error :

Ups! Ein Fehler ist aufgetreten!

    Die nachfolgenden Hinweise sollten Ihnen weiterhelfen.
    [Syntax Error] line 0, col 275: Error: Expected '.' or '(', got 'name' in vendor/doctrine/orm/lib/Doctrine/ORM/Query/QueryException.php on line 52

    Stack trace:
    
        #0 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(448): Doctrine\ORM\Query\QueryException::syntaxError('line 0, col 275...', Object(Doctrine\ORM\Query\QueryException))
#1 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2890): Doctrine\ORM\Query\Parser->syntaxError(''.' or '('')
#2 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2867): Doctrine\ORM\Query\Parser->StringPrimary()
#3 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(3151): Doctrine\ORM\Query\Parser->StringExpression()
#4 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2541): Doctrine\ORM\Query\Parser->LikeExpression()
#5 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2447): Doctrine\ORM\Query\Parser->SimpleConditionalExpression()
#6 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2423): Doctrine\ORM\Query\Parser->ConditionalPrimary()
#7 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2391): Doctrine\ORM\Query\Parser->ConditionalFactor()
#8 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(2366): Doctrine\ORM\Query\Parser->ConditionalTerm()
#9 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(1333): Doctrine\ORM\Query\Parser->ConditionalExpression()
#10 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(876): Doctrine\ORM\Query\Parser->WhereClause()
#11 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(843): Doctrine\ORM\Query\Parser->SelectStatement()
#12 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(268): Doctrine\ORM\Query\Parser->QueryLanguage()
#13 vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php(351): Doctrine\ORM\Query\Parser->getAST()
#14 vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php(263): Doctrine\ORM\Query\Parser->parse()
#15 vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php(123): Doctrine\ORM\Tools\Pagination\Paginator->getCountQuery()
#16 engine/Shopware/Components/Model/ModelManager.php(191): Doctrine\ORM\Tools\Pagination\Paginator->count()
#17 engine/Shopware/Controllers/Backend/Base.php(421): Shopware\Components\Model\ModelManager->getQueryCount(Object(Doctrine\ORM\Query))
#18 engine/Library/Enlight/Controller/Action.php(159): Shopware_Controllers_Backend_Base->getCountriesAction()
#19 engine/Library/Enlight/Controller/Dispatcher/Default.php(523): Enlight_Controller_Action->dispatch('getCountriesAct...')
#20 engine/Library/Enlight/Controller/Front.php(223): Enlight_Controller_Dispatcher_Default->dispatch(Object(Enlight_Controller_Request_RequestHttp), Object(Enlight_Controller_Response_ResponseHttp))
#21 engine/Shopware/Kernel.php(178): Enlight_Controller_Front->dispatch()
#22 vendor/symfony/http-kernel/HttpCache/HttpCache.php(487): Shopware\Kernel->handle(Object(Enlight_Controller_Request_RequestHttp), 1, true)
#23 engine/Shopware/Components/HttpCache/AppCache.php(255): Symfony\Component\HttpKernel\HttpCache\HttpCache->forward(Object(Symfony\Component\HttpFoundation\Request), true, NULL)
#24 vendor/symfony/http-kernel/HttpCache/HttpCache.php(258): Shopware\Components\HttpCache\AppCache->forward(Object(Symfony\Component\HttpFoundation\Request), true)
#25 engine/Shopware/Components/HttpCache/AppCache.php(103): Symfony\Component\HttpKernel\HttpCache\HttpCache->pass(Object(Symfony\Component\HttpFoundation\Request), true)
#26 shopware.php(113): Shopware\Components\HttpCache\AppCache->handle(Object(Symfony\Component\HttpFoundation\Request))
#27 {main}
    
 

and if I fixed with setAlias it remove the value I enter directly. it requeried the all country name

and I try to fix it by adding % to the end of the filter value and it worked.

but the problem when you used multi select option you can not search for two countries.

so I think there is a lot of problem with this change hier and it make my plugins brocken.

@ahmad-saad
Copy link
Contributor

any news about this pull request as I see the issues Team had mein Tieckt Scheduled.

so I think this is a global problem not just in my side I hope this Problem will be solve as soon as possibale.

@OliverSkroblin @bcremer
Regard's

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.

None yet

5 participants