UI Architecture Discussion

[renzon]

While discussion #3066 several opinions came up during sprint meeting. So this is the space for people share them. @rochacbruno @oshtaier @sghai @lpramuk @rplevka

[rochacbruno]

Currently we have

entity.search(name)

which I think we should keep backwards compatibility, so we do not need to touch any tests using that signature.

It can be achieved by introspecting the passed arguments.

• if search gets a single argument and it is a string so it uses the search_key for entity or the default name to search.
• If named arguments search(name='foo', category='bla') so it make a multi field based search
• if special named arguments received (following the example used by ORMs ) search(_raw='raw_query', _another_special_thing='') then we can have special things happening for those _ arguments.

The above may work and keep existing tests running without breaking and no need to touch 92 files.

[rochacbruno]

Another Idea I have is.

class UIQuerySet(object):
    """this object holds search executor"""
    def first(self):
        """returns only the first element of search result"""
         returns the entity
    def last(self):
        """returns only last element of search result"""
    def sort(self, keys):
        """sorts search result in place"""
         returns UISearchResult(entity1, entity2, ...)
      .... 
      # a bunch of needed ORM like methods can be here, (following the rule of `fat models` thin controllers) 

With the above whenever we call search(.....) if no arguments given, the search returns UIQuerySet object exposing some methods to deal with the search (that is useful when searching gives us more than 1 result).

Example:

# keep current solution working (returning entity when .search is called with a single argument)
obj = entity.search("foo") 

# calling .search() with no arguments returns an empty `UIQuerySet`
entity.search()

# get the first result having "name=foo"
obj = entity.search().first(name='foo')   (the same for last)

# get the UNIQUE result of org called "bla", if not UNIQUE raise `NotUniqueError`
org = organization.search().get(name='bla')

The above idea can be improved, but that allows us to keep search as only a proxy to the real search controller which is UIQuerySet and in that object we can have a concise naming and interface for searching and dealing with results and also we can keep current code working with no breaks.

[abalakh]

Idea of temporary solution for now with backward compatibility, while something more interesting will be implemented in the separate branch: rename current search method to something like search_raw and make it accept not entity_name, but pure string query. On the top of it implement search method, purpose of which is just to form the query like '{} = {}'.format(self.search_key, element_name) and pass it to search_raw. Something like this:

    self.search_key = 'name'

    def search_raw(query):
        # navigate to entity
        # type 'query' into search field
        # perform search
        # return result

    def search(element_name):
        query = '{} = {}'.format(self.search_key, element_name)
        return search_raw(query)

Pros:

• very easy to implement
• we won't break anything, all the tests will work as they were before, so no 90+ files changes
• search() works with element_name as it was before, and in case we need some complex query - we can use search_raw(), where we can pass something like 'name = foo OR label = bar AND description = eggs'

Cons:

• basically all the cons we have for current search() with the search_key approach will apply to this approach as well, so it can be only considered as a temporary solution to be able to automate custom queries right now, while better architecture is being implemented in background.

[omaciel]

@rochacbruno using your example, here's what I'd expect from a search method:

# DESIRED BEHAVIOR
obj = User.search("foo") 
UIQueryException: Please provide at least one valid `key` to filter by!

# BACKWARDS COMPATIBLE DESIGN
# Relies on `search_key`
obj = User.search("foo") 
assert obj.name == 'foo'

# calling .search() with no arguments returns an empty `UIQuerySet`
User.search()
UIQueryException: Missing arguments!

# Return exact match for query
obj = User.search(login="admin")
assert len(obj) == 1
assert type(obj) == list
assert obj[0].login == 'admin'

# Assuming several users:
# Bart Simpson
# Homer Simpson
#  Marge Simpson
# return multiple results
obj = User.search(lastname="Simpson")
assert len(obj) > 1
assert "Bart" in [user.lastname for user in obj]

# Assuming order of objects is sorted as follows:
# Bart Simpson
# Homer Simpson
#  Marge Simpson
obj = User.search(lastname="Simpson").sort("firstname")
assert obj.first().firstname == 'Bart'
assert obj.last().firstname == 'Marge'

# Using multiple arguments
obj = User.search(firstname="Bart", lastname="Simpson").first()
assert obj.firstname == 'Bart'

[rochacbruno]

@omaciel I like your example

obj = User.search("foo") 
UIQueryException: Please provide at least one valid `key` to filter by!

This is the right way I think, but it will break the compatibility with current test_cases, we should change all the code.

I like all the other examples you gave, only missing the case where you want to get a single unique entity obj = User.search(firstname="Bart", lastname="Simpson").first() is the way to get a single, because search should return a list of elements according to your lastname=simpson example

[omaciel]

This is the right way I think, but it will break the compatibility with current test_cases, we should change all the code.

Ahh chucks! I forgot about that... guess we would rely on the search_key as a fall back then?

