On 06/13/2013 05:18 PM, Petr Vobornik wrote:
> On 06/13/2013 04:16 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> Make sure that the success message in web UI is properly populated with 
>> actual
>> number of items that were successfully added/removed.
>>
>> https://fedorahosted.org/freeipa/ticket/3708
>>
>
> The changes look good. But I've found an existing issue in the code which was
> moved to get_succeeded:
>
> When no operation succeeds following code behaves incorrectly:
>
> +    var succeeded = data.result.completed;
> +
> +    if (!succeeded) {
>
> The condition is evaluated as true when data.result.completed === 0. It's not
> desired because the subsequent commands in the if block will raise JS error
> (data.result doesn't have results obj).
>
> So we should check for:
>     if (typeof succeeded !== 'number')
>
>

Nice catch, thanks. Updated patch attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From e9b240b21c37751e106d8e54e974fd8a58e25c93 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Thu, 13 Jun 2013 16:10:33 +0200
Subject: [PATCH] Fix displaying of success message

Make sure that the success message is properly populated with actual number of
items that were successfully added/removed.

https://fedorahosted.org/freeipa/ticket/3708
---
 install/ui/src/freeipa/association.js | 42 +++++++++++++----------------------
 install/ui/src/freeipa/ipa.js         | 15 +++++++++++++
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index 15e164963e93faddc3e9ddfbc428c1c74df3bf95..c60c7b8afe9c16ae55e5147574664c60afc43d3e 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -609,10 +609,13 @@ IPA.association_table_widget = function (spec) {
         dialog.execute = function() {
             that.add(
                 dialog.get_selected_values(),
-                function() {
+                function(data) {
                     that.refresh();
                     dialog.close();
-                    IPA.notify_success('@i18n:association.added');
+
+                    var succeeded = IPA.get_succeeded(data);
+                    var msg = text.get('@i18n:association.added').replace('${count}', succeeded);
+                    IPA.notify_success(msg);
                 },
                 function() {
                     that.refresh();
@@ -671,9 +674,12 @@ IPA.association_table_widget = function (spec) {
         dialog.execute = function() {
             that.remove(
                 selected_values,
-                function() {
+                function(data) {
                     that.refresh();
-                    IPA.notify_success('@i18n:association.removed');
+
+                    var succeeded = IPA.get_succeeded(data);
+                    var msg = text.get('@i18n:association.removed').replace('${count}', succeeded);
+                    IPA.notify_success(msg);
                 },
                 function() {
                     that.refresh();
@@ -1084,17 +1090,8 @@ exp.association_facet = IPA.association_facet = function (spec, no_init) {
                 on_success: function(data) {
                     that.refresh();
                     dialog.close();
-                    var succeeded = data.result.completed;
-
-                    if (!succeeded) {
-                        succeeded = 0;
-                        for (var i = 0; i< data.result.results.length; i++) {
-                            if (data.result.results[i].completed === 1) {
-                                succeeded++;
-                            }
-                        }
-                    }
 
+                    var succeeded = IPA.get_succeeded(data);
                     var msg = text.get('@i18n:association.added').replace('${count}', succeeded);
                     IPA.notify_success(msg);
                 },
@@ -1148,17 +1145,7 @@ exp.association_facet = IPA.association_facet = function (spec, no_init) {
                 on_success: function(data) {
                     that.refresh();
 
-                    var succeeded = data.result.completed;
-
-                    if (!succeeded) {
-                        succeeded = 0;
-                        for (var i = 0; i< data.result.results.length; i++) {
-                            if (data.result.results[i].completed === 1) {
-                                succeeded++;
-                            }
-                        }
-                    }
-
+                    var succeeded = IPA.get_succeeded(data);
                     var msg = text.get('@i18n:association.removed').replace('${count}', succeeded);
                     IPA.notify_success(msg);
                 },
@@ -1417,7 +1404,10 @@ exp.attribute_facet = IPA.attribute_facet = function(spec, no_init) {
                 function(data) {
                     that.load(data);
                     that.show_content();
-                    IPA.notify_success('@i18n:association.removed');
+
+                    var succeeded = IPA.get_succeeded(data);
+                    var msg = text.get('@i18n:association.removed').replace('${count}', succeeded);
+                    IPA.notify_success(msg);
                 },
                 function() {
                     that.refresh();
diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js
index 441a9052e9e73038ae7344f5cc042c64e6b44d94..24f450e7f1408a34b1236c17547d8534bc9d6745 100644
--- a/install/ui/src/freeipa/ipa.js
+++ b/install/ui/src/freeipa/ipa.js
@@ -1949,6 +1949,21 @@ IPA.notify_success = function(message, timeout) {
     }, timeout || IPA.config.message_timeout);
 };
 
+IPA.get_succeeded = function(data) {
+    var succeeded = data.result.completed;
+
+    if (typeof succeeded !== 'number') {
+        succeeded = 0;
+        for (var i = 0; i< data.result.results.length; i++) {
+            if (data.result.results[i].completed === 1) {
+                succeeded++;
+            }
+        }
+    }
+
+    return succeeded;
+};
+
 IPA.config = {
     default_priority: 500,
     message_timeout: 3000, // [ms]
-- 
1.8.1.4

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

Reply via email to