#7698 - modularize admin global search #9192

tzyganu · 2017-04-10T13:09:41Z

This PR solves the issue #7698.
It modularizes the admin global search and translates the preview links "keyword in Pages"...

Description

The PR makes the admin global search modularized. The composition of the search was moved to the proper module. (sales search in Sales module di.xml. Also for customer and cms pages).
I removed the preview link 'keywords in Products' because the products admin grid does not have a full text search, so clicking on that link does not give out the expected result.
In the search.phtml the text for previews ('keyword' in customers) was not translatable. Not it should be.
The list of previews was not extensible. It was hardcoded in the search.phtml template.
I added one more class member of the Magento\Backend\Controller\Adminhtml\Index\GlobalSearch that holds an array of elements that can be displayed as preview for different modules.

What I didn't do, is to increment the module version in module.xml and composer.json. I will need some guidance on that.

Fixed Issues (if relevant)

Admin Global Search was build in a hurry #7698: Admin Global Search was build in a hurry

Manual testing scenarios

Steps to reproduce are listed in the issue

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

tzyganu · 2017-04-10T15:13:42Z

I know, I know. I have some tests to fix.

rodde177 · 2017-04-12T13:25:31Z

app/code/Magento/Backend/etc/di.xml

-                </item>
-            </argument>
+            <argument name="searchModules" xsi:type="array" />
+            <argument name="prevideModules" xsi:type="array" />


typo for previewModules?

southerncomputer · 2017-04-14T16:58:55Z

Need to hook this up to elasticsearch so it really can be a global search!

maghamed

I removed the preview link 'keywords in Products' because the products admin grid does not have a full text search, so clicking on that link does not give out the expected result.

Currently it's a bug, that Product Grid doesn't provide Search functionality. And that would be fixed soon in the scope of #10089
which is awaiting to be processed.
So, no need to remove 'keywords in Products' drop-down item

maghamed · 2017-07-11T10:06:04Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+    /**
+     * @return array
+     */
+    protected function getPreviewItems()


let's give just "private" visibility for all the new functions

maghamed · 2017-07-11T10:25:26Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+            }
+            $result[] = [
+                'url' => $this->getUrl($previewConfig['url']).'?search='.$query,
+                'name' => __($previewConfig['text'], $query)


we don't have translate="true" attribute in DI.xml , as we have for UI components configuration, thus translation would not work correctly.

@maghamed Any pointers on how to handle this so the translation will work and still be able to keep the texts in DI? O maybe move them somewhere else?

@tzyganu I found a possibility to specify "string" type to be translatable in DI.xml
using the "translate" attribute
Here is the definition of this attribute for any xsi:type="string" attribute type in types.XSD schema which is used by ObjectManager

magento2/lib/internal/Magento/Framework/Data/etc/argument/types.xsd

Lines 31 to 33 in 99e85cb

<xs:extension base="argumentType">

<xs:attribute name="translate" use="optional" type="xs:boolean"/>

</xs:extension>

Here is how this attribute interpreted

magento2/lib/internal/Magento/Framework/Data/Argument/Interpreter/StringUtils.php

Lines 41 to 44 in 99e85cb

$needTranslation = isset($data['translate']) ? $this->booleanUtils->toBoolean($data['translate']) : false;

if ($needTranslation) {

$result = (string)new \Magento\Framework\Phrase($result);

}

But there is no any usage of this attribute in Magento.
And looks like all the tries to pass strings using DI.xml just don't translate at all

Like this one:

<type name="Magento\Catalog\Model\Entity\Product\Attribute\Design\Options\Container"> <arguments> <argument name="options" xsi:type="array"> <item name="option1" xsi:type="array"> <item name="value" xsi:type="string">container1</item> <item name="label" xsi:type="string">Product Info Column</item> </item> <item name="option2" xsi:type="array"> <item name="value" xsi:type="string">container2</item> <item name="label" xsi:type="string">Block after Info Column</item> </item> </argument> </arguments> </type>

or this one:

<type name="Magento\Customer\Ui\Component\MassAction\Group\Options"> <arguments> <argument name="data" xsi:type="array"> <item name="urlPath" xsi:type="string">customer/index/massAssignGroup</item> <item name="paramName" xsi:type="string">group</item> <item name="confirm" xsi:type="array"> <item name="title" xsi:type="string">Assign a Customer Group</item> <item name="message" xsi:type="string">Are you sure to assign selected customers to new group?</item> </item> </argument> </arguments> </type>

maghamed · 2017-07-11T10:26:34Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

            }
        }

        /** @var \Magento\Framework\Controller\Result\Json $resultJson */
        $resultJson = $this->resultJsonFactory->create();
        return $resultJson->setData($items);
    }