I like all the other examples you gave, only missing the case where you want to get a single unique entity obj = User.search(firstname="Bart", lastname="Simpson").first() is the way to get a single, because search should return a list of elements according to your lastname=simpson example

Agreed. Let me update my examples...

[elyezer]

Something to consider is the scoped search where you can do some interesting queries like last_name=Simpson and age > 10 for more information check the scoped search docs [1] and the Foreman docs about advanced search tips [2].

In my opinion we should have that in mind when designing the new UI search Robottelo feature, having the power of the scoped search will help 1) test the scoped search on entities that supports it 2) create more interesting queries looking for specific objects (this will ensure the system is not creating extra entities for example).

My initial proposal to this was to use search as the raw search where you can pass any query. This will break the current behavior since the current search method expects the value for the entity_name field and will require refactor. Also passing the default key all the times is something that will be very boring to do when writing tests.

Right now,I don't have any proposal different from what was already mentioned, I just want to point the features we should be able to support. I will add another comment if I come up with something.

[1] https://github.com/wvanbergen/scoped_search/wiki/query-language [2] https://theforeman.org/2012/01/advance-search-tips-in-foreman.html

[rochacbruno]

>>> user.search(_raw="last_name=Simpson and age > 10")
[<User Lisa>, <User Homer>, <User Marge>...] 

[rplevka]

@elyezer, If you really want to pass a default key to search the entity.get method, you can make it optional and have the def. search key definedvas an attribute of the ui.Entity class. I guess that would make writing testcases fun again and keep ba kward compatibility. As a bonus, we might change the behavior by passing the key to the method if needed.

Under its hood, it will actually be a search method with crafted

def_search_key = value

query.

[ldjebran]

Think we need to create some helper classes for items discoveries for some context usage first, for example by creating a WebElementEnvelope to encapsulate a selenium WebElement for example:

class WebElementEnvelope:

    _entity_base = None   # type: robottelo.ui.base.Base

    def __init__(self, parent, web_element):
        """
        :type parent: robottelo.ui.base.Base | WebElementEnvelope
        :type web_element: selenium.webdriver.remote.webelement.WebElement
        """
        if not parent:
            raise ValueError('parent must be defined')

        if not isinstance(parent, WebElementEnvelope) or not isinstance(parent, RobotteloUIBase):
            raise TypeError('parent of web envelope must be a web envelope or robottelo ui Base')

        self._parent = parent

        if not isinstance(web_element, WebElement):
            raise TypeError('web_element must of type selenium WebElement')

        self._web_element = web_element

    @property
    def web_element(self):
        return self._web_element

    @property
    def parent(self):
        return self._parent

    @property
    def entity_base(self):
        if self._entity_base:
            return self._entity_base

        parent = self.parent

        while not isinstance(parent, RobotteloUIBase):
            parent = parent.parent

        self._entity_base = parent
        return parent

    @property
    def value(self):
        value = self.web_element.get_attribute('value')
        if value is None:
            value = self.web_element.text

        return value

and creating WebElementContainer class that can have items

class WebElementContainer(WebElementEnvelope):
    sub_items_locator = None  # type: tuple[str, str]
    sub_item_class = None  # type: WebElementEnvelope

    def __init__(self, parent, web_element, sub_items_locator=None, sub_item_class=None):
        """
        :type parent: robottelo.ui.base.Base | WebElementEnvelope
        :type web_element: selenium.webdriver.remote.webelement.WebElement
        :type sub_items_locator: (str, str)
        :type sub_item_class: WebElementEnvelope
        """
        super(WebElementContainer, self).__init__(parent, web_element)
        if sub_items_locator:
            self.sub_items_locator = sub_items_locator

        if sub_item_class:
            self.sub_item_class = sub_item_class

        self._items = []  # type:

    @property
    def web_elements(self):
        """
        :return: all the sub web elements from the default locator
        :rtype : list[selenium.webdriver.remote.webelement.WebElement]
        """
        return self.web_element.find_elements(*self.sub_items_locator)

    def _get_sub_item(self, web_element):

        item_class = self.sub_item_class
        if not item_class:
            item_class = WebElementContainer

        return item_class(self, web_element)

    @property
    def items(self):
        """
        :rtype: list[WebElementEnvelope]
        """
        if self._items:
            return self._items
        items = []
        for we in self.web_elements:
            items.append(self._get_sub_item(we))

        self._items = items
        return self._items

    def clear(self):
        self._items = []

    def first(self):
        if self.items:
            return self.items[0]

        return None

    def last(self):
        if self.items:
            return self.items[-1]

        return None

this WebElementContainer can be used in many situations where we need to work with some containers and their items (tables, tabs, lists, menu ... ).

in the case of data tables for example:

class DataTableCellItem(WebElementEnvelope):

    @property
    def value(self):
        # can do some some investigation on the
        if 'fa-check' in self.web_element.get_attribute('class'):
            return True

        return super(DataTableCellItem, self).value

