Resolved an issue related to no proper valid/invalid event was

generated for the privilege control. Also, made changes in the
PrivilegeRoleModel, to allow to select a grantee only once for the
current user (as grantor), and omit them from other cell objects.

Valid/Invalid event was not properly triggered, whenever the child
attribute is a collection/model within a model (nested mode/collection
within model). This patch tried to take care of all such scenarios in
general.

web/pgadmin/browser/server_groups/servers/static/js/privilege.js

@@ -54,7 +54,7 @@
    * It has basically three properties:
    *  + grantee    - Role to which that privilege applies to.
    *                Empty value represents to PUBLIC.
-   *  + grantor    - Granter who has given this permission.
+   *  + grantor    - Grantor who has given this permission.
    *  + privileges - Privileges for that role.
    **/
   var PrivilegeRoleModel = pgNode.PrivilegeRoleModel = pgNode.Model.extend({
@@ -72,13 +72,16 @@
     schema: [{
       id: 'grantee', label:'Grantee', type:'text', group: null,
       editable: true, cellHeaderClasses: 'width_percent_40',
-      cell: 'node-list-by-name', node: 'role',
+      node: 'role',
       disabled : function(column, collection) {
         if (column instanceof Backbone.Collection) {
           // This has been called during generating the header cell
           return false;
         }
-        return !(this.node_info && this.node_info.server.user.name == column.get('grantor'));
+        return !(
+          this.node_info &&
+          this.node_info.server.user.name == column.get('grantor')
+        );
       },
       transform: function(data) {
         var res =
@@ -87,7 +90,79 @@
             );
         res.unshift({label: 'public', value: 'public'});
         return res;
-      }
+      },
+      cell: Backgrid.Extension.NodeListByNameCell.extend({
+        initialize: function(opts) {
+          var self = this,
+              override_opts = true;
+
+          // We would like to override the original options, because - we
+          // should omit the existing role/user from the privilege cell.
+          // Because - the column is shared among all the cell, we can only
+          // override only once.
+          if (opts && opts.column &&
+              opts.column instanceof Backbone.Model &&
+              opts.column.has('orig_options')) {
+            override_opts = false;
+          }
+          Backgrid.Extension.NodeListByNameCell.prototype.initialize.apply(
+            self, arguments
+          );
+
+          // Let's override the options
+          if (override_opts) {
+            var opts = self.column.get('options');
+            self.column.set('options', self.omit_selected_roles);
+            self.column.set('orig_options', opts);
+          }
+
+          var rerender = function (m) {
+            var self = this;
+            if ('grantee' in m.changed && this.model.cid != m.cid) {
+              setTimeout(
+                function() {
+                  self.render();
+                }, 50
+              );
+            }
+          }.bind(this);
+
+          // We would like to rerender all the cells of this type for this
+          // collection, because - we need to omit the newly selected roles
+          // form the list. Also, the render will be automatically called for
+          // the model represented by this cell, we will not do that again.
+          this.listenTo(self.model.collection, "change", rerender, this);
+          this.listenTo(self.model.collection, "remove", rerender, this);
+        },
+        // Remove all the selected roles (though- not mine).
+        omit_selected_roles: function() {
+          var self = this,
+              opts = self.column.get('orig_options'),
+              res = opts.apply(this),
+              selected = {},
+              cid = self.model.cid,
+              curr_user = self.model.node_info.server.user.name;
+
+          var idx = 0;
+
+          this.model.collection.each(function(m) {
+            var grantee = m.get('grantee');
+
+            if (m.cid != cid && !_.isUndefined(grantee) &&
+                curr_user == m.get('grantor')) {
+              selected[grantee] = m.cid;
+            }
+          });
+
+          res = _.filter(res, function(o) {
+            return !(o.value in selected);
+          });
+
+          this.model.collection.available_roles = {};
+
+          return res;
+        }
+      }),
     },{
       id: 'privileges', label:'Privileges',
       type: 'collection', model: PrivilegeModel, group: null,
@@ -101,7 +176,7 @@
                 this.attributes.node_info.server.user.name == column.get('grantor'));
       }
     },{
-      id: 'grantor', label: 'Granter', type: 'text', disabled: true,
+      id: 'grantor', label: 'Grantor', type: 'text', disabled: true,
       cell: 'node-list-by-name', node: 'role'
     }],
 
