public inbox for [email protected]  
help / color / mirror / Atom feed
From: Murtuza Zabuawala <[email protected]>
To: Ashesh Vashi <[email protected]>
Cc: Thom Brown <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Harshal Dhumal <[email protected]>
Cc: Akshay Joshi <[email protected]>
Subject: Re: [PATCH] Tables node (pgAdmin4)
Date: Thu, 26 May 2016 15:50:09 +0530
Message-ID: <CAKKotZQvGr5fdnAzwLigMTqghSFMziv25JwhRT5pBt8hO-pGnA@mail.gmail.com> (raw)
In-Reply-To: <CAG7mmowwyg7ToK4wpnM+mKTMy0eT7uRUsAw84t49KWr4=iJ9aQ@mail.gmail.com>
References: <CAKKotZRCf07F_SGHi4KawBtLHPSeKz5Uvzwu6ddQ=bKc1FDRRw@mail.gmail.com>
	<CAFiP3vw61T0n--F4vYXALdtrkBKznBegBPq5Ds=6uZrg02vWXQ@mail.gmail.com>
	<CAFOhELea6nCtX_T78o_3mNKJC0_emirXKGm3nq8d6mvSZV57pg@mail.gmail.com>
	<CAFiP3vypf7KEmToKsBqn1--qf_PzMV+p-d9bsMj04eoYnj5_tQ@mail.gmail.com>
	<CAKKotZQumWW_cvhvgyFc7_1Lvs8npkPW57FgVN_+wPPSurBDOw@mail.gmail.com>
	<CAKKotZQMR5zsbDAiAhUEQ46yhNE51BONu0DYjEY1WFSJbBqK+w@mail.gmail.com>
	<CAFiP3vwBUX5Ez51Fk_s+t67hRW-uZ6ViMcuTg4e+oPf+k5KqTg@mail.gmail.com>
	<CAFOhELdg9-TB=fdpGDLNkjYve9vBxYttE_PzmahsrNMQv4nn4Q@mail.gmail.com>
	<CAFiP3vxO9tej3WXYeLjZk6MgQYR0C-hp9UUWmog1E7iDvvqFmQ@mail.gmail.com>
	<CAFOhELd=_vuqwR0eGLL=KhxGaQCP6n9mcXP1GijcWh1ecCpegw@mail.gmail.com>
	<CAFiP3vw5nx2Wdt9ci-MNRV+H+dO-x9KUSZtk16EZKjdox+KRxg@mail.gmail.com>
	<CAFiP3vz9=gtNuoGEt+oxt+UYfE=7=dTUQZpCxgGkVWUhS=Kafw@mail.gmail.com>
	<CA+yw=mPp8CY+jd3FUXeV-KSd+82dztXYWySJVLuL9eoE9hrbfQ@mail.gmail.com>
	<CAFiP3vw+yN2kYktjdmJEHL8Ug8R+k_HxhN+E+pozpC=d_zs1=Q@mail.gmail.com>
	<CAKKotZQKV3nkCz8ADOV1ExoSaKWHdH4rY17xW9K3B-4qLree3Q@mail.gmail.com>
	<CAKKotZTiR-cUVTbH21WnYbHW3EaQ3J_zESyYrkL+wO4sKNSSGg@mail.gmail.com>
	<CAKKotZQyTOK4tZYQ2kk4_G-4GknU+uDVu3PaU5FDeJuCe=ywDw@mail.gmail.com>
	<CAM5-9D9R6fT9LOhCBoSbcBvzKpWcjZvN23-wHRTJKsvQqNPECA@mail.gmail.com>
	<CAFiP3vwY_cAx9b-BoxteX5+7fcxVvu+LY4y5ooy28bxvz7D8AA@mail.gmail.com>
	<CAFiP3vwfO=udZvVPdXuEnem9A2LN7MBEQt02VW1AJLuCEsRL2w@mail.gmail.com>
	<CANxoLDcJ-Qum-BJXtDx52ij72EJVzLE+XYFE+_skJF4fXVJ9jA@mail.gmail.com>
	<CA+OCxowB=LJKts18wKx982gF4ZNJnOamND0xZEMp0CnTrjeruA@mail.gmail.com>
	<CAG7mmoyD5vJ3_FS6RO5kmer=CWyRW5Lgr723denYxLM-K00eow@mail.gmail.com>
	<[email protected]>
	<CAKKotZS9MbvFpSkYtRfcLZ8_2drrMpL8T-btH4QXf-BPL0cYzQ@mail.gmail.com>
	<CAG7mmoxJHcTrTFKJGmtVLz-Wix0xpqOmcpHxs141N_Bbx4X7VQ@mail.gmail.com>
	<CAA-aLv46ECN_NZJ=Uercv7mvE+s=vceXqZsb32Qfnh_Oq6OhcA@mail.gmail.com>
	<CAA-aLv758RATV2O4QpO+zXVkiFK0p7AqR86EBtS71JJht=ek_w@mail.gmail.com>
	<CAA-aLv5aaSGcDodnguBdjfGAdZGmGa_7_szfrJSKZHiqvAp_Pw@mail.gmail.com>
	<CAG7mmoyUHGgnspSiYYnkH4i4-GesnP4Bb+T1F2hfo5ufZJ79ZA@mail.gmail.com>
	<CAA-aLv5kk5NWJGPNhr5g0tJ4ChyHiCqOrH5SfBQ00puRkxz+Bg@mail.gmail.com>
	<CAG7mmowwyg7ToK4wpnM+mKTMy0eT7uRUsAw84t49KWr4=iJ9aQ@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi,

