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

[Ana Krivokapic]

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.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0072-02-Add-automember-rebuild-command-to-the-web-UI.patch
Type: text/x-patch
Size: 8466 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131016/e6cccb9f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0073-02-Web-UI-integration-test-driver-enhancements.patch
Type: text/x-patch
Size: 1822 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131016/e6cccb9f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0074-02-Add-web-UI-integration-tests-for-automember-rebuild.patch
Type: text/x-patch
Size: 7747 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131016/e6cccb9f/attachment-0002.bin>