+
+    /**


please provide a clear description for this function

maghamed · 2017-07-11T10:27:07Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+     *
+     * @var array
+     */
+    protected $previewModules;


don't use protected for newly created properties

maghamed · 2017-07-11T10:49:14Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

@@ -21,16 +21,26 @@ class GlobalSearch extends \Magento\Backend\Controller\Adminhtml\Index
    protected $_searchModules;

    /**
+     * modules that support preview


Btw this controller should be marked with PHP DocBlock as @ api
because now it represents extension point for other search modules.

Btw it makes sense to use another extension point for these purposes

maghamed · 2017-07-11T10:52:06Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+            if (empty($className)) {
+                continue;
+            }
+            $searchInstance = $this->_objectManager->create($className);


why do we use ObjectManager here?
the dedicated factory should be used instead. Btw, this factory should check the type of needed object . All the search classes should implement common interface/provide the same contract
Because for now there is no any guarantee that created class supports contract by which you work with it below

@maghamed I didn't add this by myself. The code was there I just moved it around. But I will try to find a way around it.

Marius, you right this looks like legacy code, even taking into account that it was written quite recently.

What we usually do for such cases is to introduce new Interface with the common contract, and Factory which is responsible for producing an instance of this interface and making sure that object which would be created is an instance of the interface mentioned above.
Factory also could serve as an extension point and other developers could introduce own instance of the contract and register it using DI configuration in this factory (constructor argument).

Doing so we get:

well defined interface for some piece of functionality

well defined Factory which creates some particular implementation of this interface

extension point which helps 3rd party developers to extend basic implementations with custom one

@maghamed already on it.

maghamed · 2017-07-11T10:54:51Z

app/code/Magento/Backend/etc/di.xml

-                </item>
-            </argument>
+            <argument name="searchModules" xsi:type="array" />
+            <argument name="previewModules" xsi:type="array" />


it's not mandatory to provide these configurations

tzyganu · 2017-07-11T12:03:57Z

@maghamed Thanks for pointing me in the right direction. I will try to fix the requested changes and come back with a better code.

ishakhsuvarov · 2017-07-11T14:59:50Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

     */
    public function __construct(
        \Magento\Backend\App\Action\Context $context,
+        ItemsFactory $itemsFactory,


New argument to constructor may only be added as the last on the list to preserve backwards compatibility.
Please refer to Backwards Compatible Development guide for more information

By last, you mean last one that does not have a default value, right?

No, @ishakhsuvarov means last in a list of all existing arguments (after all existing arguments).
And each new argument added should be optional (nullable)

ishakhsuvarov · 2017-07-11T15:01:27Z

app/code/Magento/Backend/Model/Search/ItemsFactory.php

+use Magento\Backend\Api\Search\ItemsInterface;
+use Magento\Framework\ObjectManagerInterface;
+
+class ItemsFactory


Please consider using ItemFactory name for this class to be consistent with other Factories.

maghamed

Marius, please pay attention on my comments.

Also, could you please add 'keywords in Products' , because we already have this functionality worked in another PR

maghamed · 2017-07-11T15:45:02Z

app/code/Magento/Backend/Api/Search/ItemsInterface.php

+namespace Magento\Backend\Api\Search;
+
+/**
+ * @api


please provide description for this interface.
All interfaces marked as @ api should be accompanied with clear and precise description

maghamed · 2017-07-11T15:48:42Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+                $items = array_merge_recursive($items, $results);
+
+            } catch (\LogicException $exception) {
+                continue;


exception swallowing is a bad practice.
at least we need to log critical message here.
But in general, LogicalException should not be just swallowed and silently continue script execution

if you don't know in this specific case how to handle Exception, maybe make sense not to catch it at all

maghamed · 2017-07-11T15:50:10Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+    }
+
+    /**
+     * retrieve all entity items that should appear in global search


please, start the sentences with the capital letter

maghamed · 2017-07-11T15:57:31Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+use Magento\Backend\Model\Search\ItemsFactory;
+
+/**
+ * @api


everything marked as @ api should be accompanied with description

maghamed · 2017-07-11T16:02:18Z

app/code/Magento/Backend/Api/Search/ItemsInterface.php

+     * set offset
+     *
+     * @param int $start
+     * @return ItemsInterface


we don't recommend to use "fluent interfaces" and "method chaining", we support CQS where the command should return void instead of $this.
Returning $this from the setter method is allowed just for Builder implementation
(I see that we have this chaining in code before your changes, just think that if we started to improve the code quality, let's fix this out)
or let's use Builder pattern here

Thus, there are two options @tzyganu :

Build/Fill kind of SearchCriterial and introduce an independent interface for processing this SearchCriteria - in this case, we will segregate responsibilities.

Or use Builder pattern - all setters change the state, getResults() - build objects and reset the state

@maghamed I was trying the SearchCriteria approach, but hit a wall.
I was doing it like this.
have an ItemInterface with a single method getResults(\Magento\Framework\Api\SearchCriteria $searchCriteria) but I cannot add the filter in the search criteria for the query because the query filtering is dependent on the entity I'm filtering. So maybe I can do it via an additional parameter?. Have getResults look like this getResults($searchCriteria, $query) and the classes that implement this interface are responsible for handling the query.
I could also avoid changing the $searchCriteria parameter by using an additional searchCriteriaBuilder inside the implementations of getResults. (this is already like that for the customer search)

@tzyganu I think the existing SearchCriteria we have in Framework \Magento\Framework\Api\SearchCriteria would not fit your needs. It's too heavyweight for your specific case.
When I said "SearchCriteria" I meant similar approach we use with SearchCriteria:

To have a dedicated interface to describe your request (aka SearchCriteria). In your case that would be something like:

namespace Your\Custom\Api; class SearchCriteria { public function setStart($start); public function setQuery($query); public function setLimit($limit); public function getStart(); public function getQuery(); public function getLimit(); }

To have Search interfaces which will process a search query based on the SearchCriteria given as Input parameter. Something like:

namespace Your\Custom\Api; class QueryProcessor { public function getResults(SearchCriteria $searchCriteria); // or maybe Search(SearchCriteria $searchCriteria) }

Doing so we make a responsibility segregation and don't have problems with managing and keeping the state in entity responsible for search request execution because all the parameters come as input from the outside each time.

maghamed · 2017-07-11T16:06:11Z

app/code/Magento/Backend/Api/Search/ItemsInterface.php

+     *
+     * @return array
+     */
+    public function getResults();


This interface is pretty bad one because it has a temporal coupling
on the level of contract. Because contract implies an order in which these methods would be called.
Firstly initialize all setters, and then call getResuls()

BTW This interface looks like a good application of Builder pattern for me.
So, if we would rename it to SearchResultBuilder that will make more sense.

maghamed · 2017-07-11T16:11:49Z

app/code/Magento/Backend/Model/Search/ItemsAbstract.php

+use Magento\Backend\Api\Search\ItemsInterface;
+use Magento\Framework\DataObject;
+
+abstract class ItemsAbstract extends DataObject implements ItemsInterface


introducing of abstract classes is a bad practice. In M2 we are trying to avoid using Inheritance Based API.

use composition instead, if your main reason for Inheritance usage to get rid of Copy-Paste

maghamed · 2017-07-11T16:57:38Z

app/code/Magento/Backend/Model/Search/ItemsFactory.php

+    public function create($instanceName, array $data = [])
+    {
+        $object =  $this->objectManager->create($instanceName, $data);
+        if ($object instanceof ItemsInterface) {


it's better to make a type check before the object instantiation.
Use string which represents class name and check whether it's an implementation of needed class

maghamed · 2017-07-11T17:10:05Z

app/code/Magento/Backend/Model/Search/ItemsFactory.php

+     */
+    public function create($instanceName, array $data = [])
+    {
+        $object =  $this->objectManager->create($instanceName, $data);


looks like it's better to use 'get' instead of 'create' as this should be stateless object

maghamed · 2017-07-11T17:11:04Z

app/code/Magento/Backend/Model/Search/ItemsFactory.php

+            return $object;
+        }
+        throw new \LogicException(
+            "The class '{$instanceName}' does not implement ".ItemsInterface::class


please use spaces between concatenation
"" . ItemsInterface::class

maghamed · 2017-07-11T17:14:09Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+use Magento\Backend\Model\Search\ItemsFactory;
+
+/**
+ * @api


Current Controller became big enough and now contains Business logic inside.
It's better to introduce dedicated handler/model for this.

In M2 we are trying to make Controllers as light/ thin as possible

This Handler would be marked as @ api intead of current controller

maghamed · 2017-07-11T17:17:54Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+            if ($previewConfig['acl'] && !$this->_authorization->isAllowed($previewConfig['acl'])) {
+                continue;
+            }
+            if (!$previewConfig['url'] || !$previewConfig['text']) {


not correct comparison
!$previewConfig['url'] || !$previewConfig['text']

use isset instead

tzyganu · 2017-07-12T06:15:59Z

@maghamed Ok. Let's recap. It would be easier for me to follow your instructions since I'm not that familiarized with all your code quality standards.

No abstract classes just to avoid copy-paste
to temporal coupling interfaces. Use a builder instead (I have to check how other builders work. An example I can follow would be nice)
check class type before instantiating object
use objects that return the results as singletons (get instead of create)
Don't mark the full global search controller as api. Just one action?
Don't hide the LogicException.
put back the product search
merge to latest develop and fix conflicts.
small coding standards fixes (like spaces, capitalization of comments, ...)

For the string translation in di.xml files, I propose to add the translate attribute on the item tags and when the translation if fixed to be picked up this will work (maybe I can even take a swing in a different PR at fixing this)

Let me know if I missed something.

One more question. Should I close this PR and open a new cleaner one following your guidelines? Or should I continue on this as it is easier to follow?

get latest develop from upstream

maghamed · 2017-07-12T12:02:22Z

Hey @tzyganu

No abstract classes just to avoid copy-paste

We are against the usage of any abstract class in newly created code. And all the existing abstract classes is the point of refactoring now or in future. For example, we would like to eliminate all the "Layer Super Types" like AbstractBlock, AbstractModel, AbstractConstructor.
We prefer to use composition instead.

to temporal coupling interfaces.

Correct. You should not design interface in a way to have an implicit relationship between two, or more, methods of a class requiring clients to invoke one method before the other.
Builder would not help you to get rid of temporal coupling, you should re-design your interface to segregate responsibilities

check class type before instantiating object

It's not a general rule. just to be more secure, because in your specific case factory accepts class to instantiate as external argument.

Don't mark the full global search controller as api. Just one action?

We are not fans of having business logic in the controllers. In your case Controller became an extension point, and anybody who wants to extend/customize search. Or add new module for search should add additional parameter for this controller. But our Controllers are responsible just for specific entry point. For example, for Mobile application we could have another Controller.
Thus, we need to copy-paste this business logic to support all the entry points.
The best rule is to keep controllers as thin as possible.

Based on the above this logic should belong to some model, not to Controller.

Don't hide the LogicException.

That's true for all the exception types

put back the product search

correct

small coding standards fixes (like spaces, capitalization of comments, ...)

would be nice

For the string translation in di.xml files, I propose to add the translate attribute on the item

yes, let's do this

Or should I continue on this as it is easier to follow?

I think we can continue in the scope of this PR

BTW , that would be helpful for you to get aware with main Magento principles and guidelines :

tzyganu · 2017-07-13T13:05:05Z

@maghamed I'm done. I hope. Just codacy is complaining a bit. I did the last changes I can think of. Please have another look.

ishakhsuvarov · 2017-07-13T15:18:07Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

+        ItemFactory $itemFactory,
+        SearchCriteriaFactory $criteriaFactory,
+        array $searchModules = [],
+        array $previewModules = []


Such changes to the class constructor violate backwards compatibility policy:

New dependencies must not be added to the middle, only as a last parameter.

All newly added arguments must be nullable.

Something like

array $searchModules = [], ItemFactory $itemFactory = null, SearchCriteriaFactory $criteriaFactory = null, array $previewModules = []

And then loading the optional constructor if null via $newDependency ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\New\Dependency\Interface::class) would work well here I think.

ishakhsuvarov · 2017-07-13T15:19:08Z

app/code/Magento/Backend/Api/Search/ItemsInterface.php

+/**
+ * @api
+ */
+interface ItemsInterface


I suggest renaming this interface, as Items is too broad definition.
Also getResults method in ItemsInterface creates confusion, since there may not be results of items.

@ishakhsuvarov do you have a suggestion for @tzyganu in this case? I could think that ResultsInterface and getResults would work well.

ishakhsuvarov · 2017-07-13T15:21:35Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

    ) {
+        $this->itemFactory = $itemFactory;


Please refer to the Adding a constructor parameter section of Backwards Compatible Development Guide for a definition and code example for adding constructor parameters.

ishakhsuvarov · 2017-07-13T15:25:29Z

app/code/Magento/Backend/Controller/Adminhtml/Index/GlobalSearch.php

        $this->_searchModules = $searchModules;
+        $this->previewModules = $previewModules;


Please consider extracting this logic to a dedicated model, since It does not look like it is in controllers responsibility.
Magento 2 is aiming to implement controllers as lightweight as possible, moving all possible parts outside.

@ishakhsuvarov I agree whit this, but I don't see a solution to move the search logic to a separate class and keep backwards compatibility (as suggested in other comments) at the same time. Maybe I can move this to a separate class, and you release it when you break BC for something else anyway.

ishakhsuvarov · 2017-07-13T15:27:29Z

app/code/Magento/Backend/Model/Search/ItemFactory.php

+     *
+     * @param string $instanceName
+     * @param array $data
+     * @return ItemsInterface


@throws annotation may be added here as well

ishakhsuvarov · 2017-07-13T15:28:13Z

app/code/Magento/Backend/Model/Search/ItemFactory.php

+use Magento\Backend\Api\Search\ItemsInterface;
+use Magento\Framework\ObjectManagerInterface;
+
+class ItemFactory


Brief description for this class may be added here

ishakhsuvarov · 2017-07-13T15:30:35Z

app/code/Magento/CatalogSearch/Model/Search/Catalog.php

     * @param \Magento\Framework\Stdlib\StringUtils $string
     * @param QueryFactory $queryFactory
     */
    public function __construct(
-        \Magento\Backend\Helper\Data $adminhtmlData,
+        \Magento\Backend\Helper\Data $adminHtmlData,


This change is considered to be backwards incompatible, please revert to maximize backwards compatibility

ishakhsuvarov · 2017-07-13T15:32:52Z

app/code/Magento/CatalogSearch/Model/Search/Catalog.php

     */
-    public function load()
+    public function getResults(SearchCriteria $searchCriteria)


This change effectively removes method load, this violates backwards compatibility for the third-party code which may rely on it.
You may add a new method and proxy calls from the old one, given that result will be compatible

ishakhsuvarov · 2017-07-13T15:34:39Z

app/code/Magento/Customer/Model/Backend/Search/Customer.php

     * @param \Magento\Framework\Api\FilterBuilder $filterBuilder
     * @param \Magento\Customer\Helper\View $customerViewHelper
     */
    public function __construct(
-        \Magento\Backend\Helper\Data $adminhtmlData,


Same as above

ishakhsuvarov · 2017-07-13T15:35:20Z

app/code/Magento/Customer/Model/Backend/Search/Customer.php

@@ -3,32 +3,19 @@
 * Copyright © Magento, Inc. All rights reserved.


Unfortunately a class can not be moved, since already existing 3rd-party code may already rely on it.

@ishakhsuvarov How about if I deprecate the class, leave it empty and make it extend my new class (the one in the right module)? Would that solve the issue for 3rd party code that may use plugins on it?

ishakhsuvarov · 2017-07-13T15:35:27Z

app/code/Magento/Customer/Model/Backend/Search/Customer.php

     */
-    public function load()
+    public function getResults(SearchCriteria $searchCriteria)


same as above

ishakhsuvarov · 2017-07-18T13:30:02Z

Hi @tzyganu
Do you have any questions regarding the code reviews? Please let us know if you would like any help with coding also.
Thanks!

tzyganu · 2017-07-18T17:31:29Z

@ishakhsuvarov Thanks for the tips. I'm on vacation for the next 2 weeks and I don't want to see any code in this period. 😄 If you're in a hurry, you can pic it up from where I left off. If not I will give it an other shot it august.

ishakhsuvarov · 2017-07-25T09:03:59Z

@tzyganu We are not in a hurry for sure. Take your time. Just let us know if you are going to need any assistance.

dmanners · 2017-08-11T07:43:49Z

I love the fact you have moved the classes into the "right" modules. I think this a great improvement, though I cannot think 100% how to stop the BC breaks between the new and old classes other than leaving them as they are and just deprecating them. Maybe @ishakhsuvarov or @maghamed have a great example some place of this sort of change.

ishakhsuvarov · 2017-08-14T12:18:27Z

@dmanners For now we may not remove the old implementations, only mark those with @deprecated annotations and possibly proxying the calls to the new ones to maximize compatibility.
When it is applicable those annotations will be scanned and "old" classes removed, if possible.

ishakhsuvarov · 2017-08-17T15:35:06Z

@tzyganu I am closing this PR for now due to inactivity. Please reopen and fix merge conflicts when you would like to continue.
Thank you for collaboration.

tzyganu · 2017-08-21T08:39:47Z

@ishakhsuvarov I don't think I will pick this up again. At least until you make up your mind.
I got mixed comments about this PR. You (as in core team) want me to move the logic to a separate class (which sounds reasonable) and at the same time keep the backwards compatibility. I don't have a (clean) solution on how to do both. Just let me know when you are willing to break BC on this one and maybe I will pick it up again.

orlangur · 2017-08-21T10:47:30Z

@tzyganu,

willing to break BC on this one

I don't think it will ever happen. The one always should deprecate some unneeded classes/methods and only in some next release they will be removed.

If you feel some implementation approach with respect to http://devdocs.magento.com/guides/v2.1/ext-best-practices/extension-coding/backwards-compatible-development/index.html does not seem to be clean, let's discuss it first and try to figure out the cleanest possible BC-compliant approch (maybe, we will determine that it's still too dirty to be embodied into life, who knows...).

strajeru added 2 commits April 10, 2017 15:50

#7698 - modularize admin global search

36f3410

#7698 - modularize admin global search

3af1834

fix failing test

cec4e90

rodde177 reviewed Apr 12, 2017

View reviewed changes

fix typo

ce0d756

maghamed self-requested a review April 21, 2017 12:19

maghamed self-assigned this Apr 21, 2017

maghamed suggested changes Jul 11, 2017

View reviewed changes

maghamed reviewed Jul 11, 2017

View reviewed changes

strajeru added 2 commits July 11, 2017 17:16

add interface for search items

a0d3bc2

Merge remote-tracking branch 'origin/develop' into M2-7698

5c5d676

ishakhsuvarov reviewed Jul 11, 2017

View reviewed changes

ishakhsuvarov self-assigned this Jul 11, 2017

maghamed suggested changes Jul 11, 2017

View reviewed changes

maghamed reviewed Jul 11, 2017

View reviewed changes

ishakhsuvarov added the Progress: needs update label Jul 12, 2017

tzyganu and others added 2 commits July 12, 2017 10:40

Merge pull request #2 from magento/develop

3391728

get latest develop from upstream

merge latest develop

4a3090c

strajeru added 2 commits July 12, 2017 16:11

separated get results from search query

ce31871

separated get results from search query

3e5a937

strajeru added 3 commits July 13, 2017 12:10

7698: fix code style

820c116

7698: fix code style

f95d6e2

7698: fix code style

fa050cb

ishakhsuvarov reviewed Jul 13, 2017

View reviewed changes

okorshenko added this to the July 2017 milestone Jul 20, 2017

okorshenko added the develop label Jul 20, 2017

okorshenko modified the milestones: July 2017, August 2017 Aug 1, 2017

ishakhsuvarov closed this Aug 17, 2017

	<xs:extension base="argumentType">
	<xs:attribute name="translate" use="optional" type="xs:boolean"/>
	</xs:extension>

	$needTranslation = isset($data['translate']) ? $this->booleanUtils->toBoolean($data['translate']) : false;
	if ($needTranslation) {
	$result = (string)new \Magento\Framework\Phrase($result);
	}

		$this->_searchModules = $searchModules;
		$this->previewModules = $previewModules;

		@@ -3,32 +3,19 @@
		* Copyright © Magento, Inc. All rights reserved.

#7698 - modularize admin global search #9192

#7698 - modularize admin global search #9192

Conversation

tzyganu commented Apr 10, 2017 • edited by okorshenko Loading

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

tzyganu commented Apr 10, 2017

Choose a reason for hiding this comment

southerncomputer commented Apr 14, 2017

maghamed left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maghamed Jul 11, 2017 • edited Loading

Choose a reason for hiding this comment

maghamed Jul 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzyganu commented Jul 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maghamed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maghamed Jul 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzyganu commented Jul 12, 2017

maghamed commented Jul 12, 2017

tzyganu commented Jul 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ishakhsuvarov commented Jul 18, 2017

tzyganu commented Jul 18, 2017

ishakhsuvarov commented Jul 25, 2017

dmanners commented Aug 11, 2017

ishakhsuvarov commented Aug 14, 2017

ishakhsuvarov commented Aug 17, 2017

tzyganu commented Aug 21, 2017

orlangur commented Aug 21, 2017

tzyganu commented Apr 10, 2017 •

edited by okorshenko

Loading

maghamed left a comment •

edited

Loading

maghamed Jul 11, 2017 •

edited

Loading

maghamed Jul 11, 2017 •

edited

Loading

maghamed Jul 11, 2017 •

edited

Loading