public inbox for [email protected]  
help / color / mirror / Atom feed
From: Aditya Toshniwal <[email protected]>
To: pgadmin-hackers <[email protected]>
Subject: [pgAdmin][RM5227] Cannot add user
Date: Fri, 10 Apr 2020 10:32:17 +0530
Message-ID: <CAM9w-_=LGp94RH-2Dt4_UtJahCqn_KCNgJLPSak5X3-BPA6phQ@mail.gmail.com> (raw)

Hi Hackers,

Attached is the patch to fix following user management related issues:
1) Unable to add a user when there are many users. The dialog went blank.
Fixed.
2) Error message did not have a close button. Fixed.
3) When clicked on the add button, then cursor will focus on the email box
of the new row for convenience.
4) When the dialog opens, the cursor will focus on the search text box.
5) By default, "User" role will be selected when adding a new user. Also
removed an empty option in the roles dropdown.
6) When the search filter is applied and we try to add an already existing
user, the validation is done only on the filtered data and not on all the
users data. This is fixed.

Please review.

-- 
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


Attachments:

  [application/octet-stream] RM5227.patch (10.1K, 3-RM5227.patch)
  download | inline diff:
diff --git a/web/pgadmin/static/js/backgrid.pgadmin.js b/web/pgadmin/static/js/backgrid.pgadmin.js
index a5833c4e8..7729a02ee 100644
--- a/web/pgadmin/static/js/backgrid.pgadmin.js
+++ b/web/pgadmin/static/js/backgrid.pgadmin.js
@@ -909,12 +909,6 @@ define([
       if (!_.isArray(optionValues))
         throw new TypeError('optionValues must be an array');
 
-      /*
-       * Add empty option as Select2 requires any empty '<option><option>' for
-       * some of its functionality to work.
-       */
-      optionValues.unshift(this.defaults.opt);
-
       var optionText = null,
         optionValue = null,
         self = this,
@@ -923,6 +917,7 @@ define([
           openOnEnter: false,
           multiple: false,
           showOnScroll: true,
+          first_empty: true,
         }, self.defaults.select2,
         (col.select2 || {})
         ),
@@ -932,6 +927,13 @@ define([
         multiple: select2_opts.multiple,
       })).appendTo(self.$el);
 
+      /*
+       * Add empty option as Select2 requires any empty '<option><option>' for
+       * some of its functionality to work.
+       */
+      if(select2_opts.first_empty) {
+        optionValues.unshift(this.defaults.opt);
+      }
       for (var i = 0; i < optionValues.length; i++) {
         var opt = optionValues[i];
 
@@ -1978,11 +1980,13 @@ define([
       this.$customSearchBox = $el;
       this.$customSearchBox.attr('type','search');
       this.$customSearchBox.on('keydown', this.search.bind(this));
+      return this;
     },
 
     unsetCustomSearchBox: function() {
       this.$customSearchBox.off('keydown', this.search.bind(this));
       this.$customSearchBox = null;
+      return this;
     },
   });
 
diff --git a/web/pgadmin/static/scss/_backgrid.overrides.scss b/web/pgadmin/static/scss/_backgrid.overrides.scss
index 4f87cc1be..6b7eada28 100644
--- a/web/pgadmin/static/scss/_backgrid.overrides.scss
+++ b/web/pgadmin/static/scss/_backgrid.overrides.scss
@@ -1,9 +1,12 @@
 .backgrid th, .backgrid td {
-  font-weight: normal!important;
   text-align: left;
   line-height: $line-height-base;
 }
 
+.backgrid td {
+  font-weight: normal!important;
+}
+
 .backgrid.table th.sortable > button {
   width: 100%;
   text-align: left;
@@ -112,6 +115,10 @@
     white-space: pre-wrap;
 }
 
+.backgrid > thead > th.renderable {
+  font-weight: bold;
+}
+
 table.backgrid {
   position: initial;
 }
diff --git a/web/pgadmin/tools/user_management/static/js/user_management.js b/web/pgadmin/tools/user_management/static/js/user_management.js
index 2fbe1dd59..e2a058530 100644
--- a/web/pgadmin/tools/user_management/static/js/user_management.js
+++ b/web/pgadmin/tools/user_management/static/js/user_management.js
@@ -10,11 +10,11 @@
 define([
   'sources/gettext', 'sources/url_for', 'jquery', 'underscore', 'pgadmin.alertifyjs',
   'pgadmin.browser', 'backbone', 'backgrid', 'backform', 'pgadmin.browser.node', 'pgadmin.backform',
-  'pgadmin.user_management.current_user',
+  'pgadmin.user_management.current_user', 'sources/utils',
   'backgrid.select.all', 'backgrid.filter',
 ], function(
   gettext, url_for, $, _, alertify, pgBrowser, Backbone, Backgrid, Backform,
-  pgNode, pgBackform, userInfo
+  pgNode, pgBackform, userInfo, commonUtils,
 ) {
 
   // if module is already initialized, refer to that.
@@ -283,7 +283,7 @@ define([
             username: undefined,
             email: undefined,
             active: true,
-            role: undefined,
+            role: '2',
             newPassword: undefined,
             confirmPassword: undefined,
             auth_source: 'internal',
@@ -361,6 +361,7 @@ define([
             select2: {
               allowClear: false,
               openOnEnter: false,
+              first_empty: false,
             },
             options: function(controlOrCell) {
               var options = [];
@@ -398,6 +399,7 @@ define([
             type: 'switch',
             cell: 'switch',
             cellHeaderClasses: 'width_percent_10',
+            sortable: false,
             editable: function(m) {
               if (m instanceof Backbone.Collection) {
                 return true;
@@ -417,6 +419,7 @@ define([
             cell: PasswordDepCell,
             cellHeaderClasses: 'width_percent_20',
             deps: ['auth_source'],
+            sortable: false,
             editable: function(m) {
               if (m.get('auth_source') == 'internal') {
                 return true;
@@ -433,6 +436,7 @@ define([
             cell: PasswordDepCell,
             cellHeaderClasses: 'width_percent_20',
             deps: ['auth_source'],
+            sortable: false,
             editable: function(m) {
               if (m.get('auth_source') == 'internal') {
                 return true;
@@ -459,7 +463,7 @@ define([
               );
               this.errorModel.set('email', errmsg);
               return errmsg;
-            } else if (!!this.get('email') && this.collection.where({
+            } else if (!!this.get('email') && this.collection.nonFilter.where({
               'email': this.get('email'), 'auth_source': 'internal',
             }).length > 1) {
               errmsg = gettext('The email address %s already exists.',
@@ -739,7 +743,7 @@ define([
                   '        <div class="pr-2"> ',
                   '          <i class="fa fa-exclamation-triangle text-danger" aria-hidden="true"></i> ',
                   '        </div> ',
-                  '        <div class="alert-text" role="status></div> ',
+                  '        <div class="alert-text" role="status"></div> ',
                   '        <div class="ml-auto close-error-bar"> ',
                   '          <a class="close-error fa fa-times text-danger"></a> ',
                   '        </div> ',
@@ -773,6 +777,7 @@ define([
                     var self = this;
                     self.changedUser = null;
                     self.invalidUsers = {};
+                    self.nonFilter = this;
 
                     self.on('add', self.onModelAdd);
                     self.on('remove', self.onModelRemove);
@@ -864,14 +869,14 @@ define([
                 }),
                 userCollection = this.userCollection = new UserCollection(),
                 header =
-                  `<div class="navtab-inline-controls">
+                  `<div class="navtab-inline-controls pgadmin-controls">
                     <div class="input-group">
                       <div class="input-group-prepend">
                         <span class="input-group-text fa fa-search" id="labelSearch"></span>
                         </div>
                           <input type="search" class="form-control" id="txtGridSearch" placeholder="` + gettext('Search') + `" aria-label="Search" aria-describedby="labelSearch" />
                         </div>
-                        <button id="btn_refresh" type="button" class="btn btn-secondary btn-navtab-inline add" title="Add">
+                        <button id="btn_add" type="button" class="btn btn-secondary btn-navtab-inline add" title="Add">
                           <span class="fa fa-plus "></span>
                         </button>
                       </div>
@@ -935,7 +940,7 @@ define([
               this.elements.content.appendChild(this.$content[0]);
 
               // Render Search Filter
-              userFilter(userCollection).setCustomSearchBox($('#txtGridSearch'));
+              userCollection.nonFilter = userFilter(userCollection).setCustomSearchBox($('#txtGridSearch')).shadowCollection;
               userCollection.fetch();
 
               this.$content.find('a.close-error').on('click',() => {
@@ -945,47 +950,33 @@ define([
 
               this.$content.find('button.add').first().on('click',(e) => {
                 e.preventDefault();
-                var canAddRow = true;
-
-                if (canAddRow) {
-                  // There should be only one empty row.
-
-                  var isEmpty = false,
-                    unsavedModel = null;
-
-                  userCollection.each(function(model) {
-                    if (!isEmpty) {
-                      isEmpty = model.isNew();
-                      unsavedModel = model;
-                    }
-                  });
-                  var idx;
-
-                  if (isEmpty) {
-                    idx = userCollection.indexOf(unsavedModel);
-                    var row = view.body.rows[idx].$el;
-
+                // There should be only one empty row.
+                for(const [idx, model] of userCollection.models.entries()) {
+                  if(model.isNew()) {
+                    let row = view.body.rows[idx].$el;
                     row.addClass('new');
-                    $(row).pgMakeVisible('backform-tab');
+                    $(row).pgMakeVisible('backgrid');
+                    $(row).find('.email').trigger('click');
                     return false;
                   }
+                }
 
-                  $(view.body.$el.find($('tr.new'))).removeClass('new');
-                  var m = new(UserModel)(null, {
-                    handler: userCollection,
-                    top: userCollection,
-                    collection: userCollection,
-                  });
-                  userCollection.add(m);
-
-                  idx = userCollection.indexOf(m);
-                  var newRow = view.body.rows[idx].$el;
+                $(view.body.$el.find($('tr.new'))).removeClass('new');
+                var m = new(UserModel)(null, {
+                  handler: userCollection,
+                  top: userCollection,
+                  collection: userCollection,
+                });
+                userCollection.add(m);
 
-                  newRow.addClass('new');
-                  $(newRow).pgMakeVisible('backform-tab');
-                  return false;
-                }
+                var newRow = view.body.rows[userCollection.indexOf(m)].$el;
+                newRow.addClass('new');
+                $(newRow).pgMakeVisible('backgrid');
+                $(newRow).find('.email').trigger('click');
+                return false;
               });
+
+              commonUtils.findAndSetFocus(this.$content);
             },
             callback: function(e) {
               if (e.button.element.name == 'dialog_help') {


view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: [pgAdmin][RM5227] Cannot add user
  In-Reply-To: <CAM9w-_=LGp94RH-2Dt4_UtJahCqn_KCNgJLPSak5X3-BPA6phQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox