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.

>From 7559aeda6632e39c0bd912c567d7eccc5c7710f2 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Wed, 25 Sep 2013 11:29:31 +0200
Subject: [PATCH] Add automember rebuild command to the web UI

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3928
---
 install/ui/src/freeipa/automember.js | 44 ++++++++++++++++++++++++++++++++++++
 install/ui/src/freeipa/host.js       | 18 ++++++++++++++-
 install/ui/src/freeipa/user.js       | 16 +++++++++++--
 install/ui/test/data/ipa_init.json   | 10 ++++----
 ipalib/plugins/internal.py           | 10 ++++----
 5 files changed, 87 insertions(+), 11 deletions(-)

diff --git a/install/ui/src/freeipa/automember.js b/install/ui/src/freeipa/automember.js
index f8083b89e49ec270ed6170f9826ef4cd45e08865..2dff34227351c582d028a43bc71bc2fe6a7d3142 100644
--- a/install/ui/src/freeipa/automember.js
+++ b/install/ui/src/freeipa/automember.js
@@ -699,12 +699,55 @@ IPA.automember.default_group_widget = function(spec) {
     return that;
 };
 
+IPA.automember.rebuild_action = function(spec) {
+
+    spec = spec || {};
+    spec.name = spec.name || 'automember_rebuild';
+    spec.label = spec.label || '@i18n:actions.automember_rebuild';
+
+    var that = IPA.action(spec);
+
+    that.execute_action = function(facet) {
+        var entity = facet.entity.name;
+        if (facet.name == 'search') {
+            var entries = facet.get_selected_values();
+        } else {
+            entries = facet.get_pkeys();
+        }
+
+        var options = {};
+        if (entries.length > 0) {
+            options[entity + 's'] = entries;
+        } else if (entity == 'user') {
+            options['type'] = 'group';
+        } else {
+            options['type'] = 'hostgroup';
+        }
+
+        var command = IPA.command({
+            entity: 'automember',
+            method: 'rebuild',
+            options: options,
+            on_success: function() {
+                IPA.notify_success('@i18n:actions.automember_rebuild_success');
+            },
+            on_error: function() {
+            }
+        });
+
+        command.execute();
+    };
+
+    return that;
+};
+
 exp.entity_spec = make_spec();
 
 exp.register = function() {
     var e = reg.entity;
     var w = reg.widget;
     var f = reg.field;
+    var a = reg.action;
 
     e.register({
         type: 'automember',
@@ -713,6 +756,7 @@ exp.register = function() {
     });
     w.register('automember_condition', IPA.automember.condition_widget);
     f.register('automember_condition', IPA.automember.condition_field);
+    a.register('automember_rebuild', exp.rebuild_action);
 };
 
 phases.on('registration', exp.register);
diff --git a/install/ui/src/freeipa/host.js b/install/ui/src/freeipa/host.js
index f5007538e8ad1ea2e372c194b129f6c668d31b3e..ff1e5da5b13a0e16d177b902aed877229e1e0137 100644
--- a/install/ui/src/freeipa/host.js
+++ b/install/ui/src/freeipa/host.js
@@ -63,7 +63,16 @@ return {
                     label: '@i18n:objects.host.enrolled',
                     formatter: 'boolean'
                 }
-            ]
+            ],
+            actions: [
+                'select',
+                {
+                    $type: 'automember_rebuild',
+                    name: 'automember_rebuild',
+                    label: '@i18n:actions.automember_rebuild'
+                }
+            ],
+            header_actions: ['select_action', 'automember_rebuild']
         },
         {
             $type: 'details',
@@ -144,6 +153,12 @@ return {
                 }
             ],
             actions: [
+                'select',
+                {
+                    $type: 'automember_rebuild',
+                    name: 'automember_rebuild',
+                    label: '@i18n:actions.automember_rebuild'
+                },
                 'host_unprovision',
                 {
                     $type: 'set_otp',
@@ -165,6 +180,7 @@ return {
                 'cert_revoke',
                 'cert_restore'
             ],
+            header_actions: ['select_action', 'automember_rebuild'],
             state: {
                 evaluators: [
                     IPA.host.has_password_evaluator,
diff --git a/install/ui/src/freeipa/user.js b/install/ui/src/freeipa/user.js
index 1fdcf25a58fd94fa6c11f5106a585bf44614ad67..e20fc1ab2bb408c9c50b40fe25b110a9d6cf714b 100644
--- a/install/ui/src/freeipa/user.js
+++ b/install/ui/src/freeipa/user.js
@@ -61,6 +61,12 @@ return {
                 'title'
             ],
             actions: [
+                'select',
+                {
+                    $type: 'automember_rebuild',
+                    name: 'automember_rebuild',
+                    label: '@i18n:actions.automember_rebuild'
+                },
                 {
                     $type: 'batch_disable',
                     hide_cond: ['self-service']
@@ -70,6 +76,7 @@ return {
                     hide_cond: ['self-service']
                 }
             ],
+            header_actions: ['select_action', 'automember_rebuild'],
             control_buttons: [
                 {
                     name: 'disable',
@@ -234,9 +241,14 @@ return {
                 'enable',
                 'disable',
                 'delete',
-                'reset_password'
+                'reset_password',
+                {
+                    $type: 'automember_rebuild',
+                    name: 'automember_rebuild',
+                    label: '@i18n:actions.automember_rebuild'
+                }
             ],
-            header_actions: ['select_action', 'enable', 'disable', 'delete'],
+            header_actions: ['select_action', 'enable', 'disable', 'delete', 'automember_rebuild'],
             state: {
                 evaluators: [
                     {
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 8e0b2a33cc45d2a68878b7f3ab49b1491ae7c3f5..bac77962e8f3b2629bc80a2a168b2c1daa823270 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -14,10 +14,12 @@
                     },
                     "actions": {
                         "apply": "Apply",
-                        "confirm": "Are you sure you want to proceed with the action.",
-                        "delete_confirm": "Are you sure you want to delete ${object}",
-                        "disable_confirm": "Are you sure you want to disable ${object}",
-                        "enable_confirm": "Are you sure you want to enable ${object}",
+                        "automember_rebuild": "Rebuild auto membership",
+                        "automember_rebuild_success": "Automember rebuild membership task completed",
+                        "confirm": "Are you sure you want to proceed with the action?",
+                        "delete_confirm": "Are you sure you want to delete ${object}?",
+                        "disable_confirm": "Are you sure you want to disable ${object}?",
+                        "enable_confirm": "Are you sure you want to enable ${object}?",
                         "title": "Actions"
                     },
                     "association": {
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 83b505dae1c6349097f7ad5ed20ab25b5a262aa8..0491491274ee9d281dbb35386849221127ad28c5 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -149,10 +149,12 @@ class i18n_messages(Command):
         },
         "actions": {
             "apply": _("Apply"),
-            "confirm": _("Are you sure you want to proceed with the action."),
-            "delete_confirm": _("Are you sure you want to delete ${object}"),
-            "disable_confirm": _("Are you sure you want to disable ${object}"),
-            "enable_confirm": _("Are you sure you want to enable ${object}"),
+            "automember_rebuild": _("Rebuild auto membership"),
+            "automember_rebuild_success": _("Automember rebuild membership task completed"),
+            "confirm": _("Are you sure you want to proceed with the action?"),
+            "delete_confirm": _("Are you sure you want to delete ${object}?"),
+            "disable_confirm": _("Are you sure you want to disable ${object}?"),
+            "enable_confirm": _("Are you sure you want to enable ${object}?"),
             "title": _("Actions"),
         },
         "association": {
-- 
1.8.3.1

>From 7e454901e32e1ffdf7acf435830287cb6fdd5657 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Wed, 25 Sep 2013 11:34:11 +0200
Subject: [PATCH] Web UI integration test driver enhancement

Add a select_option function to handle selecting option from a select box

https://fedorahosted.org/freeipa/ticket/3928
---
 ipatests/test_webui/ui_driver.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py
index 46d7d6794ee5c6e02aad0fa81894371759df8cad..1a1af09f1b722931235aa53e519094c8f26c5889 100644
--- a/ipatests/test_webui/ui_driver.py
+++ b/ipatests/test_webui/ui_driver.py
@@ -698,6 +698,17 @@ def check_option(self, name, value=None, parent=None):
         assert opt is not None, "Option not found: %s" % name
         opt.click()
 
+    def select_option(self, name, value, parent=None):
+        """
+        Select value in a select box.
+        """
+        if not parent:
+            parent = self.get_form()
+
+        opt_s = "select[name=%s] option[value='%s']" % (name, value)
+        option = self.find(opt_s, By.CSS_SELECTOR, parent, strict=True)
+        option.click()
+
     def select_combobox(self, name, value, parent=None):
         """
         Select value in a combobox. Search if not found.
@@ -969,6 +980,8 @@ def fill_fields(self, fields, parent=None, undo=False):
                 self.check_option(key, val, parent)
             elif widget_type == 'checkbox':
                 self.check_option(key, parent=parent)
+            elif widget_type == 'selectbox':
+                self.select_option(key, val, parent)
             elif widget_type == 'combobox':
                 self.select_combobox(key, val, parent)
             elif widget_type == 'add_table_record':
-- 
1.8.3.1

>From 328da8a231de5591dbbbc646515087c3f7a714e1 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Wed, 25 Sep 2013 11:38:07 +0200
Subject: [PATCH] Add web UI integration tests for automember rebuild

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3928
---
 ipatests/test_webui/test_automember.py | 197 +++++++++++++++++++++++++++++++++
 1 file changed, 197 insertions(+)

diff --git a/ipatests/test_webui/test_automember.py b/ipatests/test_webui/test_automember.py
index f51e5d9b1fed593e1522307ab538020121fd95fa..93cebeb40301558b7237f5ed6524652825a12e57 100644
--- a/ipatests/test_webui/test_automember.py
+++ b/ipatests/test_webui/test_automember.py
@@ -81,3 +81,200 @@ def test_crud(self):
 
         # cleanup
         self.delete(hostgroup.ENTITY, [hostgroup.DATA])
+
+    def test_rebuild_membership_hosts(self):
+        """
+        Test automember rebuild membership feature for hosts
+        """
+        self.init_app()
+
+        domain = self.config.get('ipa_domain')
+        host1 = 'web1.%s' % domain
+        host2 = 'web2.%s' % domain
+
+        # Add a hostgroup
+        self.add_record('hostgroup', {
+            'pkey': 'webservers',
+            'add': [
+                ('textbox', 'cn', 'webservers'),
+                ('textarea', 'description', 'webservers'),
+            ]
+        })
+
+        # Add a host
+        self.add_record('host', {
+            'pkey': host1,
+            'add': [
+                ('textbox', 'hostname', 'web1'),
+                ('combobox', 'dnszone', domain),
+                ('checkbox', 'force', 'checked'),
+            ]
+        })
+
+        # Add another host
+        self.add_record('host', {
+            'pkey': host2,
+            'add': [
+                ('textbox', 'hostname', 'web2'),
+                ('combobox', 'dnszone', domain),
+                ('checkbox', 'force', 'checked'),
+            ]
+        })
+
+        # Add an automember rule
+        self.add_record(
+            'automember',
+            {'pkey': 'webservers', 'add': [('combobox', 'cn', 'webservers')]},
+            facet='searchhostgroup'
+        )
+
+        # Add a condition for automember rule
+        self.navigate_to_record('webservers')
+        self.add_table_record(
+            'automemberinclusiveregex',
+            {'fields': [
+                ('selectbox', 'key', 'fqdn'),
+                ('textbox', 'automemberinclusiveregex', '^web[1-9]+')
+            ]}
+        )
+
+        # Assert that hosts are not members of hostgroup
+        self.navigate_to_record('webservers', entity='hostgroup')
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record(host1, negative=True)
+        self.assert_record(host2, negative=True)
+
+        # Rebuild membership for first host, using action on host details facet
+        self.navigate_to_record(host1, entity='host')
+        self.action_list_action('automember_rebuild')
+
+        # Assert that host is now a member of hostgroup
+        self.navigate_to_record('webservers', entity='hostgroup')
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record(host1)
+        self.assert_record(host2, negative=True)
+
+        # Remove host from hostgroup
+        self.delete_record(host1)
+
+        # Assert that host is not a member of hostgroup
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record(host1, negative=True)
+        self.assert_record(host2, negative=True)
+
+        # Rebuild membership for all hosts, using action on hosts search facet
+        self.navigate_by_menu('identity/host')
+        self.navigate_by_breadcrumb('Hosts')
+        self.action_list_action('automember_rebuild')
+
+        # Assert that hosts are now members of hostgroup
+        self.navigate_to_record('webservers', entity='hostgroup')
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record(host1)
+        self.assert_record(host2)
+
+        # Delete hostgroup, hosts and automember rule
+        self.delete('hostgroup', [{'pkey': 'webservers'}])
+        self.delete('host', [{'pkey': host1}, {'pkey': host2}])
+        self.delete('automember', [{'pkey': 'webservers'}],
+                    facet='searchhostgroup')
+
+    def test_rebuild_membership_users(self):
+        """
+        Test automember rebuild membership feature for users
+        """
+        self.init_app()
+
+        # Add a group
+        self.add_record('group', {
+            'pkey': 'devel',
+            'add': [
+                ('textbox', 'cn', 'devel'),
+                ('textarea', 'description', 'devel'),
+            ]
+        })
+
+        # Add a user
+        self.add_record('user', {
+            'pkey': 'dev1',
+            'add': [
+                ('textbox', 'uid', 'dev1'),
+                ('textbox', 'givenname', 'Dev'),
+                ('textbox', 'sn', 'One'),
+            ]
+        })
+
+        # Add another user
+        self.add_record('user', {
+            'pkey': 'dev2',
+            'add': [
+                ('textbox', 'uid', 'dev2'),
+                ('textbox', 'givenname', 'Dev'),
+                ('textbox', 'sn', 'Two'),
+            ]
+        })
+
+        # Add an automember rule
+        self.add_record(
+            'automember',
+            {'pkey': 'devel', 'add': [('combobox', 'cn', 'devel')]},
+            facet='searchgroup'
+        )
+
+        # Add a condition for automember rule
+        self.navigate_to_record('devel')
+        self.add_table_record(
+            'automemberinclusiveregex',
+            {'fields': [
+                ('selectbox', 'key', 'uid'),
+                ('textbox', 'automemberinclusiveregex', '^dev[1-9]+')
+            ]}
+        )
+
+        # Assert that users are not members of group
+        self.navigate_to_record('devel', entity='group')
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record('dev1', negative=True)
+        self.assert_record('dev2', negative=True)
+
+        # Rebuild membership for first user, using action on user details facet
+        self.navigate_to_record('dev1', entity='user')
+        self.action_list_action('automember_rebuild')
+
+        # Assert that user is now a member of group
+        self.navigate_to_record('devel', entity='group')
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record('dev1')
+        self.assert_record('dev2', negative=True)
+
+        # Remove user from group
+        self.delete_record('dev1')
+
+        # Assert that user is not a member of group
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record('dev1', negative=True)
+        self.assert_record('dev2', negative=True)
+
+        # Rebuild membership for all users, using action on users search facet
+        self.navigate_by_menu('identity/user')
+        self.navigate_by_breadcrumb('Users')
+        self.action_list_action('automember_rebuild')
+
+        # Assert that users are now members of group
+        self.navigate_to_record('devel', entity='group')
+        self.facet_button_click('refresh')
+        self.wait_for_request()
+        self.assert_record('dev1')
+        self.assert_record('dev2')
+
+        # Delete group, users and automember rule
+        self.delete('group', [{'pkey': 'devel'}])
+        self.delete('user', [{'pkey': 'dev1'}, {'pkey': 'dev2'}])
+        self.delete('automember', [{'pkey': 'devel'}], facet='searchgroup')
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to