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

[Petr Vobornik]

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
-- 
Petr Vobornik