@@ -124,7 +199,10 @@
       /*
        * Define the collection of the privilege supported by this model
        */
-      var privileges = this.get('privileges') || {};
+      var self = this,
+          models = self.get('privileges'),
+          privileges = this.get('privileges') || {};
+
       if (_.isArray(privileges)) {
         privileges = new (pgNode.Collection)(
             models, {
@@ -138,7 +216,7 @@
       }
 
       var privs = {};
-      _.each(this.privileges, function(p) {
+      _.each(self.privileges, function(p) {
         privs[p] = {
           'privilege_type': p, 'privilege': false, 'with_grant': false
         }
@@ -152,20 +230,28 @@
         privileges.add(p, {silent: true});
       });
 
-      this.on("change:grantee", this.granteeChanged)
+      self.on("change:grantee", self.granteeChanged);
-      return this;
+      privileges.on('change', function() {
+        self.trigger('change:privileges', self);
+      });
+
+      return self;
     },
+
     granteeChanged: function() {
       var privileges = this.get('privileges'),
-        grantee = this.get('grantee');
+          grantee = this.get('grantee');
+
       // Reset all with grant options if grantee is public.
       if (grantee == 'public') {
         privileges.each(function(m) {
-          m.set("with_grant", false);
+          m.set("with_grant", false, {silent: true});
         });
       }
     },
+
     toJSON: function(session) {
+
       if (session) {
         return pgNode.Model.prototype.toJSON.apply(this, arguments);
       }
@@ -191,19 +277,32 @@
         errmsg = null,
         changedAttrs = this.sessAttrs,
         msg = undefined;
-      // We will throw error if user have not entered
+
-      // either grantee or privileges
+      if (_.isUndefined(this.get('grantee'))) {
-      if (_.has(changedAttrs, 'grantor')) {
+        msg = window.pgAdmin.Browser.messages.PRIV_GRANTEE_NOT_SPECIFIED;
-          if (_.isUndefined(this.get('grantee')) ||
+        this.errorModel.set('grantee', msg);
-          this.get('privileges').length == 0) {
+        errmsg = msg;
-            errmsg = 'Please specify grantee/privileges';
-            this.errorModel.set('grantee', errmsg);
-            return errmsg;
-          }
       } else {
-        this.errorModel.unset('grantee');
+         this.errorModel.unset('grantee');
       }
-      return null;
+
+      var anyPrivSelected = false;
+      this.attributes['privileges'].each(
+        function(p) {
+          if (p.get('privilege')) {
+            anyPrivSelected = true;
+          }
+        });
+
+      if (!anyPrivSelected) {
+        msg = window.pgAdmin.Browser.messages.NO_PRIV_SELECTED;
+        this.errorModel.set('privileges', msg);
+        errmsg = errmsg || msg;
+      } else {
+        this.errorModel.unset('privileges');
+      }
+
+      return errmsg;
     }
   });
 
@@ -314,6 +413,7 @@
        * Listen to the checkbox value change and update the model accordingly.
        */
       privilegeChanged: function(ev) {
+
         if (ev && ev.target) {
           /*
            * We're looking for checkboxes only.
@@ -327,6 +427,7 @@
               collection = this.model.get('privileges'),
               grantee = this.model.get('grantee');
 
+          this.undelegateEvents();
           /*
            * If the checkbox selected/deselected is for 'ALL', we will select all
            * the checkbox for each privilege.
@@ -395,7 +496,10 @@
            * Set the values for each Privilege Model.
            */
           collection.each(function(m) {
-            m.set({'privilege': allPrivilege, 'with_grant': allWithGrant});
+            m.set(
+              {'privilege': allPrivilege, 'with_grant': allWithGrant},
+              {silent: true}
+            );
           });
         } else {
           /*
@@ -423,12 +527,12 @@
               $allGrant.prop('disabled', true);
               $allGrant.prop('checked', false);
             } else if (grantee != "public") {
-                $elGrant.prop('disabled', false);
+              $elGrant.prop('disabled', false);
             }
           } else if (!checked) {
             $allGrant.prop('checked', false);
           }
-          collection.get(privilege_type).set(attrs);
+          collection.get(privilege_type).set(attrs, {silent: true});
 
           if (checked) {
             var $allPrivileges = $tbl.find(
@@ -454,10 +558,40 @@
             }
           }
         }
+        this.model.trigger('change', this.model);
+
+        var anySelected = false,
+            msg = null;
+
+        collection.each(function(m) {
+          anySelected = anySelected || m.get('privilege');
+        });
+
+        if (anySelected) {
+          this.model.errorModel.unset('privileges');
+          if (this.model.errorModel.has('grantee')) {
+            msg = this.model.errorModel.get('grantee');
+          }
+        } else {
+          this.model.errorModel.set(
+            'privileges', window.pgAdmin.Browser.messages.NO_PRIV_SELECTED
+            );
+          msg = window.pgAdmin.Browser.messages.NO_PRIV_SELECTED;
+        }
+        if (msg) {
+          this.model.collection.trigger(
+            'pgadmin-session:model:invalid', msg, this.model
+            );
+        } else {
+          this.model.collection.trigger(
+            'pgadmin-session:model:valid', this.model
+            );
+        }
       }
+      this.delegateEvents();
     },
 
-      lostFocus: function(ev) {
+    lostFocus: function(ev) {
       /*
        * We lost the focus, it's time for us to exit the editor.
        */
@@ -545,7 +679,7 @@
       var self = this;
       Backgrid.Cell.prototype.initialize.apply(this, arguments);
 
-      self.model.on("change:grantee", function () {
+      self.model.on("change:grantee", function() {
         if (!self.$el.hasClass("editor")) {
           /*
            * Add time out before render; As we might want to wait till model
@@ -560,4 +694,5 @@
   });
 
   return PrivilegeRoleModel;
+
 }));

web/pgadmin/browser/server_groups/servers/templates/servers/servers.js

@@ -38,6 +38,11 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
           category: 'drop', priority: 5, label: '{{ _('Disconnect Server...') }}',
           icon: 'fa fa-chain-broken', enable : 'is_connected'
         }]);
+
+        pgBrowser.messages['PRIV_GRANTEE_NOT_SPECIFIED'] =
+          '{{ _('Please select the grantee from the list!') }}';
+        pgBrowser.messages['NO_PRIV_SELECTED'] =
+          '{{ _('Please select at least one privilege to grant!') }}';
       },
       is_not_connected: function(node) {
         return (node && node.connected != true);

web/pgadmin/browser/static/js/datamodel.js

@@ -266,15 +266,7 @@ function(_, pgAdmin, $, Backbone) {
              *
              * If not parent found, we will raise the issue
              */
-            if (msg != null) {
+            if (_.size(self.errorModel.attributes) == 0) {
-              if (self.collection || self.handler) {
-                (self.collection || self.handler).trigger(
-                    'pgadmin-session:model:invalid', msg, self
-                    );
-              } else {
-                self.trigger('pgadmin-session:invalid', msg, self);
-              }
-            } else if (_.size(self.errorModel.attributes) == 0) {
               if (self.collection || self.handler) {
                 (self.collection || self.handler).trigger(
                     'pgadmin-session:model:valid', self
@@ -282,6 +274,15 @@ function(_, pgAdmin, $, Backbone) {
               } else {
                 self.trigger('pgadmin-session:valid', self.sessChanged(), self);
               }
+            } else {
+              msg = msg || _.values(self.errorModel.attributes)[0];
+              if (self.collection || self.handler) {
+                (self.collection || self.handler).trigger(
+                    'pgadmin-session:model:invalid', msg, self
+                    );
+              } else {
+                self.trigger('pgadmin-session:invalid', msg, self);
+              }
             }
           }
 
@@ -387,6 +388,7 @@ function(_, pgAdmin, $, Backbone) {
         self.on('pgadmin-session:added', self.onChildChanged);
         self.on('pgadmin-session:removed', self.onChildChanged);
       },
+
       onChildInvalid: function(msg, obj) {
         var self = this;
 
@@ -421,18 +423,17 @@ function(_, pgAdmin, $, Backbone) {
             _.findIndex(self.objects, comparator);
           }
 
-          if (objName && !self.errorModel.has(objName)) {
+          if (objName) {
             /*
-             * We will trigger error information only once.
+             * Update the error message for this object.
              */
-            if (!_.size(self.errorModel.attributes)) {
-              if (self.handler) {
-                (self.handler).trigger('pgadmin-session:model:invalid', msg, self);
-              } else  {
-                self.trigger('pgadmin-session:invalid', msg, self);
-              }
-            }
             self.errorModel.set(objName, msg);
+
+            if (self.handler) {
+              (self.handler).trigger('pgadmin-session:model:invalid', msg, self);
+            } else  {
+              self.trigger('pgadmin-session:invalid', msg, self);
+            }
           }
         }
 
@@ -473,54 +474,53 @@ function(_, pgAdmin, $, Backbone) {
               _.findIndex(self.objects, comparator);
           }
 
+          var msg = null,
+              validate = function(m) {
+                if ('validate' in obj && typeof(obj.validate) == 'function') {
+                  msg = obj.validate();
+
+                  return msg;
+                }
+                return null;
+              };
+
+          if (obj instanceof Backbone.Collection) {
+            for (idx in obj.models) {
+              if (validate(obj.models[idx]))
+                break;
+            }
+          } else if (obj instanceof Backbone.Model) {
+            validate(obj);
+          }
+
           if (objName && self.errorModel.has(objName)) {
-
+            if (!msg) {
-            self.errorModel.unset(objName);
+              self.errorModel.unset(objName);
-
-            /*
-             * We will trigger validation information
-             */
-            if (_.size(self.errorModel.attributes) == 0) {
-              if (self.handler) {
-                (self.handler).trigger('pgadmin-session:model:valid', self);
-              } else  {
-                self.trigger(
-                    'pgadmin-session:valid', self.sessChanged(), self
-                    );
-              }
             } else {
-              var msg = _.values(self.errorModel.attributes)[0];
+              self.errorModel.set(objName, msg);
+            }
+          }
 
-              if (self.handler) {
+          /*
-                (self.handler).trigger(
+           * We will trigger validation information
-                    'pgadmin-session:model:invalid', msg, self
+           */
-                    );
+          if (_.size(self.errorModel.attributes) == 0) {
-              } else  {
+            if (self.handler) {
-                self.trigger('pgadmin-session:invalid', msg, self);
+              (self.handler).trigger('pgadmin-session:model:valid', self);
-              }
+            } else  {
+              self.trigger(
+                  'pgadmin-session:valid', self.sessChanged(), self
+                  );
             }
           } else {
-            /*
+            msg = msg || _.values(self.errorModel.attributes)[0];
-             * We will trigger validation information
-             */
-            if (_.size(self.errorModel.attributes) == 0) {
-              if (self.handler) {
-                (self.handler).trigger('pgadmin-session:model:valid', self);
-              } else  {
-                self.trigger(
-                    'pgadmin-session:valid', self.sessChanged(), self
-                    );
-              }
-            } else {
-              var msg = _.values(self.errorModel.attributes)[0];
 
-              if (self.handler) {
+            if (self.handler) {
-                (self.handler).trigger(
+              (self.handler).trigger(
-                    'pgadmin-session:model:invalid', msg, self
+                  'pgadmin-session:model:invalid', msg, self
-                    );
+                  );
-              } else  {
+            } else  {
-                self.trigger('pgadmin-session:invalid', msg, self);
+              self.trigger('pgadmin-session:invalid', msg, self);
-              }
             }
           }
         }
@@ -632,7 +632,7 @@ function(_, pgAdmin, $, Backbone) {
             var msg = m.validate();
 
             if (msg) {
-              self.sessAttrs['invalid'].push(m);
+              self.sessAttrs['invalid'][m.cid] = msg;
             }
           }
         });
@@ -644,15 +644,14 @@ function(_, pgAdmin, $, Backbone) {
         self.on('pgadmin-session:model:valid', self.onModelValid);
       },
       onModelInvalid: function(msg, m) {
-        var self = this;
+        var self = this,
+            invalidModels = self.sessAttrs['invalid'];
 
         if (self.trackChanges) {
           // Do not add the existing invalid object
-          if (self.objFindInSession(m, 'invalid') == -1) {
+          invalidModels[m.cid] = msg;
-            self.sessAttrs['invalid'].push(m);
-          }
 
-          // Inform the parent that - I am an invalid object.
+          // Inform the parent that - I am an invalid model.
           if (self.handler) {
             (self.handler).trigger('pgadmin-session:model:invalid', msg, self);
           } else {
@@ -663,22 +662,17 @@ function(_, pgAdmin, $, Backbone) {
         return true;
       },
       onModelValid: function(m) {
-        var self = this;
+        var self = this,
+            invalidModels = self.sessAttrs['invalid'];
 
         if (self.trackChanges) {
           // Find the object the invalid list, if found remove it from the list
           // and inform the parent that - I am a valid object now.
-          var idx = self.objFindInSession(m, 'invalid');
+          if (m.cid in invalidModels) {
-          if (idx != -1) {
+            delete invalidModels[m.cid];
-            self.sessAttrs['invalid'].splice(m, 1);
           }
 
-          // Inform the parent that - I am the valid object.
+          this.triggerValidationEvent.apply(this);
-          if (self.handler) {
-            (self.handler).trigger('pgadmin-session:model:valid', self);
-          } else {
-            self.trigger('pgadmin-session:valid', self.sessChanged(), self);
-          }
         }
 
         return true;
@@ -799,21 +793,12 @@ function(_, pgAdmin, $, Backbone) {
             msg = obj.validate();
 
             if (msg) {
-              self.sessAttrs['invalid'].push(obj);
+              (self.sessAttrs['invalid'])[obj.cid] = msg;
             }
           }
 
-          /*
+          // Let the parent/listener know about my status (valid/invalid).
-           * If the collection was already invalid, we don't need to inform the
+          this.triggerValidationEvent.apply(this);
-           * parent, or raise the event for the invalid status.
-           */
-          if (!isAlreadyInvalid && !_.isUndefined(msg) && !_.isNull(msg)) {
-            if (self.handler) {
-              self.handler.trigger('pgadmin-session:model:invalid', msg, self);
-            } else {
-              self.trigger('pgadmin-session:invalid', msg, self);
-            }
-          }
 
           return true;
         }
@@ -821,7 +806,7 @@ function(_, pgAdmin, $, Backbone) {
           msg = obj.validate();
 
           if (msg) {
-            self.sessAttrs['invalid'].push(obj);
+            (self.sessAttrs['invalid'])[obj.cid] = msg;
           }
         }
         self.sessAttrs['added'].push(obj);
@@ -831,17 +816,8 @@ function(_, pgAdmin, $, Backbone) {
          */
         (self.handler || self).trigger('pgadmin-session:added', self, obj);
 
-        /*
+        // Let the parent/listener know about my status (valid/invalid).
-         * If the collection was already invalid, we don't need to inform the
+        this.triggerValidationEvent.apply(this);
-         * parent, or raise the event for the invalid status.
-         */
-        if (!isAlreadyInvalid && !_.isUndefined(msg) && !_.isNull(msg)) {
-          if (self.handler) {
-            self.handler.trigger('pgadmin-session:model:invalid', msg, self);
-          } else {
-            self.trigger('pgadmin-session:invalid', msg, self);
-          }
-        }
 
         return true;
       },
@@ -851,29 +827,23 @@ function(_, pgAdmin, $, Backbone) {
           return true;
 
         var self = this,
-            idx = self.objFindInSession(obj, 'added'),
+            invalidModels = self.sessAttrs['invalid'],
-            copy = _.clone(obj);
+            copy = _.clone(obj),
+            idx = self.objFindInSession(obj, 'added');
 
         // We need to remove it from the invalid object list first.
-        if (idx >= 0) {
+        if (obj.cid in invalidModels) {
-          self.sessAttrs['invalid'].splice(idx, 1);
+          delete invalidModels[obj.cid];
         }
 
-        idx = self.objFindInSession(obj, 'added');
-
         // Hmm - it was newly added, we can safely remove it.
         if (idx >= 0) {
           self.sessAttrs['added'].splice(idx, 1);
 
           (self.handler || self).trigger('pgadmin-session:removed', self, copy);
 
-          if (_.size(self.sessAttrs['invalid']) == 0) {
+          // Let the parent/listener know about my status (valid/invalid).
-            if (self.handler) {
+          this.triggerValidationEvent.apply(this);
-              self.handler.trigger('pgadmin-session:model:valid', self);
-            } else {
-              self.trigger('pgadmin-session:valid', self.sessChanged(), self);
-            }
-          }
 
           return true;
         }
@@ -891,25 +861,53 @@ function(_, pgAdmin, $, Backbone) {
 
         self.sessAttrs['deleted'].push(obj);
 
+        // Let the parent/listener know about my status (valid/invalid).
+        this.triggerValidationEvent.apply(this);
+
         /*
-         * This object has been remove, that means - we can safely say, it has been
+         * This object has been remove, we need to check (if we still have any
-         * modified.
+         * other invalid message pending).
          */
-        if (_.size(self.sessAttrs['invalid']) == 0) {
+
-          if (self.handler) {
+        return true;
-            self.handler.trigger('pgadmin-session:model:valid', self);
+      },
-          } else {
+      triggerValidationEvent: function() {
-            self.trigger('pgadmin-session:valid', true, self);
+        var self = this,
+            msg = null,
+            invalidModels = self.sessAttrs['invalid'],
+            validModels = [];
+
+        for (var key in invalidModels) {
+          msg = invalidModels[key];
+          if (msg) {
+            break;
           }
-        } else {
+          else {
-          if (self.handler) {
+            // Hmm..
-            self.handler.trigger('pgadmin-session:model:invalid', self);
+            // How come - you have been assinged in invalid list.
-          } else {
+            // I will make a list of it, and remove it later.
-            self.trigger('pgadmin-session:invalid', true, self);
+            validModels.push(key);
           }
         }
 
-        return true;
+        // Let's remove the un
+        for (key in validModels) {
+          delete invalidModels[validModels[key]];
+        }
+
+        if (!msg) {
+          if (self.handler) {
+            self.handler.trigger('pgadmin-session:model:valid', self);
+          } else {
+            self.trigger('pgadmin-session:valid', self.sessChanged(), self);
+          }
+        } else {
+          if (self.handler) {
+            self.handler.trigger('pgadmin-session:model:invalid', msg, self);
+          } else {
+            self.trigger('pgadmin-session:invalid', msg, self);
+          }
+        }
       },
       onModelChange: function(obj) {
 
@@ -943,7 +941,6 @@ function(_, pgAdmin, $, Backbone) {
 
         if (idx >= 0) {
 
-
           if (!obj.sessChanged()) {
             // This object is no more updated, removing it from the changed
             // models list.