Refactor SearchModule #3470
Refactor SearchModule #3470
Conversation
- eliminate or deprecate all legacy code usage. - update previous SearchableInterface to add form handling - update Users search functionality - correct search block template overrides in core themes - update documentation
[ci skip] [skip ci]
Apply fixes from StyleCI
CHANGELOG-1.4.md
Outdated
|
|
||
| - Fixes: | ||
| - Corrected path to legacy module's admin icons. | ||
| - Made display names of Menu and Theme modules more readable (#3448). | ||
|
|
||
| - Features: | ||
| - Added Permission-based controls for MenuModule menu items (#3314). | ||
| - Added Permission-based controls for MenuModule menu items (#3314).' |
There was a problem hiding this comment.
Typo (quote at the end of the line)
| @@ -94,7 +135,7 @@ public function getFormOptions() | |||
| } | |||
| // remove disabled | |||
| foreach ($searchModules as $displayName => $moduleName) { | |||
| if ((bool) $this->getVar('disable_' . $moduleName, true)) { | |||
| if ((bool) $this->getVar('disable_' . $moduleName, false)) { | |||
There was a problem hiding this comment.
BC break?
I'd guess that a default value of true is more reasonable because it does not immediately add new modules to the search (which may not be desired).
There was a problem hiding this comment.
It didn't work the other way for me.
| if ($setActiveDefaults) { | ||
| $activeModules[$moduleName] = 1; | ||
| } | ||
| if ($this->getVar('disable_' . $moduleName)) { |
| throw new AccessDeniedException(); | ||
| } | ||
|
|
||
| $startnum = $request->query->filter('startnum', 0, false, FILTER_VALIDATE_INT); |
There was a problem hiding this comment.
$request->query->getInt('startnum', 0)
| } | ||
|
|
||
| $startnum = $request->query->filter('startnum', 0, false, FILTER_VALIDATE_INT); | ||
| $itemsPerPage = $this->getVar('itemsperpage'); |
| */ | ||
| public function buildForm(FormBuilderInterface $builder, array $options) | ||
| { | ||
| if ($options['permissionApi']->hasPermission($builder->getName() . '::', '::', ACCESS_READ)) { |
There was a problem hiding this comment.
What does $builder->getName() return here for example?
There was a problem hiding this comment.
in that case, the module name
| 'translator' => $this->get('translator.default'), | ||
| 'action' => $this->get('router')->generate('zikulasearchmodule_search_execute') | ||
| ]); | ||
| $form->add($moduleFormBuilder->getForm()); |
There was a problem hiding this comment.
This is where the "magic" is done :-)
| @@ -11,7 +11,6 @@ | |||
|
|
|||
| namespace Zikula\SearchModule\Block; | |||
|
|
|||
| use ModUtil; | |||
There was a problem hiding this comment.
do not import from global namespace
does this make a difference?
Personally I prefer the import against using \Foo because it is more explicite about which classes are used within a class. Just a matter of taste or is there a behavioural difference?
There was a problem hiding this comment.
no behavior diff AFAIK but easier to delete on merge to master without forgetting the import
Description