class DataTableRow(WebElementContainer):
    sub_items_locations = (By.XPATH, '//td')
    sub_item_class = DataTableCellItem

    items_attributes = []
    # eg. for User items_attributes = ['user_name', 'first_name', 'sur_name', 'email, 'administrator',
    # 'last_login_time', 'authorized_by', 'actions']
    # this can be implemented also
    # user_name = WebItem(_class=Some_WebElementEnvelope)  # by default
    # first_name = WebItem(_class=Some_WebElementEnvelope)
    # ...
    # actions = WebItem(_class=Actions_WebElementEnvelope)

    def __init__(self, parent, web_element, sub_items_locator=None, sub_item_class=None, items_attributes=None):
        """
        :type parent: robottelo.ui.base.Base | WebElementEnvelope
        :type web_element: selenium.webdriver.remote.webelement.WebElement
        :type sub_items_locator: (str, str)
        :type sub_item_class: WebElementEnvelope
        :type items_attributes: list[str]
        """
        super(DataTableRow).__init__(parent, web_element, sub_items_locator=sub_items_locator,
                                     sub_item_class=sub_item_class)
        if items_attributes is not None:
            self.items_attributes = items_attributes

    def __getattr__(self, attr_name):
        # return the value of sub item in items attributes
        items_attributes = self.items_attributes
        if not items_attributes:
            items_attributes = self.entity_base.get_search_items_attributes()
        if attr_name in items_attributes:
            item_ind = items_attributes.index(attr_name)
            if 0 <= item_ind < len(self.items):
                return self.items[item_ind].value

        raise AttributeError('attribute {0} does not exist'.format(attr_name))

    def select(self):
        # do some operations
        # find first WebElement <a/> and click
        pass

class DataTable(WebElementContainer):
    sub_items_locations = (By.XPATH, '//tr')
    sub_item_class = DataTableRow

    def get(self, **kwargs):
        """ implement get by using attributes
        :rtype: List[DataTableRow]
        """

In the case of search function: as we have this functionality customized in some sub classes that have no search box or the search box is enclosed under other elements we can separate the logic of searching and reading the data by using decorators on search functions that inject the final data web container in the search function.

def search_box(function=None, locator=default_search_box_locator, web_container_class=DataTable):
    """
    locate the filter box and submit a query
    :type function: callable
    :type locator: tuple
    :param function: entity search callable function
    :param locator: the locator from the build in locators or from from _search_locator this is tuple containing
                    the (strategy, filter_value_or_xpath)
    """

    def main_wrapper(func):

        @functools.wraps(func)
        def function_wrapper(entity, *args, **kw):
            # determine the search strategy      
            # activate the search by getting the first arg  and looking _raw in kw 
            # or parse the first element to find comparison strings 
            # or search attributes in kw  for DataTable.sub_item_class.items_attributes 
            # get the data table web_element
            # set kwargs['_search_strategy'] = 'old' in case of old search startegy
            web_container = DataTable(entity, data_table_web_element) 
            return func(entity, web_container, *args, **kw)

        return function_wrapper

    def wait_function(func):
        return main_wrapper(func)

    if function:
        return main_wrapper(function)
    else:
        return wait_function

class Base:

    def get_search_items_attributes(self):
        raise NotImplemented()    

    @search_box(locator=('id', 'search'))
    def search(self, web_container, text, **kwargs):
        # in case or id search return the first item of web_container for compatibility
        search_strategy = kwargs.get('_search_strategy', 'old')
        if search_strategy == 'old'  and web_container and web_container.items:
            return web_container.items[0].web_element

        return web_container

class ActivationKey(Base):

   def get_search_items_attributes(self):
        return ['name', 'host_limit', 'environment', 'content_view']

# usage
a = ActivationKey().search('environment = Library')
# functions to use
a.first().name
a.last().environment
# perhaps we need to define with attribute is the id
a.get('rh7').content_view
all_tems = a.items
# but each item also contain the selenium web element that if a cell contain buttons we can click them
a.first().web_element

# or possible usage, to get a filtered list of  the current content
a.get_all(environment='Library', content_view='Default Organization View')

ActivationKey().search('environment = Library', _raw=True)
ActivationKey().search(name = 'rh7')
ActivationKey().search('name = rh7')
ActivationKey().search('rh7')

in case of complex search location like Container

create a decorator like

def search_container(function=None, locator=None, resource=None, container=None):
    pass

class Container(Base):

    @search_container()
    def search(self, web_container, text, **kwargs):
        search_strategy = kwargs.get('_search_strategy', 'old')
        if search_strategy == 'old' and web_container and web_container.items:
            return web_container.items[0].web_element

        return web_container
# usage 
Container().search('text_to_search', _resource='resource_name', _container='container_name')
# _resource and _container will be used by the decorator to locate the search box and activate the
# search