[Freeipa-devel] [PATCHES] 0072-0074 Add automember rebuild membership to the web UI

[Petr Vobornik]

On 11/14/2013 10:55 AM, Petr Vobornik wrote:
> On 10/29/2013 12:33 PM, Ana Krivokapic wrote:
>> On 10/24/2013 03:48 PM, Petr Vobornik wrote:
>>> On 10/16/2013 11:37 PM, Ana Krivokapic wrote:
>>>> On 09/27/2013 04:38 PM, Petr Vobornik wrote:
>>>>> On 09/25/2013 11:51 AM, Ana Krivokapic wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This patch set addresses ticket
>>>>>> https://fedorahosted.org/freeipa/ticket/3928.
>>>>>>
>>>>>> Patch 0072 hooks the new automember-rebuild command to the web UI
>>>>>> (user and host
>>>>>> pages).
>>>>>> Patch 0073 adds some fixes to the web UI test driver, which are
>>>>>> necessary for
>>>>>> patch 0074.
>>>>>> Patch 0074 adds web UI integration tests for the new feature.
>>>>>>
>>>>>> The patch set applies on top of my patches 0068-0071
>>>>>>
>>>>>
>>>>> patch 72: Me, Ana and Kyle discussed position of the actions and the
>>>>> decision was to move them to action list.
>>>>>
>>>>> `ipa automember-rebuild --type=hostgroup` can't be run from UI
>>>>> (already discussed with Ana as well)
>>>>>
>>>>> I'm thinking about adding/creating refresh facet policies for this
>>>>> action so that appropriate facet would be marked as dirty and
>>>>> therefore refreshed so user would not have click association facet
>>>>> refresh button.
>>>>>
>>>>> patch 73: Looks OK but some changes might not be needed.
>>>>>
>>>>> patch 74: I would use different methods for certain actions to be
>>>>> consistent with other tests and to make the test more robust against
>>>>> Web UI refactoring:
>>>>>
>>>>> 1.
>>>>>> self.click_on_link('Refresh')
>>>>>
>>>>> For buttons in .facet-controls use rather
>>>>> `self.facet_button_click('refresh')` where 'refresh' is the button
>>>>> name, not text.
>>>>>
>>>>> 2.
>>>>>> self.add_record(
>>>>>>              'automember',
>>>>>>              {'pkey': 'webservers', 'add': [
>>>>>>                  ('selectbox', 'key', 'fqdn'),
>>>>>>                  ('textbox', 'automemberinclusiveregex',
>>>>>> '^web[1-9]+')
>>>>>>              ]},
>>>>>>              facet='hostgrouprule',
>>>>>>              facet_btn_css_class='widget',
>>>>>>              navigate=False
>>>>>>          )
>>>>>
>>>>> use `add_table_record('automemberinclusiveregex', data, parent)`.
>>>>> Example in test_dns.py:94.
>>>>>
>>>>> 3. A nitpick(or not even that): When working on one entity, I would
>>>>> rather use `navigate_by_breadcrumb('link text') instead of direct
>>>>> `navigate_to_record` call which changes url. It resembles user's
>>>>> behavior more. But it depends on situation. Ie. when I'm on hostgroup
>>>>> page and want to go to host search page, but the last host page was
>>>>> some details or association I may just use `navigate_to_entity('host',
>>>>> 'search'). Anyway, the important thing is to avoid navigating to the
>>>>> same url twice in a row - IE10 does not like that.
>>>>>
>>>>> 4. Do not use `wait(1)` for AJAX calls. The test will fail if there is
>>>>> bigger delay. User rather `wait_for_request(n=X)` where X is number of
>>>>> AJAX calls you want to wait for.
>>>>>
>>>>> 5. Instead of `click_on_link('Rebuild auto membership')` you should
>>>>> use `action_panel_action(panel_name, action)` There is also similar
>>>>> call for
>>>>> executing action list action: `action_list_action(action_name)`.
>>>>>
>>>>> 6. Nitpick: you can reduce the cleanup code:
>>>>>> self.navigate_by_menu('identity/host')
>>>>>> self.delete('host', [{'pkey': host1}])
>>>>>> self.delete('host', [{'pkey': host2}])
>>>>>
>>>>> Can be written as:
>>>>>
>>>>>      self.delete('host', [{'pkey': host1}, {'pkey': host2}])
>>>>>
>>>>> or
>>>>>      self.navigate_by_menu('identity/host')
>>>>>      self.delete_record('host', [host1, host2])
>>>>>
>>>>> `delete` is basically a shortcut for `navigate_to_entity` and
>>>>> `delete_record` with CRUD data format.
>>>>
>>>> All fixed, new patches attached.
>>>>
>>>
>>> Patch 72: ACK
>>>
>>> Patch 73: existing UI_driver.select method has the same functionality
>>> as new
>>> select_option method.
>>>
>>> Patch 74: ACK
>>
>> Oops, I missed the existing select() method. Thanks for poining it
>> out. Updated
>> patch 73 drops the unnecessary select_option() and uses existing
>> select().
>>
>
> ACK

Pushed to master.

-- 
Petr Vobornik