PFA patch for RM#1231 & RM#1237.

Additionally I have shifted 'Advanced tab' after columns & constraint tab.


@Thom,

Reading "Of type" drop-down, We borrowed sql query from pgadmin3 to
populate "Of type" combo box as given below.
I was not able to find a way to differentiate between table & composite
type populating from below given query,
I'll put in todo list, may be someone from community can help in future.

SELECT t.oid,
  quote_ident(n.nspname)||'.'||quote_ident(t.typname) AS typname
  FROM pg_type t, pg_namespace n
WHERE t.typtype='c' AND t.typnamespace=n.oid
  AND NOT (n.nspname like 'pg_%' OR n.nspname='information_schema')
ORDER BY typname;


Regards,
Murtuza


On 25-May-2016, at 9:34 pm, Ashesh Vashi <[email protected]>
wrote:

Thanks.
On May 25, 2016 21:01, "Thom Brown" <[email protected]> wrote:

> Thanks.  I've raised all of these in tickets 1231 to 1237.
>
> Thom
>
> On 25 May 2016 at 04:23, Ashesh Vashi <[email protected]>
> wrote:
>
>> Hi Thom,
>>
>> Can you please create RM cases for the same, it will be easy to track
>> those issues?
>> We're using "https://redmine.postgresql.org/projects/pgadmin4/"; for the
>> same.
>>
>> --
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com/;
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi;
>>
>> On Wed, May 25, 2016 at 6:42 AM, Thom Brown <[email protected]> wrote:
>>
>>> On 25 May 2016 at 02:00, Thom Brown <[email protected]> wrote:
>>> > On 25 May 2016 at 00:29, Thom Brown <[email protected]> wrote:
>>> >> On 24 May 2016 at 19:09, Ashesh Vashi <[email protected]>
>>> wrote:
>>> >>>
>>> >>> On Mon, May 23, 2016 at 6:35 PM, Murtuza Zabuawala
>>> >>> <[email protected]> wrote:
>>> >>>>
>>> >>>> Hi,
>>> >>>>
>>> >>>> PFA patch, which will fixes below mentioned issues,
>>> >>>
>>> >>> Committed.
>>> >>
>>> >>
>>> >> In the Create Table dialog, on the Advanced tab, the "Of type"
>>> drop-down
>>> >> lists tables and composite types, but those are supposed to just be
>>> >> composite types.
>>> >>
>>> >> Also, when using OF in CREATE TABLE, LIKE is no longer valid, so LIKE
>>> should
>>> >> be disabled when using OF.  Also, there should probably be a way of
>>> setting
>>> >> options for the columns taken from the composite type.  For example:
>>> >>
>>> >> CREATE TYPE inventory AS (product_id bigint, product_name text, weight
>>> >> numeric);
>>> >>
>>> >> CREATE TABLE stock OF inventory (
>>> >>     PRIMARY KEY (product_id),
>>> >>     weight WITH OPTIONS DEFAULT 0
>>> >> );
>>> >>
>>> >> There's currently no way of doing this (neither the primary key, nor
>>> the
>>> >> default value for any columns).  It should probably automatically
>>> populate
>>> >> the columns from the composite type on the columns tab.
>>> >>
>>> >> Also, could the generated SQL have an empty line between each
>>> statement?
>>> >
>>> > Another thing I've noticed is that, when adding columns, the "Is
>>> > primary key?" column is greyed out.  Why not just make those
>>> > modifiable in that view rather than having to go into the details for
>>> > each column?
>>> >
>>> > When there's something that hasn't been set up correctly, like adding
>>> > variables for a column, but not actually selecting a variable, the SQL
>>> > pane, correctly, doesn't show an output, but the error message,
>>> > "Please provide input for variable." isn't enough to identify what
>>> > needs checking.  Could the relevant tab and field/row be
>>> > highlighted/coloured in red/bold?  This is more general, so doesn't
>>> > just apply to the create table dialog.
>>>
>>> Last issue today:
>>>
>>> When using "Of type", it's not valid to allow additional columns to be
>>> added, so those should be prevented.
>>>
>>> Thom
>>>
>>
>>
>


-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [application/octet-stream] RM_1231_1237.patch (12.9K, 3-RM_1231_1237.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js
index 3c3da63..23c3be6 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js
@@ -327,84 +327,6 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
           type: 'multiline', mode: ['properties', 'create', 'edit'],
           disabled: 'inSchema'
         },{
-          id: 'typname', label:'{{ _('Of type') }}', cell: 'string', control: 'node-ajax-options',
-          type: 'text', mode: ['properties', 'create', 'edit'],
-          disabled: 'checkOfType', url: 'get_oftype', group: '{{ _('Advanced') }}', deps: ['coll_inherits'],
-          transform: function(data, cell) {
-            var control = cell || this,
-              m = control.model;
-              m.of_types_tables = data;
-              return data;
-          },
-          control: Backform.NodeAjaxOptionsControl.extend({
-              // When of_types changes we need to clear columns collection
-              onChange: function() {
-                Backform.NodeAjaxOptionsControl.prototype.onChange.apply(this, arguments);
-                var self = this,
-                  tbl_oid = undefined,
-                  tbl_name = self.model.get('typname'),
-                  data = undefined,
-                  arg = undefined,
-                  column_collection = self.model.get('columns');
-
-                if (!_.isUndefined(tbl_name) &&
-                    tbl_name !== '' && column_collection.length !== 0) {
-                  var msg = '{{ _('Changing of type table will clear columns collection') }}';
-                  alertify.confirm(msg, function (e) {
-                    if (e) {
-                      // User clicks Ok, lets clear columns collection
-                      column_collection.reset();
-                    } else {
-                      return this;
-                    }
-                  });
-                } else if (!_.isUndefined(tbl_name) && tbl_name === '') {
-                  column_collection.reset();
-                }
-
-                // Run Ajax now to fetch columns
-                if (!_.isUndefined(tbl_name) && tbl_name !== '') {
-                  arg = { 'tname': tbl_name }
-                  data = self.model.fetch_columns_ajax.apply(self, [arg]);
-                  // Add into column collection
-                  column_collection.set(data, { merge:false,remove:false });
-                }
-              }
-            })
-        },{
-          id: 'fillfactor', label:'{{ _('Fill factor') }}', cell: 'integer',
-          type: 'int', mode: ['create', 'edit'], min: 10, max: 100,
-          disabled: 'inSchema',group: '{{ _('Advanced') }}'
-        },{
-          id: 'relhasoids', label:'{{ _('Has OIDs?') }}', cell: 'switch',
-          type: 'switch', mode: ['properties', 'create', 'edit'],
-          disabled: 'inSchema', group: '{{ _('Advanced') }}'
-        },{
-          id: 'relpersistence', label:'{{ _('Unlogged?') }}', cell: 'switch',
-          type: 'switch', mode: ['properties', 'create', 'edit'],
-          disabled: 'inSchemaWithModelCheck',
-          group: '{{ _('Advanced') }}'
-        },{
-          id: 'conname', label:'{{ _('Primary key') }}', cell: 'string',
-          type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
-          disabled: 'inSchema'
-        },{
-          id: 'reltuples', label:'{{ _('Rows (estimated)') }}', cell: 'string',
-          type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
-          disabled: 'inSchema'
-        },{
-          id: 'rows_cnt', label:'{{ _('Rows (counted)') }}', cell: 'string',
-          type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
-          disabled: 'inSchema'
-        },{
-          id: 'relhassubclass', label:'{{ _('Inherits tables?') }}', cell: 'switch',
-          type: 'switch', mode: ['properties'], group: '{{ _('Advanced') }}',
-          disabled: 'inSchema'
-        },{
-          id: 'is_sys_table', label:'{{ _('System table?') }}', cell: 'switch',
-          type: 'switch', mode: ['properties'],
-          disabled: 'inSchema'
-        },{
           id: 'coll_inherits', label: '{{ _('Inherited from table(s)') }}',
           url: 'get_inherits', type: 'array', group: '{{ _('Columns') }}',
           disabled: 'checkInheritance', deps: ['typname'],
@@ -492,9 +414,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
             })
         },{
           id: 'coll_inherits', label: '{{ _('Inherited from table(s)') }}',
-          url: 'get_inherits', type: 'text', group: '{{ _('Advanced') }}',
-          disabled: 'checkInheritance',
-          mode: ['properties'],
+          type: 'text', group: '{{ _('Advanced') }}',disabled: 'checkInheritance',
+          mode: ['properties']
         },{
           id: 'inherited_tables_cnt', label:'{{ _('Inherited tables count') }}', cell: 'string',
           type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
@@ -506,7 +427,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
           model: pgBrowser.Nodes['column'].model,
           subnode: pgBrowser.Nodes['column'].model,
           mode: ['create', 'edit'],
-          disabled: 'inSchema',
+          disabled: 'inSchema', deps: ['typname'],
           canAdd: 'check_grid_add_condition',
           canEdit: true, canDelete: true,
           // For each row edit/delete button enable/disable
@@ -678,33 +599,111 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
               }
         }]
         },{
+          id: 'typname', label:'{{ _('Of type') }}', cell: 'string', control: 'node-ajax-options',
+          type: 'text', mode: ['properties', 'create', 'edit'],
+          disabled: 'checkOfType', url: 'get_oftype', group: '{{ _('Advanced') }}', deps: ['coll_inherits'],
+          transform: function(data, cell) {
+            var control = cell || this,
+              m = control.model;
+              m.of_types_tables = data;
+              return data;
+          },
+          control: Backform.NodeAjaxOptionsControl.extend({
+              // When of_types changes we need to clear columns collection
+              onChange: function() {
+                Backform.NodeAjaxOptionsControl.prototype.onChange.apply(this, arguments);
+                var self = this,
+                  tbl_oid = undefined,
+                  tbl_name = self.model.get('typname'),
+                  data = undefined,
+                  arg = undefined,
+                  column_collection = self.model.get('columns');
+
+                if (!_.isUndefined(tbl_name) &&
+                    tbl_name !== '' && column_collection.length !== 0) {
+                  var msg = '{{ _('Changing of type table will clear columns collection') }}';
+                  alertify.confirm(msg, function (e) {
+                    if (e) {
+                      // User clicks Ok, lets clear columns collection
+                      column_collection.reset();
+                    } else {
+                      return this;
+                    }
+                  });
+                } else if (!_.isUndefined(tbl_name) && tbl_name === '') {
+                  column_collection.reset();
+                }
+
+                // Run Ajax now to fetch columns
+                if (!_.isUndefined(tbl_name) && tbl_name !== '') {
+                  arg = { 'tname': tbl_name }
+                  data = self.model.fetch_columns_ajax.apply(self, [arg]);
+                  // Add into column collection
+                  column_collection.set(data, { merge:false,remove:false });
+                }
+              }
+            })
+        },{
+          id: 'fillfactor', label:'{{ _('Fill factor') }}', cell: 'integer',
+          type: 'int', mode: ['create', 'edit'], min: 10, max: 100,
+          disabled: 'inSchema',group: '{{ _('Advanced') }}'
+        },{
+          id: 'relhasoids', label:'{{ _('Has OIDs?') }}', cell: 'switch',
+          type: 'switch', mode: ['properties', 'create', 'edit'],
+          disabled: 'inSchema', group: '{{ _('Advanced') }}'
+        },{
+          id: 'relpersistence', label:'{{ _('Unlogged?') }}', cell: 'switch',
+          type: 'switch', mode: ['properties', 'create', 'edit'],
+          disabled: 'inSchemaWithModelCheck',
+          group: '{{ _('Advanced') }}'
+        },{
+          id: 'conname', label:'{{ _('Primary key') }}', cell: 'string',
+          type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
+          disabled: 'inSchema'
+        },{
+          id: 'reltuples', label:'{{ _('Rows (estimated)') }}', cell: 'string',
+          type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
+          disabled: 'inSchema'
+        },{
+          id: 'rows_cnt', label:'{{ _('Rows (counted)') }}', cell: 'string',
+          type: 'text', mode: ['properties'], group: '{{ _('Advanced') }}',
+          disabled: 'inSchema'
+        },{
+          id: 'relhassubclass', label:'{{ _('Inherits tables?') }}', cell: 'switch',
+          type: 'switch', mode: ['properties'], group: '{{ _('Advanced') }}',
+          disabled: 'inSchema'
+        },{
+          id: 'is_sys_table', label:'{{ _('System table?') }}', cell: 'switch',
+          type: 'switch', mode: ['properties'],
+          disabled: 'inSchema'
+        },{
           type: 'nested', control: 'fieldset', label: '{{ _('Like') }}',
           group: '{{ _('Advanced') }}',
           schema:[{
             id: 'like_relation', label:'{{ _('Relation') }}', cell: 'string',
-            type: 'text', mode: ['create', 'edit'],
+            type: 'text', mode: ['create', 'edit'], deps: ['typname'],
             control: 'node-ajax-options', url: 'get_relations',
-            disabled: 'inSchemaWithModelCheck', group: '{{ _('Like') }}'
+            disabled: 'isLikeDisable', group: '{{ _('Like') }}'
           },{
             id: 'like_default_value', label:'{{ _('With default values?') }}', cell: 'switch',
-            type: 'switch', mode: ['create', 'edit'],
-            disabled: 'inSchemaWithModelCheck', group: '{{ _('Like') }}'
+            type: 'switch', mode: ['create', 'edit'], deps: ['typname'],
+            disabled: 'isLikeDisable', group: '{{ _('Like') }}'
           },{
             id: 'like_constraints', label:'{{ _('With constraints?') }}', cell: 'switch',
-            type: 'switch', mode: ['create', 'edit'],
-            disabled: 'inSchemaWithModelCheck', group: '{{ _('Like') }}'
+            type: 'switch', mode: ['create', 'edit'], deps: ['typname'],
+            disabled: 'isLikeDisable', group: '{{ _('Like') }}'
           },{
             id: 'like_indexes', label:'{{ _('With indexes?') }}', cell: 'switch',
-            type: 'switch', mode: ['create', 'edit'],
-            disabled: 'inSchemaWithModelCheck', group: '{{ _('Like') }}'
+            type: 'switch', mode: ['create', 'edit'], deps: ['typname'],
+            disabled: 'isLikeDisable', group: '{{ _('Like') }}'
           },{
             id: 'like_storage', label:'{{ _('With storage?') }}', cell: 'switch',
-            type: 'switch', mode: ['create', 'edit'],
-            disabled: 'inSchemaWithModelCheck', group: '{{ _('Like') }}'
+            type: 'switch', mode: ['create', 'edit'], deps: ['typname'],
+            disabled: 'isLikeDisable', group: '{{ _('Like') }}'
           },{
             id: 'like_comments', label:'{{ _('With comments?') }}', cell: 'switch',
-            type: 'switch', mode: ['create', 'edit'],
-            disabled: 'inSchemaWithModelCheck', group: '{{ _('Like') }}'
+            type: 'switch', mode: ['create', 'edit'], deps: ['typname'],
+            disabled: 'isLikeDisable', group: '{{ _('Like') }}'
           }]
         },{
           // Here we will create tab control for auto-vacuum
@@ -804,12 +803,24 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
           }
           return true;
         },
+        // We will disable Like if ofType is defined
+        isLikeDisable: function(m) {
+          if(!m.inSchemaWithModelCheck.apply(this, [m]) &&
+              ( _.isUndefined(m.get('typname')) ||
+                _.isNull(m.get('typname')) ||
+                String(m.get('typname')).replace(/^\s+|\s+$/g, '') == '')) {
+            return false;
+          }
+          return true;
+        },
         // Check for column grid when to Add
         check_grid_add_condition: function(m) {
           var enable_flag = true;
           if(!m.inSchema.apply(this, [m])) {
             // if of_type then disable add in grid
-            if (!_.isUndefined(m.get('typname')) && !_.isNull(m.get('typname'))) {
+            if (!_.isUndefined(m.get('typname')) &&
+                !_.isNull(m.get('typname')) &&
+                m.get('typname') !== '') {
                     enable_flag = false;
                 }
           }


view thread (49+ 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], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Tables node (pgAdmin4)
  In-Reply-To: <CAKKotZQvGr5fdnAzwLigMTqghSFMziv25JwhRT5pBt8hO-pGnA@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