Re: [RM2522] Improve grid/column select all operation

[Murtuza Zabuawala]

I guess so, number of columns do have overhead on selection & copy/paste
operation because of arbitrary copy paste feature, now we have to scan
start cell (first selected cell) & end cell (last selected cell) for each
row, In old code we were selecting complete row which was fast but now user
can select different columns as well, so we have scan for each selected
columns.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jun 30, 2017 at 12:07 PM, Dave Page  wrote:

> Hi
>
> On Thursday, June 29, 2017, Harshal Dhumal  com> wrote:
>
>>
>>
>> On Thu, Jun 29, 2017 at 4:33 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find attached patch for RM2522.
>>>
>>> With this patch grid/column select all time is reduce to ~1 second from
>>> 8-10 seconds.
>>>
>> This benchmarking is performed with 100k rows and 2 columns (int, text).
>>
>
> I see very little improvement (~15s to 14s). My test case has  108786 rows
> in it, and is generated from the query:
>
> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
> union all
> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
> union all
> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
> union all
> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>
> Maybe the number of columns has something to do with it?
>
> AND... I'm still left with nothing being copied to the clipboard (the main
> issue by the way, which no patch for this so far seems to have tackled).
>
> Thanks.
>
>
>>
>>
>>>
>>> The solution was to use simple array concatenation instead of underscore
>>> union while getting index of all selected complete rows.
>>> Underscore union function is only useful when user selects different
>>> ranges from grid and those ranges overlaps. In this case union function
>>> removes duplicate (overlapped) rows.
>>> However result grid in sqleditor do not support overlapped row selection
>>> so we can simply cancat rows from different ranges without worrying about
>>> overlapped row selection.
>>>
>>>
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: [RM2522] Improve grid/column select all operation

[Murtuza Zabuawala]

++ Corrected sentence

On Fri, Jun 30, 2017 at 1:27 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> I guess so, number of columns do have overhead on selection & copy/paste
> operation because of arbitrary copy paste feature, now we have to scan
> start cell (first selected cell) & end cell (last selected cell) for each
> row, In old code we were selecting complete row which was fast but now user
> can select different columns as well, so we have to iterate over each
> selected columns.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, Jun 30, 2017 at 12:07 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thursday, June 29, 2017, Harshal Dhumal > om> wrote:
>>
>>>
>>>
>>> On Thu, Jun 29, 2017 at 4:33 PM, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find attached patch for RM2522.

 With this patch grid/column select all time is reduce to ~1 second from
 8-10 seconds.

>>> This benchmarking is performed with 100k rows and 2 columns (int, text).
>>>
>>
>> I see very little improvement (~15s to 14s). My test case has  108786
>> rows in it, and is generated from the query:
>>
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>> union all
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>> union all
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>> union all
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>>
>> Maybe the number of columns has something to do with it?
>>
>> AND... I'm still left with nothing being copied to the clipboard (the
>> main issue by the way, which no patch for this so far seems to have
>> tackled).
>>
>> Thanks.
>>
>>
>>>
>>>

 The solution was to use simple array concatenation instead of
 underscore union while getting index of all selected complete rows.
 Underscore union function is only useful when user selects different
 ranges from grid and those ranges overlaps. In this case union function
 removes duplicate (overlapped) rows.
 However result grid in sqleditor do not support overlapped row
 selection so we can simply cancat rows from different ranges without
 worrying about overlapped row selection.



 --
 *Harshal Dhumal*
 *Sr. Software Engineer*

 EnterpriseDB India: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

>>>
>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>

Re: [RM2522] Improve grid/column select all operation

[Dave Page]

On Fri, Jun 30, 2017 at 8:57 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> I guess so, number of columns do have overhead on selection & copy/paste
> operation because of arbitrary copy paste feature, now we have to scan
> start cell (first selected cell) & end cell (last selected cell) for each
> row, In old code we were selecting complete row which was fast but now user
> can select different columns as well, so we have scan for each selected
> columns.
>

Yeah, but 14 - 15 seconds for 100K rows? It takes a fraction of that time
to find them in the database, return the results to the pgAdmin server, and
render them.

We should be able to generate a CSV representation of all the selected
fields in far less time.


>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, Jun 30, 2017 at 12:07 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thursday, June 29, 2017, Harshal Dhumal > om> wrote:
>>
>>>
>>>
>>> On Thu, Jun 29, 2017 at 4:33 PM, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find attached patch for RM2522.

 With this patch grid/column select all time is reduce to ~1 second from
 8-10 seconds.

>>> This benchmarking is performed with 100k rows and 2 columns (int, text).
>>>
>>
>> I see very little improvement (~15s to 14s). My test case has  108786
>> rows in it, and is generated from the query:
>>
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>> union all
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>> union all
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>> union all
>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>>
>> Maybe the number of columns has something to do with it?
>>
>> AND... I'm still left with nothing being copied to the clipboard (the
>> main issue by the way, which no patch for this so far seems to have
>> tackled).
>>
>> Thanks.
>>
>>
>>>
>>>

 The solution was to use simple array concatenation instead of
 underscore union while getting index of all selected complete rows.
 Underscore union function is only useful when user selects different
 ranges from grid and those ranges overlaps. In this case union function
 removes duplicate (overlapped) rows.
 However result grid in sqleditor do not support overlapped row
 selection so we can simply cancat rows from different ranges without
 worrying about overlapped row selection.



 --
 *Harshal Dhumal*
 *Sr. Software Engineer*

 EnterpriseDB India: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

>>>
>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [RM2522] Improve grid/column select all operation

[Harshal Dhumal]

Hi,

This patch is not related to improve copy/past operation. It's only related
to grid selection.
The improvement is only between when user clicks on grid select all column
to until all row are selected (turned into blue). With large columns though
improvement is not large but it not putting any overhead. Other hand with
less columns but large row count the improvement it quite noticeable. Code
change is only single line to use array concatenation instead of underscore
union . I had look at union function
implementation. It first flattens result set recursively and then removes
duplicate which is not required in our case. All we want is indexes of
complete rows in selected rows (ranges).





-- 
*Harshal Dhumal*
*Sr. Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jun 30, 2017 at 1:30 PM, Dave Page  wrote:

>
>
> On Fri, Jun 30, 2017 at 8:57 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> I guess so, number of columns do have overhead on selection & copy/paste
>> operation because of arbitrary copy paste feature, now we have to scan
>> start cell (first selected cell) & end cell (last selected cell) for each
>> row, In old code we were selecting complete row which was fast but now user
>> can select different columns as well, so we have scan for each selected
>> columns.
>>
>
> Yeah, but 14 - 15 seconds for 100K rows? It takes a fraction of that time
> to find them in the database, return the results to the pgAdmin server, and
> render them.
>
> We should be able to generate a CSV representation of all the selected
> fields in far less time.
>
>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Fri, Jun 30, 2017 at 12:07 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thursday, June 29, 2017, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>


 On Thu, Jun 29, 2017 at 4:33 PM, Harshal Dhumal <
 harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch for RM2522.
>
> With this patch grid/column select all time is reduce to ~1 second
> from 8-10 seconds.
>
 This benchmarking is performed with 100k rows and 2 columns (int, text).

>>>
>>> I see very little improvement (~15s to 14s). My test case has  108786
>>> rows in it, and is generated from the query:
>>>
>>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>>> union all
>>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>>> union all
>>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>>> union all
>>> select * from pg_class c, pg_attribute a where c.oid = a.attrelid
>>>
>>> Maybe the number of columns has something to do with it?
>>>
>>> AND... I'm still left with nothing being copied to the clipboard (the
>>> main issue by the way, which no patch for this so far seems to have
>>> tackled).
>>>
>>> Thanks.
>>>
>>>


>
> The solution was to use simple array concatenation instead of
> underscore union while getting index of all selected complete rows.
> Underscore union function is only useful when user selects different
> ranges from grid and those ranges overlaps. In this case union function
> removes duplicate (overlapped) rows.
> However result grid in sqleditor do not support overlapped row
> selection so we can simply cancat rows from different ranges without
> worrying about overlapped row selection.
>
>
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

pgAdmin 4 commit: Improve speed of Select All in the results grid. Fixe

[Dave Page]

Improve speed of Select All in the results grid. Fixes #2522

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=2d653129108179af270a5cd6e65b758d437f6176
Author: Harshal Dhumal 

Modified Files
--
web/pgadmin/static/js/selection/range_selection_helper.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Re: [RM2522] Improve grid/column select all operation

[Dave Page]

On Fri, Jun 30, 2017 at 9:39 AM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> This patch is not related to improve copy/past operation. It's only
> related to grid selection.
> The improvement is only between when user clicks on grid select all column
> to until all row are selected (turned into blue). With large columns though
> improvement is not large but it not putting any overhead. Other hand with
> less columns but large row count the improvement it quite noticeable. Code
> change is only single line to use array concatenation instead of underscore
> union . I had look at union function
> implementation. It first flattens result set recursively and then removes
> duplicate which is not required in our case. All we want is indexes of
> complete rows in selected rows (ranges).
>
>
Ah, I see.

In that case, patch applied. Thanks!

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][Patch]: Feature #2506 - Allow the dashboard panel to be closed

[Dave Page]

Thanks - seems to work for me :-). Patch applied.

On Thu, Jun 29, 2017 at 1:35 PM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Wed, Jun 28, 2017 at 7:08 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Jun 27, 2017 at 11:54 PM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Tue, Jun 27, 2017 at 8:08 PM, Dave Page  wrote:
>>>
 Hi

 I've had to revert this. Whilst it seems to work, after showing/hiding
 the dashboard, I later find that when I completely reload the app, it
 fails, leaving just the object menu present. I haven't come up with a
 concrete case to reproduce it. In the console, I see:


 wcDocker.min.js:38 Uncaught TypeError: Cannot read property '__save'
 of null at e.save (wcDocker.min.js:38) at Object.save_current_layout (
 browser.js:340) at e.handleVisibility (panel.js:156) at e.__trigger (
 wcDocker.min.js:34) at e.trigger (wcDocker.min.js:38) at e.clear (
 wcDocker.min.js:38) at Object.init (browser.js:386) at (index):278 at
 Object.execCb (require.min.js:29) at Z.check (require.min.js:18)

 I couldn't reproduce this issue. I have tried many things to reproduce
>>> it but not succeeded. I also switched server_mode but no luck.
>>> I think there should be any specific condition that fails this but have
>>> no clue.
>>>
>>> I have rebased this patch and attached.
>>>
>>
>> Thanks. I applied the patch, restarted my server, and did a hard-reload
>> in my browser and immediately saw the problem - see the attached screenshot.
>>
> Fixed
>
>>
>>
> Clearing the saved browser layout from the config DB solved the problem
>> for me (once I refreshed the browser), so something is likely getting
>> messed up in there. I've attached the value that was previously saved as
>> inserting that into a test database will likely be easier than trying to
>> reproduce the issue.
>>
>> Thanks!
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgAdmin 4 commit: Allow the dashboard panel to be closed. Fixes #2506

[Dave Page]

Allow the dashboard panel to be closed. Fixes #2506

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=501120701eecfda0fed14dcc11edf54f2ac333f3
Author: Khushboo Vashi 

Modified Files
--
web/pgadmin/browser/static/js/panel.js | 38 +-
.../browser/templates/browser/js/browser.js| 24 --
web/pgadmin/dashboard/__init__.py  |  8 +++--
.../dashboard/templates/dashboard/js/dashboard.js  | 10 +-
.../datagrid/templates/datagrid/js/datagrid.js |  4 +--
.../debugger/templates/debugger/js/debugger.js |  4 +--
.../debugger/templates/debugger/js/debugger_ui.js  |  2 +-
web/pgadmin/utils/menu.py  |  4 ++-
8 files changed, 73 insertions(+), 21 deletions(-)

pgAdmin 4 commit: Update alertify alerts to use the styling defined in

[Dave Page]

Update alertify alerts to use the styling defined in the styleguide.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=2a30a86e7d5e562040500f448fbb0d143ff2cff9
Author: Joao Pedro De Almeida Pereira 

Modified Files
--
.../servers/databases/casts/static/js/cast.js  |  10 +-
.../schemas/synonyms/static/js/synonym.js  |   7 +-
.../check_constraint/js/check_constraint.js|  10 +-
.../templates/foreign_key/js/foreign_key.js|  10 +-
.../databases/schemas/tables/static/js/table.js|  21 ++--
.../triggers/templates/trigger/js/trigger.js   |  15 ++-
.../databases/schemas/views/static/js/mview.js |   7 +-
.../servers/databases/static/js/database.js|  16 ++-
.../servers/pgagent/static/js/pga_job.js   |  13 ++-
.../server_groups/servers/static/js/server.js  |  55 ++---
.../templates/tablespaces/js/tablespaces.js|  13 ++-
web/pgadmin/browser/templates/browser/js/node.js   |  24 ++--
.../templates/file_manager/js/file_manager.js  |  11 +-
.../templates/file_manager/js/utility.js   |  44 ---
web/pgadmin/static/css/bootstrap.overrides.css | 127 +
web/pgadmin/static/scss/pgadmin.scss   |   6 +
.../tools/backup/templates/backup/js/backup.js |  14 +--
.../tools/debugger/templates/debugger/js/direct.js |  51 +++--
.../templates/import_export/js/import_export.js|   7 +-
.../templates/maintenance/js/maintenance.js|  13 ++-
.../tools/restore/templates/restore/js/restore.js  |  10 +-
.../tools/sqleditor/static/css/sqleditor.css   |   6 +-
.../sqleditor/templates/sqleditor/js/sqleditor.js  |  27 +++--
.../user_management/js/user_management.js  |  15 ++-
24 files changed, 253 insertions(+), 279 deletions(-)

pgAdmin 4 commit: Add support for SCSS building in webpack.

[Dave Page]

Add support for SCSS building in webpack.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=ba3ab7b322ef940a53e2e4309b39855d1bac0b66
Author: Joao Pedro De Almeida Pereira 

Modified Files
--
web/package.json |   3 +
web/pgadmin/static/scss/pgadmin.scss |   0
web/pgadmin/templates/base.html  |   2 +-
web/webpack.config.js|  18 ++
web/yarn.lock| 356 +--
5 files changed, 361 insertions(+), 18 deletions(-)

pgAdmin 4 commit: Update alertify alerts to use the styling defined in

[Dave Page]

Update alertify alerts to use the styling defined in the styleguide (missed 
some files in the original commit).

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=f2d2075d81718ec02550fb592851aa330d327b24
Author: Joao Pedro De Almeida Pereira 

Modified Files
--
web/pgadmin/static/js/alerts/alertify_wrapper.js   |  40 +
web/pgadmin/static/scss/_alert.scss| 119 +++
web/pgadmin/static/scss/_alertify.overrides.scss   | 166 +
web/pgadmin/static/scss/_othercolors.scss  | 156 +++
web/pgadmin/static/scss/_typography.scss   |  68 +
.../javascript/alerts/alertify_wrapper_spec.js |  41 +
6 files changed, 590 insertions(+)

Re: [pgadmin-hackers][Patch] Change alert styling

[Dave Page]

OK, thanks - patch applied.

Note to all hackers: As we modify our override CSS moving forwards, it
should be migrated to SCSS, and use the defined colour variable names.

Thanks.



On Thu, Jun 29, 2017 at 2:25 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi!
>
> Are you committing to migrating all the main CSS to SCSS? Whilst I can see
>> the advantages of SCSS, I don't want to end up with a mix and match mess of
>> the two (though, I'd be fine with keeping the smaller, module-specific CSS
>> if it doesn't make sense to migrate it).
>
>
> It makes more sense to switch the CSS over as we need it and change the
> classes to be using the correct variables and nest them appropriately.
>
>
> What is the advantage of this over simply overriding the existing styles?
>
>
> We can't simply override the styles because we are adding an icon to the
> alert. So since we have to add a wrapper anyway we decided to style it like
> we normally would.
>
> Thanks,
> João & Sarah
>
> On Wed, Jun 28, 2017 at 8:51 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Wed, Jun 28, 2017 at 4:40 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Hackers,
>>>
>>> In this patch we are introducing SCSS that we generated for the style
>>> guide.
>>> [image: Inline image 1]
>>>
>>> The patch is split into 2 files:
>>> - 01-add-scss-building-to-webpack.diff
>>> In this file we add the compilation of SCSS into CSS to webpack
>>>
>>
>> Are you committing to migrating all the main CSS to SCSS? Whilst I can
>> see the advantages of SCSS, I don't want to end up with a mix and match
>> mess of the two (though, I'd be fine with keeping the smaller,
>> module-specific CSS if it doesn't make sense to migrate it).
>>
>>
>>>
>>> - 02-change-alert-messages.diff
>>> In this file we create a Wrapper for Alertify's success and error
>>> function so we can introduce new styling on them. We change all the places
>>> in the app where alertify.success, alertify.notify (with the type 'error'
>>> or 'success') and alertify.error are called.
>>>
>>>
>> What is the advantage of this over simply overriding the existing styles?
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgAdmin 4 commit: Fix a couple of gripes from the JS linter on Windows.

[Dave Page]

Fix a couple of gripes from the JS linter on Windows.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=ffb00b502ab149234b27aeaf38d418707bb6085a

Modified Files
--
.../event_handlers/handle_query_output_keyboard_event_spec.js| 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

Re: [pgAdmin4]: Webpacking of static JS/CSS

[Surinder Kumar]

Hi

On Fri, Jun 30, 2017 at 2:29 AM, George Gelashvili 
wrote:

>  Hey Surinder,
>
>
> ​1. ​
>> Tools
>> ​(tools.js)​ - It will contain all JS modules under tools directory
>>
>>
>> ​2. ​
>> Browser
>> ​(browser.js)​ - It will contain all JS modules under browser directory
>>
>
> By under, do you mean every javascript file recursively under the browser
> directory?
>
​No, I mean a single bundled javascript ​file of various nodes static JS
files.

>
>
>> ​4. Common.js and/or vendor.js - Common.js will contains all common js
>> files/libraries shared by modules like backform, backbone, underscore etc.
>>
>
> We're okay with bundling vendorized code for a first pass to webpack
> everything. The goal should be to pull code out of the shared javascript
> bundle as we unvendor dependencies.
>
Yes, bundled vendor.js will have all unvendor dependencies and files will
refer from node_modules.

> For naming: Common.js is an overloaded term (http://requirejs.org/docs/com
> monjs.html). We prefer vendor.js. Will there be non-vendor code put into
> this bundle?
> ​I think vendor.js should only contain vendor files. We can create another
> file(xyz) that will contains JS files shared across the modules.
>
> ​What do you think ?​
>
>
​
>
>> Entry points JS: main.js - It will load above modules ascynchronusly or
>> it will be minified version of all.
>> define('', ['common', 'tools', 'browser', 'grid'], function(...) {});
>>
> or these modules can also be loaded using dependency; For example, load
>> grid.js on database node expand.
>>
>
> Could you go into some more detail on this decision? How would main.js be
> used in the app? Is the idea to still use define from require.js in
> javascript being webpacked?
>
​In pgAdmin4 there is no main.js, but entry point is browser/index.html.​
This is how I think bundled JS will load:

Load vendor.js using  in base.html

*​browser/​index.html*

require(
​  ​
['pgadmin',
​'sources/generated/common', ​
'sources/generated/bundled'],
​  ​
function() {
}
​)​

*sources/generated/bundled​.js​*

​define
(
​  ​
['
​sources/generated/browser
', 'sources/generated/
​tools
'
​, 'sources/generated/grid'​
],
​  ​
function
​(​
pgBrowser) {
​  pgBrowser.init();​
}
)​
​


​or we can use 'import'​ instead of 'require' for files that will be
bundled like 'bundled.js'
I keep require/define call untouched in module JS, and webpack converts
'define' calls to __webpack_require__(module_id) to load dependent modules.

>
> ​Currently I am working on converting 'requirejs config shim dependency
>> into webpack' using imports-loader and exports-loader which is taking
>> time and will look for alternative if it doesn't works. Once dependency is
>> properly defined, the task to generate other module JS will become quite
>> easy.
>>
>
> We used imports-loader and exports-loader for tests (see
> web/webpack.test.config.js)
>
​Hmm. it is useful.​ but I am using shim-loader
 which is compatible with
requirejs shim format.

>
>
>> I also found, the CSS imported using import 'slickgrid/slick.grid.css';
>>  statement is put into  tags
>> ​ in html​
>> , instead must be loaded separately as a file so overrides.css can work.
>>
>
> Those three css files from slickgrid need to be brought into the
> application somehow because they won't be available after building the app,
> since SlickGrid has been un-vendored. We opted to webpack the css together
> with the required slickgrid js files. Can the overrides.css files be
> modified to work with this bundle? Which overrides.css file is not
> working now?
>
​This affects the CSS font styles of edit grid header or might affect other
changes as well. we'll have to look how to override vendor slick grid css
which is now rendered in HTML markup.


> Cheers,
> Matt and George
>

Build failed in Jenkins: pgadmin4-master-python33 #207

[pgAdmin 4 Jenkins]

See 


Changes:

[Dave Page] Fix a couple of gripes from the JS linter on Windows.

--
[...truncated 289.08 KB...]
Update materialized view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_add.DatabaseAddTestCase)
Check Databases Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_delete.DatabaseDeleteTestCase)
Check Databases Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_get.DatabasesGetTestCase)
Check Databases Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_put.DatabasesUpdateTestCase)
Check Databases Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.test_resource_groups_add.ResourceGroupsAddTestCase)
Add resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.test_resource_groups_delete.ResourceGroupsDeleteTestCase)
Delete resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.test_resource_groups_put.ResourceGroupsPutTestCase)
Put resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.tests_resource_groups_get.ResourceGroupsGetTestCase)
Get resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_add.LoginRoleAddTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_delete.LoginRoleDeleteTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_get.LoginRoleGetTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_put.LoginRolePutTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_add.TableSpaceAddTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_delete.TableSpaceDeleteTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_get.TablespaceGetTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_put.TableSpaceUpdateTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_check_recovery.TestCheckRecovery)
Test for check recovery ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_dependencies_sql.TestDependenciesSql)
Test dependencies SQL file ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_dependents_sql.TestDependentsSql)
Test dependencies SQL file ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_role_dependencies_sql.TestRoleDependenciesSql)
Test Role Dependencies SQL file ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_server_add.ServersAddTestCase)
Default Server Node url ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_server_delete.ServerDeleteTestCase)
Default Server Node url ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_server_get.ServersGetTestCase)
Default Server Node url ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_server_put.ServerUpdateTestCase)
Default Server Node url ... ok
runTest (pgadmin.browser.server_groups.tests.test_sg_get.SgNodeTestCase)
Check Server Group Node ... ok
runTest 
(pgadmin.utils.javascript.tests.test_javascript_bundler.JavascriptBundlerTestCase)
scenario name: JavascriptBundlerTestCase ... ok
runTest 
(pgadmin.utils.tests.test_versioned_template_loader.TestVersionedTemplateLoader)
Render a template when called ... ok
runTest 
(pgadmin.utils.tests.test_versioned_template_loader.TestVersionedTemplateLoader)
Render a version 9.1 template when it is present ... ok
runTest 
(pgadmin.utils.tests.test_versioned_template_loader.TestVersionedTemplateLoader)
Render a version 9.2 template when request for a higher version ... ok
runTest 
(pgadmin.utils.tests.test_versioned_template_loader.TestVersionedTemplateLoader)
Render default version when version 9.0 was requested and only 9.1 and 9.2 are 
present ... ok
runTest 
(pgadmin.utils.tests.test_versioned_template_loader.TestVersionedTemplateLoader)
Raise error when version is smaller than available templates ... ok
runTest 
(regression.python_test_utils.sql_template_test_base.SQLTemplateTestBase)
parent test class ... ok

--
Ran 163 tests in 25.568s

OK

==
Test Result Summary
==

PostgreSQL 9.6:

151 tests passed
0 tests failed
12 tests skipped:
PackagePutTestCase (Fetch Packa

Re: [pgAdmin4]: Webpacking of static JS/CSS

[Surinder Kumar]

Hi

So far, I have been able to generate bundled 'sources/generated/browser.js'
which contains only three browser modules (Server-group, Server, Database)
and load it from 'browser/index.html'(entry point) and it works(except few
JS error present in file). so will fix them.

Then I will add one by one dashboard module, nodes(cast, schema etc.),
tools which i earlier removed(because their JS had conflicts with vendor
JS) and make changes accordingly to webpack config and make other modules
to load properly.

So far the JS files that won't be the part of  bundled JS and will load
separately according to dependency defined in externals in webpack config:

1. browser/utils.js - This file is extracted from browser.js which contains
Jinja syntax related code for menus, panels and preference related settings
from browser.js
2. endpoints.js
3. messages.js
4. supported_servers.js
5. current_user.js
6. gettext.js
7. translations.js

All other JS files will go into bundled JS.

Currently replacing '{{ url_for }}' in dashboard JS with client side
'url_for' and load with bundled JS.

Thanks,
Surinder

On Fri, Jun 30, 2017 at 3:34 PM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi
>
> On Fri, Jun 30, 2017 at 2:29 AM, George Gelashvili  > wrote:
>
>>  Hey Surinder,
>>
>>
>> ​1. ​
>>> Tools
>>> ​(tools.js)​ - It will contain all JS modules under tools directory
>>>
>>>
>>> ​2. ​
>>> Browser
>>> ​(browser.js)​ - It will contain all JS modules under browser directory
>>>
>>
>> By under, do you mean every javascript file recursively under the browser
>> directory?
>>
> ​No, I mean a single bundled javascript ​file of various nodes static JS
> files.
>
>>
>>
>>> ​4. Common.js and/or vendor.js - Common.js will contains all common js
>>> files/libraries shared by modules like backform, backbone, underscore etc.
>>>
>>
>> We're okay with bundling vendorized code for a first pass to webpack
>> everything. The goal should be to pull code out of the shared javascript
>> bundle as we unvendor dependencies.
>>
> Yes, bundled vendor.js will have all unvendor dependencies and files will
> refer from node_modules.
>
>> For naming: Common.js is an overloaded term (
>> http://requirejs.org/docs/commonjs.html). We prefer vendor.js. Will
>> there be non-vendor code put into this bundle?
>> ​I think vendor.js should only contain vendor files. We can create
>> another file(xyz) that will contains JS files shared across the modules.
>>
>> ​What do you think ?​
>>
>>
> ​
>>
>>> Entry points JS: main.js - It will load above modules ascynchronusly or
>>> it will be minified version of all.
>>> define('', ['common', 'tools', 'browser', 'grid'], function(...) {});
>>>
>> or these modules can also be loaded using dependency; For example, load
>>> grid.js on database node expand.
>>>
>>
>> Could you go into some more detail on this decision? How would main.js be
>> used in the app? Is the idea to still use define from require.js in
>> javascript being webpacked?
>>
> ​In pgAdmin4 there is no main.js, but entry point is browser/index.html.​
> This is how I think bundled JS will load:
>
> Load vendor.js using  in base.html
>
> *​browser/​index.html*
>
> require(
> ​  ​
> ['pgadmin',
> ​'sources/generated/common', ​
> 'sources/generated/bundled'],
> ​  ​
> function() {
> }
> ​)​
>
> *sources/generated/bundled​.js​*
>
> ​define
> (
> ​  ​
> ['
> ​sources/generated/browser
> ', 'sources/generated/
> ​tools
> '
> ​, 'sources/generated/grid'​
> ],
> ​  ​
> function
> ​(​
> pgBrowser) {
> ​  pgBrowser.init();​
> }
> )​
> ​
>
>
> ​or we can use 'import'​ instead of 'require' for files that will be
> bundled like 'bundled.js'
> I keep require/define call untouched in module JS, and webpack converts
> 'define' calls to __webpack_require__(module_id) to load dependent modules.
>
>>
>> ​Currently I am working on converting 'requirejs config shim dependency
>>> into webpack' using imports-loader and exports-loader which is taking
>>> time and will look for alternative if it doesn't works. Once dependency is
>>> properly defined, the task to generate other module JS will become quite
>>> easy.
>>>
>>
>> We used imports-loader and exports-loader for tests (see
>> web/webpack.test.config.js)
>>
> ​Hmm. it is useful.​ but I am using shim-loader
>  which is compatible with
> requirejs shim format.
>
>>
>>
>>> I also found, the CSS imported using import 'slickgrid/slick.grid.css';
>>>  statement is put into  tags
>>> ​ in html​
>>> , instead must be loaded separately as a file so overrides.css can work.
>>>
>>
>> Those three css files from slickgrid need to be brought into the
>> application somehow because they won't be available after building the app,
>> since SlickGrid has been un-vendored. We opted to webpack the css together
>> with the required slickgrid js files. Can the overrides.css files be
>> modified to work with this bundle? Which overrides.css file is not
>> working now?
>>
> ​This affects the CSS 

Re: [pgAdmin4]: Webpacking of static JS/CSS

[Dave Page]

Awesome work. Do you have an ETA for a feature complete test patch?

On Fri, Jun 30, 2017 at 11:41 AM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi
>
> So far, I have been able to generate bundled
> 'sources/generated/browser.js' which contains only three browser modules
> (Server-group, Server, Database) and load it from
> 'browser/index.html'(entry point) and it works(except few JS error present
> in file). so will fix them.
>
> Then I will add one by one dashboard module, nodes(cast, schema etc.),
> tools which i earlier removed(because their JS had conflicts with vendor
> JS) and make changes accordingly to webpack config and make other modules
> to load properly.
>
> So far the JS files that won't be the part of  bundled JS and will load
> separately according to dependency defined in externals in webpack config:
>
> 1. browser/utils.js - This file is extracted from browser.js which
> contains Jinja syntax related code for menus, panels and preference related
> settings from browser.js
> 2. endpoints.js
> 3. messages.js
> 4. supported_servers.js
> 5. current_user.js
> 6. gettext.js
> 7. translations.js
>
> All other JS files will go into bundled JS.
>
> Currently replacing '{{ url_for }}' in dashboard JS with client side
> 'url_for' and load with bundled JS.
>
> Thanks,
> Surinder
>
> On Fri, Jun 30, 2017 at 3:34 PM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi
>>
>> On Fri, Jun 30, 2017 at 2:29 AM, George Gelashvili <
>> ggelashv...@pivotal.io> wrote:
>>
>>>  Hey Surinder,
>>>
>>>
>>> ​1. ​
 Tools
 ​(tools.js)​ - It will contain all JS modules under tools directory


 ​2. ​
 Browser
 ​(browser.js)​ - It will contain all JS modules under browser directory

>>>
>>> By under, do you mean every javascript file recursively under the
>>> browser directory?
>>>
>> ​No, I mean a single bundled javascript ​file of various nodes static JS
>> files.
>>
>>>
>>>
 ​4. Common.js and/or vendor.js - Common.js will contains all common js
 files/libraries shared by modules like backform, backbone, underscore etc.

>>>
>>> We're okay with bundling vendorized code for a first pass to webpack
>>> everything. The goal should be to pull code out of the shared javascript
>>> bundle as we unvendor dependencies.
>>>
>> Yes, bundled vendor.js will have all unvendor dependencies and files will
>> refer from node_modules.
>>
>>> For naming: Common.js is an overloaded term (
>>> http://requirejs.org/docs/commonjs.html). We prefer vendor.js. Will
>>> there be non-vendor code put into this bundle?
>>> ​I think vendor.js should only contain vendor files. We can create
>>> another file(xyz) that will contains JS files shared across the modules.
>>>
>>> ​What do you think ?​
>>>
>>>
>> ​
>>>
 Entry points JS: main.js - It will load above modules ascynchronusly or
 it will be minified version of all.
 define('', ['common', 'tools', 'browser', 'grid'], function(...) {});

>>> or these modules can also be loaded using dependency; For example, load
 grid.js on database node expand.

>>>
>>> Could you go into some more detail on this decision? How would main.js
>>> be used in the app? Is the idea to still use define from require.js in
>>> javascript being webpacked?
>>>
>> ​In pgAdmin4 there is no main.js, but entry point is browser/index.html.​
>> This is how I think bundled JS will load:
>>
>> Load vendor.js using  in base.html
>>
>> *​browser/​index.html*
>>
>> require(
>> ​  ​
>> ['pgadmin',
>> ​'sources/generated/common', ​
>> 'sources/generated/bundled'],
>> ​  ​
>> function() {
>> }
>> ​)​
>>
>> *sources/generated/bundled​.js​*
>>
>> ​define
>> (
>> ​  ​
>> ['
>> ​sources/generated/browser
>> ', 'sources/generated/
>> ​tools
>> '
>> ​, 'sources/generated/grid'​
>> ],
>> ​  ​
>> function
>> ​(​
>> pgBrowser) {
>> ​  pgBrowser.init();​
>> }
>> )​
>> ​
>>
>>
>> ​or we can use 'import'​ instead of 'require' for files that will be
>> bundled like 'bundled.js'
>> I keep require/define call untouched in module JS, and webpack converts
>> 'define' calls to __webpack_require__(module_id) to load dependent modules.
>>
>>>
>>> ​Currently I am working on converting 'requirejs config shim dependency
 into webpack' using imports-loader and exports-loader which is taking
 time and will look for alternative if it doesn't works. Once dependency is
 properly defined, the task to generate other module JS will become quite
 easy.

>>>
>>> We used imports-loader and exports-loader for tests (see
>>> web/webpack.test.config.js)
>>>
>> ​Hmm. it is useful.​ but I am using shim-loader
>>  which is compatible with
>> requirejs shim format.
>>
>>>
>>>
 I also found, the CSS imported using import 'slickgrid/slick.grid.css';
  statement is put into  tags
 ​ in html​
 , instead must be loaded separately as a file so overrides.css can
 work.

>>>
>>> Those three css files f

Re: UI issue

[Dave Page]

On Fri, Jun 30, 2017 at 12:29 PM, Dave Page  wrote:

>
>
> On Fri, Jun 30, 2017 at 12:25 PM, Dave Page  wrote:
>
>>
>>
>> On Fri, Jun 30, 2017 at 12:23 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Fri, Jun 30, 2017 at 4:39 PM, Dave Page  wrote:
>>>


 On Fri, Jun 30, 2017 at 12:07 PM, Harshal Dhumal <
 harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> I'm getting this wired ui issue when I launched query tool using
> view/edit menu. Is anybody getting such issue?
>

 Looks like you dragged the query tool off the main tab bar and docked
 it to the left of the browser. Did you try resetting your view?

>>> I just tried again by resetting layout and opened query tool using
>>> view/edit data -> view all rows and still getting same issue
>>>
>>
>> Weird. I don't see it. Anyone else?
>>
>
> I take that back; I just saw it happen in the feature tests.
>

So, further info: I see this effect with View Data, but *not* with the
Query Tool. Could it be related to the renaming of the menu options/tabs?


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: UI issue

[Khushboo Vashi]

Hi,

Please find the attached patch for the same.
This is due to my precious patch.

Thanks,
Khushboo

On Fri, Jun 30, 2017 at 5:06 PM, Dave Page  wrote:

>
>
> On Fri, Jun 30, 2017 at 12:29 PM, Dave Page  wrote:
>
>>
>>
>> On Fri, Jun 30, 2017 at 12:25 PM, Dave Page  wrote:
>>
>>>
>>>
>>> On Fri, Jun 30, 2017 at 12:23 PM, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>


 On Fri, Jun 30, 2017 at 4:39 PM, Dave Page  wrote:

>
>
> On Fri, Jun 30, 2017 at 12:07 PM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> I'm getting this wired ui issue when I launched query tool using
>> view/edit menu. Is anybody getting such issue?
>>
>
> Looks like you dragged the query tool off the main tab bar and docked
> it to the left of the browser. Did you try resetting your view?
>
 I just tried again by resetting layout and opened query tool using
 view/edit data -> view all rows and still getting same issue

>>>
>>> Weird. I don't see it. Anyone else?
>>>
>>
>> I take that back; I just saw it happen in the feature tests.
>>
>
> So, further info: I see this effect with View Data, but *not* with the
> Query Tool. Could it be related to the renaming of the menu options/tabs?
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/js/datagrid.js b/web/pgadmin/tools/datagrid/templates/datagrid/js/datagrid.js
index 9b72a11..2096e16 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/js/datagrid.js
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/js/datagrid.js
@@ -379,7 +379,7 @@ define([
 newWin.document.title = grid_title;
   });
 } else {
-  var dashboardPanel = pgBrowser.docker.findPanels('properites');
+  var dashboardPanel = pgBrowser.docker.findPanels('properties');
   var dataGridPanel = pgBrowser.docker.addPanel('frm_datagrid', wcDocker.DOCK.STACKED, dashboardPanel[0]);
 
   // Set panel title and icon

Re: Tree view icon samples

[Chethana Kumar]

Hello Team,

Below are the updates for TreeView Icons -

1. Fine tuned the icons - Database, Language, Primary and Foreign key,
Unique Constraint
2. Designed new concept for - Casts and Converts, Trigger
3. Attached the color blind test result for "Primary and Foreign key", now
I feel it is pretty evident to differentiate for blind user
4. Attached "thought_process.png" where I have explained the thought
process for newly proposed icons (referred:www.postgresql.org)
5. Added little depth to all icons to provide the Afforance of Click






On Thu, Jun 29, 2017 at 10:21 AM, Ashesh Vashi <
ashesh.va...@enterprisedb.com> wrote:

>
> On Thu, Jun 29, 2017 at 9:09 AM, Chethana Kumar <
> chethana.ku...@enterprisedb.com> wrote:
>
>> Hi Ashesh,
>>
>> I tried to eliminate the colour blind issue which might occur with the
>> primary & foregin key icons.
>> But still I will use the app to test the colors, just let me know your
>> opinion now.
>>
> Please do.
>
> One more point: :-)
> Icons for some nodes are missing here.
> i.e. Partition table (introduced in 'Declarative Partition' patch),
> Materialized View, etc.
>
> -- Thanks, Ashesh
>
>>
>> Regards,
>> Chethana kumar
>>
>> On Wed, Jun 28, 2017 at 11:13 PM, Ashesh Vashi <
>> ashesh.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Jun 28, 2017 22:32, "Robert Eckhardt"  wrote:
>>>
>>> I like them as well. The only comment I have is that the red icon on
>>> disconnected DB and corrupted DB are difficult to make out. They are
>>> clearly differentiable but the icon itself is unclear.
>>>
>>> A colour blind person may not be able to differentiate the primary &
>>> foreign key icons.
>>>
>>> -- Ashesh
>>>
>>>
>>> -- Rob
>>>
>>> On Wed, Jun 28, 2017 at 12:49 PM, Dave Page  wrote:
>>>
 Hi

 > On 28 Jun 2017, at 12:41, Chethana Kumar <
 chethana.ku...@enterprisedb.com> wrote:
 >
 > Hi Team,
 >
 > I am coming up with some sample icons for Tree View Control.
 >
 > Thought process behind the concept -
 >
 > 1. Included EDB color brand feel (gray and blue colors)

 You mean pgAdmin colour brand :-). EDB is orange & black.

 > 2. Eliminated the distraction of too many colors and provided
 consistency through the icons
 > 3. User will recognize the icon shapes rather than recalling it by
 their colors (like in existing icons) so focused more on shapes
 > 4. It also adds value to the brand since it has EDB color tones in it
 >
 >
 > So please feel free to share your views on the same

 I like them - they look much more modern and stylish, but still fit
 with what they represent.

 Other opinions please?

 Good work!

>>>
>>>
>>>
>>
>>
>> --
>> Chethana Kumar
>> Principal UI/UX Designer
>> EnterpriseDB Corporation
>>
>>
>> The Postgres Database Company
>>
>> P: +91 86981 57146
>> www.enterprisedb.com
>>
>
>


-- 
Chethana Kumar
Principal UI/UX Designer
EnterpriseDB Corporation


The Postgres Database Company

P: +91 86981 57146
www.enterprisedb.com

pgAdmin 4 commit: Fix typo that caused a layout issue with the Edit Gri

[Dave Page]

Fix typo that caused a layout issue with the Edit Grid.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=7c09a8f8272e9d697ea9f8cfddf6f3a34a4d9fe9
Author: Khushboo Vashi 

Modified Files
--
web/pgadmin/tools/datagrid/templates/datagrid/js/datagrid.js | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

Re: UI issue

[Dave Page]

Thanks, applied. I also renamed the variable to something more suitable.

On Fri, Jun 30, 2017 at 12:49 PM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch for the same.
> This is due to my precious patch.
>
> Thanks,
> Khushboo
>
> On Fri, Jun 30, 2017 at 5:06 PM, Dave Page  wrote:
>
>>
>>
>> On Fri, Jun 30, 2017 at 12:29 PM, Dave Page  wrote:
>>
>>>
>>>
>>> On Fri, Jun 30, 2017 at 12:25 PM, Dave Page  wrote:
>>>


 On Fri, Jun 30, 2017 at 12:23 PM, Harshal Dhumal <
 harshal.dhu...@enterprisedb.com> wrote:

>
>
> On Fri, Jun 30, 2017 at 4:39 PM, Dave Page  wrote:
>
>>
>>
>> On Fri, Jun 30, 2017 at 12:07 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> I'm getting this wired ui issue when I launched query tool using
>>> view/edit menu. Is anybody getting such issue?
>>>
>>
>> Looks like you dragged the query tool off the main tab bar and docked
>> it to the left of the browser. Did you try resetting your view?
>>
> I just tried again by resetting layout and opened query tool using
> view/edit data -> view all rows and still getting same issue
>

 Weird. I don't see it. Anyone else?

>>>
>>> I take that back; I just saw it happen in the feature tests.
>>>
>>
>> So, further info: I see this effect with View Data, but *not* with the
>> Query Tool. Could it be related to the renaming of the menu options/tabs?
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Jenkins build is back to normal : pgadmin4-master-python33 #208

[pgAdmin 4 Jenkins]

See 

[pgAdmin4][Patch] To fix the issues in Debugger module

[Murtuza Zabuawala]

Hi,

Please find patch to fix the below issues,

1) Debugger page layout
RM#2512

2) To fix the issue when you user clicks on Stop button and gets connection
lost error.
RM#2511

@Dave,
When we stop the debugger using pldbg_abort_target() function and try to
start it again in the same connection, we are not able to do so because we
do not get response from plugin(async query results shows busy status
only), I think that is the reason why in pgAdmin3 we have disable all the
buttons and release the connection if user clicks on stop button, For time
being I have done the same in pgAdmin4 until we find the solution for this
issue.

Misc changes are for PEP-8.

Please review.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/tools/debugger/__init__.py 
b/web/pgadmin/tools/debugger/__init__.py
index 8cebffc..35a0378 100644
--- a/web/pgadmin/tools/debugger/__init__.py
+++ b/web/pgadmin/tools/debugger/__init__.py
@@ -524,7 +524,7 @@ def initialize_target(debug_type, sid, did, scid, func_id, 
tri_id=None):
 'newBrowserTab': new_browser_tab})
 
 
-@blueprint.route('/close/', methods=["GET"])
+@blueprint.route('/close/', methods=["DELETE"])
 def close(trans_id):
 """
 close(trans_id)
@@ -849,10 +849,16 @@ def execute_debugger_query(trans_id, query_type):
 template_path = 'debugger/sql/v2'
 
 if conn.connected():
-sql = render_template("/".join([template_path, query_type + ".sql"]), 
session_id=obj['session_id'])
-# As the query type is continue or step_into or step_over then we may 
get result after some time so poll the
-# result. We need to update the frame id variable when user move the 
next step for debugging.
-if query_type == 'continue' or query_type == 'step_into' or query_type 
== 'step_over':
+sql = render_template(
+"/".join([template_path, query_type + ".sql"]),
+session_id=obj['session_id']
+)
+# As the query type is continue or step_into or step_over then we
+# may get result after some time so poll the result.
+# We need to update the frame id variable when user move the next
+# step for debugging.
+if query_type == 'continue' or query_type == 'step_into' or \
+query_type == 'step_over':
 # We should set the frame_id to 0 when execution starts.
 if obj['frame_id'] != 0:
 session_obj = debugger_data[str(trans_id)]
@@ -860,29 +866,30 @@ def execute_debugger_query(trans_id, query_type):
 update_session_debugger_transaction(trans_id, session_obj)
 
 status, result = conn.execute_async(sql)
-return make_json_response(data={'status': status, 'result': 
result})
+return make_json_response(
+data={'status': status, 'result': result}
+)
 elif query_type == 'abort_target':
 status, result = conn.execute_dict(sql)
-
-manager.release(did=obj['database_id'], conn_id=obj['conn_id'])
-
-# Delete the existing debugger data in session variable
-del session['debuggerData'][str(trans_id)]
-del session['functionData'][str(trans_id)]
-
 if not status:
 return internal_server_error(errormsg=result)
 else:
-return make_json_response(info=gettext('Target Aborted.'), 
data={'status': status, 'result': result})
+return make_json_response(
+info=gettext('Debugging aborted successfully.'),
+data={'status': 'Success', 'result': result}
+)
 else:
 status, result = conn.execute_dict(sql)
 if not status:
 return internal_server_error(errormsg=result)
 else:
-status = False
-result = gettext('Not connected to server or connection with the 
server has been closed.')
+result = gettext('Not connected to server or connection '
+ 'with the server has been closed.')
+return internal_server_error(errormsg=result)
 
-return make_json_response(data={'status': 'Success', 'result': 
result['rows']})
+return make_json_response(
+data={'status': 'Success', 'result': result['rows']}
+)
 
 
 @blueprint.route('/messages//', methods=["GET"])
@@ -915,9 +922,12 @@ def messages(trans_id):
 status, result = conn.poll()
 notify = conn.messages()
 if notify:
-# In notice message we need to find "PLDBGBREAK" string to find 
the port number to attach.
-# Notice message returned by the server is   "NOTICE:  
PLDBGBREAK:7".
-# From the above message we need to find out port number as "7" so 
below logic will find 7 as port number
+# In notice message we need to find "PLDBGBREAK" string to find 

Re: Tree view icon samples

[Dave Page]

Hi

- The Primary Key and Foreign Key icons should have more differentiation
than just colour. Maybe add a 1 to Primary Key?

- Did you get a list of the other icons that aren't on that sheet yet?

- I'm not sure I like the Casts and Conversions icons (mis-spelt as
Converts by the previous owner). The text on the makes them seem textual
than graphical.

Thanks!

On Fri, Jun 30, 2017 at 12:56 PM, Chethana Kumar <
chethana.ku...@enterprisedb.com> wrote:

> Hi Team,  Attachment was missing.
>
>
>
> On Fri, Jun 30, 2017 at 5:24 PM, Chethana Kumar <
> chethana.ku...@enterprisedb.com> wrote:
>
>> Hello Team,
>>
>> Below are the updates for TreeView Icons -
>>
>> 1. Fine tuned the icons - Database, Language, Primary and Foreign key,
>> Unique Constraint
>> 2. Designed new concept for - Casts and Converts, Trigger
>> 3. Attached the color blind test result for "Primary and Foreign key",
>> now I feel it is pretty evident to differentiate for blind user
>> 4. Attached "thought_process.png" where I have explained the thought
>> process for newly proposed icons (referred:www.postgresql.org)
>> 5. Added little depth to all icons to provide the Afforance of Click
>>
>>
>>
>>
>>
>>
>> On Thu, Jun 29, 2017 at 10:21 AM, Ashesh Vashi <
>> ashesh.va...@enterprisedb.com> wrote:
>>
>>>
>>> On Thu, Jun 29, 2017 at 9:09 AM, Chethana Kumar <
>>> chethana.ku...@enterprisedb.com> wrote:
>>>
 Hi Ashesh,

 I tried to eliminate the colour blind issue which might occur with the
 primary & foregin key icons.
 But still I will use the app to test the colors, just let me know your
 opinion now.

>>> Please do.
>>>
>>> One more point: :-)
>>> Icons for some nodes are missing here.
>>> i.e. Partition table (introduced in 'Declarative Partition' patch),
>>> Materialized View, etc.
>>>
>>> -- Thanks, Ashesh
>>>

 Regards,
 Chethana kumar

 On Wed, Jun 28, 2017 at 11:13 PM, Ashesh Vashi <
 ashesh.va...@enterprisedb.com> wrote:

>
>
> On Jun 28, 2017 22:32, "Robert Eckhardt"  wrote:
>
> I like them as well. The only comment I have is that the red icon on
> disconnected DB and corrupted DB are difficult to make out. They are
> clearly differentiable but the icon itself is unclear.
>
> A colour blind person may not be able to differentiate the primary &
> foreign key icons.
>
> -- Ashesh
>
>
> -- Rob
>
> On Wed, Jun 28, 2017 at 12:49 PM, Dave Page  wrote:
>
>> Hi
>>
>> > On 28 Jun 2017, at 12:41, Chethana Kumar <
>> chethana.ku...@enterprisedb.com> wrote:
>> >
>> > Hi Team,
>> >
>> > I am coming up with some sample icons for Tree View Control.
>> >
>> > Thought process behind the concept -
>> >
>> > 1. Included EDB color brand feel (gray and blue colors)
>>
>> You mean pgAdmin colour brand :-). EDB is orange & black.
>>
>> > 2. Eliminated the distraction of too many colors and provided
>> consistency through the icons
>> > 3. User will recognize the icon shapes rather than recalling it by
>> their colors (like in existing icons) so focused more on shapes
>> > 4. It also adds value to the brand since it has EDB color tones in
>> it
>> >
>> >
>> > So please feel free to share your views on the same
>>
>> I like them - they look much more modern and stylish, but still fit
>> with what they represent.
>>
>> Other opinions please?
>>
>> Good work!
>>
>
>
>


 --
 Chethana Kumar
 Principal UI/UX Designer
 EnterpriseDB Corporation


 The Postgres Database Company

 P: +91 86981 57146 <+91%2086981%2057146>
 www.enterprisedb.com

>>>
>>>
>>
>>
>> --
>> Chethana Kumar
>> Principal UI/UX Designer
>> EnterpriseDB Corporation
>>
>>
>> The Postgres Database Company
>>
>> P: +91 86981 57146 <+91%2086981%2057146>
>> www.enterprisedb.com
>>
>
>
>
> --
> Chethana Kumar
> Principal UI/UX Designer
> EnterpriseDB Corporation
>
>
> The Postgres Database Company
>
> P: +91 86981 57146 <+91%2086981%2057146>
> www.enterprisedb.com
>



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[pgAdmin4][Patch]: Refactor of the History Tab

[Shruti Iyer]

Hi Hackers,

Attached is the patch that refactors the react components that supports the
history detail pane. We moved inline styling. whenever possible, to scss.

Thanks,
Joao and Shruti


refactor-react-history.diff
Description: Binary data

Re: [pgAdmin4][Patch]: Refactor of the History Tab

[Robert Eckhardt]

Surinder,

It would be great if you could take a look at this an see if the style
changes are more inline with what you were thinking when you commented on
the first patch.

-- Rob

On Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer  wrote:

> Hi Hackers,
>
> Attached is the patch that refactors the react components that supports
> the history detail pane. We moved inline styling. whenever possible, to
> scss.
>
> Thanks,
> Joao and Shruti
>

Re: Tree view icon samples

[Strauch, Sheldon]

Dave et al,

Might I suggest flipping the foreign key icon left for right? That way, the
key "points" to the left, back toward something pre-existing whilst the
primary key "points" to the right, forward towards something new?

Also, for me, the updated index icon loses its intuitiveness as it suggests
an ordinal list or an enumeration. I think the clue in the old icon was the
index tab at the top of the illustration.

Finally, the new trigger icon suggests, to me, that one is trying to do
something with a document, maybe copying it. Whereas the old icon suggests
an action to be taken.

Just my two cents...

On Fri, Jun 30, 2017 at 9:57 AM, Dave Page  wrote:

> Hi
>
> - The Primary Key and Foreign Key icons should have more differentiation
> than just colour. Maybe add a 1 to Primary Key?
>
> - Did you get a list of the other icons that aren't on that sheet yet?
>
> - I'm not sure I like the Casts and Conversions icons (mis-spelt as
> Converts by the previous owner). The text on the makes them seem textual
> than graphical.
>
> Thanks!
>
> On Fri, Jun 30, 2017 at 12:56 PM, Chethana Kumar <
> chethana.ku...@enterprisedb.com> wrote:
>
>> Hi Team,  Attachment was missing.
>>
>>
>>
>> On Fri, Jun 30, 2017 at 5:24 PM, Chethana Kumar <
>> chethana.ku...@enterprisedb.com> wrote:
>>
>>> Hello Team,
>>>
>>> Below are the updates for TreeView Icons -
>>>
>>> 1. Fine tuned the icons - Database, Language, Primary and Foreign key,
>>> Unique Constraint
>>> 2. Designed new concept for - Casts and Converts, Trigger
>>> 3. Attached the color blind test result for "Primary and Foreign key",
>>> now I feel it is pretty evident to differentiate for blind user
>>> 4. Attached "thought_process.png" where I have explained the thought
>>> process for newly proposed icons (referred:www.postgresql.org)
>>> 5. Added little depth to all icons to provide the Afforance of Click
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Thu, Jun 29, 2017 at 10:21 AM, Ashesh Vashi <
>>> ashesh.va...@enterprisedb.com> wrote:
>>>

 On Thu, Jun 29, 2017 at 9:09 AM, Chethana Kumar <
 chethana.ku...@enterprisedb.com> wrote:

> Hi Ashesh,
>
> I tried to eliminate the colour blind issue which might occur with the
> primary & foregin key icons.
> But still I will use the app to test the colors, just let me know your
> opinion now.
>
 Please do.

 One more point: :-)
 Icons for some nodes are missing here.
 i.e. Partition table (introduced in 'Declarative Partition' patch),
 Materialized View, etc.

 -- Thanks, Ashesh

>
> Regards,
> Chethana kumar
>
> On Wed, Jun 28, 2017 at 11:13 PM, Ashesh Vashi <
> ashesh.va...@enterprisedb.com> wrote:
>
>>
>>
>> On Jun 28, 2017 22:32, "Robert Eckhardt" 
>> wrote:
>>
>> I like them as well. The only comment I have is that the red icon on
>> disconnected DB and corrupted DB are difficult to make out. They are
>> clearly differentiable but the icon itself is unclear.
>>
>> A colour blind person may not be able to differentiate the primary &
>> foreign key icons.
>>
>> -- Ashesh
>>
>>
>> -- Rob
>>
>> On Wed, Jun 28, 2017 at 12:49 PM, Dave Page 
>> wrote:
>>
>>> Hi
>>>
>>> > On 28 Jun 2017, at 12:41, Chethana Kumar <
>>> chethana.ku...@enterprisedb.com> wrote:
>>> >
>>> > Hi Team,
>>> >
>>> > I am coming up with some sample icons for Tree View Control.
>>> >
>>> > Thought process behind the concept -
>>> >
>>> > 1. Included EDB color brand feel (gray and blue colors)
>>>
>>> You mean pgAdmin colour brand :-). EDB is orange & black.
>>>
>>> > 2. Eliminated the distraction of too many colors and provided
>>> consistency through the icons
>>> > 3. User will recognize the icon shapes rather than recalling it by
>>> their colors (like in existing icons) so focused more on shapes
>>> > 4. It also adds value to the brand since it has EDB color tones in
>>> it
>>> >
>>> >
>>> > So please feel free to share your views on the same
>>>
>>> I like them - they look much more modern and stylish, but still fit
>>> with what they represent.
>>>
>>> Other opinions please?
>>>
>>> Good work!
>>>
>>
>>
>>
>
>
> --
> Chethana Kumar
> Principal UI/UX Designer
> EnterpriseDB Corporation
>
>
> The Postgres Database Company
>
> P: +91 86981 57146 <+91%2086981%2057146>
> www.enterprisedb.com
>


>>>
>>>
>>> --
>>> Chethana Kumar
>>> Principal UI/UX Designer
>>> EnterpriseDB Corporation
>>>
>>>
>>> The Postgres Database Company
>>>
>>> P: +91 86981 57146 <+91%2086981%2057146>
>>> www.enterprisedb.com
>>>
>>
>>
>>
>> --
>> Chethana Kumar
>> Principal UI/UX Designer
>> EnterpriseDB Corporation
>>
>>
>> The Postgres Database Company
>>
>> P: +91 86981 57146 <+91%2086981%2057146>
>> www.e

[pgAdmin4][Patch] To fix the duplicate entry in History panel

[Murtuza Zabuawala]

Hi,

PFA minor patch to fix the issue where duplicate entry is made of
successful executed query one with status message and one without status
message.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js 
b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 173301b..ad43c70 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -1854,7 +1854,6 @@ define([
 
   // Show message in message and history tab in case of query tool
   self.total_time = self.get_query_run_time(self.query_start_time, 
self.query_end_time);
-  self.update_msg_history(true, "", false);
   var msg1 = S(gettext("Successfully run. Total query runtime: 
%s.")).sprintf(self.total_time).value();
   var msg2 = S(gettext("%s rows 
affected.")).sprintf(self.rows_affected).value();
 

Re: [pgAdmin4][Patch]: Refactor of the History Tab

[Murtuza Zabuawala]

Hi,

On Fri, Jun 30, 2017 at 10:30 PM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> ​Hi,
>
> Review comments:
>
> 1) I see the font used for class .query-history .entry​ is monospace,
> shouldn't it be Helvetica as per style guide ? the font for Messages > text
> is also monospace.
>
> 2) Can 1px top border be added above first entry in left panel to
> differentiate from above panels ?
>
> 3) No query execution message appears if i run same query second time,
> please refer screenshot.
>
I think this isn't related to this patch, just now I have sent patch for
similar issue.

>
> Apart from this patch looks good.
>
> Thanks,
> Surinder
>
>
>
> On Fri, Jun 30, 2017 at 9:58 PM, Robert Eckhardt 
> wrote:
>
>> Surinder,
>>
>> It would be great if you could take a look at this an see if the style
>> changes are more inline with what you were thinking when you commented on
>> the first patch.
>>
>> -- Rob
>>
>> On Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer  wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached is the patch that refactors the react components that supports
>>> the history detail pane. We moved inline styling. whenever possible, to
>>> scss.
>>>
>>> Thanks,
>>> Joao and Shruti
>>>
>>
>>
>

Re: [pgAdmin4][Patch]: Refactor of the History Tab

[Dave Page]

> On 30 Jun 2017, at 18:00, Surinder Kumar  
> wrote:
> 
> ​Hi,
> 
> Review comments:
> 
> 1) I see the font used for class .query-history .entry​ is monospace, 
> shouldn't it be Helvetica as per style guide ? the font for Messages > text 
> is also monospace.

No, the style guide needs an entry for monospaced fonts which are very much 
needed in a few places like this.

> 
> 2) Can 1px top border be added above first entry in left panel to 
> differentiate from above panels ?
> 
> 3) No query execution message appears if i run same query second time, please 
> refer screenshot.
> 
> Apart from this patch looks good.
> 
> Thanks,
> Surinder
> 
> 
> 
>> On Fri, Jun 30, 2017 at 9:58 PM, Robert Eckhardt  
>> wrote:
>> Surinder,
>> 
>> It would be great if you could take a look at this an see if the style 
>> changes are more inline with what you were thinking when you commented on 
>> the first patch. 
>> 
>> -- Rob
>> 
>>> On Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer  wrote:
>>> Hi Hackers,
>>> 
>>> Attached is the patch that refactors the react components that supports the 
>>> history detail pane. We moved inline styling. whenever possible, to scss.
>>> 
>>> Thanks,
>>> Joao and Shruti
>> 
> 
> 

Re: [pgAdmin4][Patch] To fix the duplicate entry in History panel

[George Gelashvili]

Nice! Looks good. We were investigating this bug before you sent this and
it looks like your patch resolves the duplicate calls to update_msg_history

Cheers,
Sarah and George

On Fri, Jun 30, 2017 at 1:14 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA minor patch to fix the issue where duplicate entry is made of
> successful executed query one with status message and one without status
> message.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Patch for sqleditor js syntax issue

[Harshal Dhumal]

Hi,

Please find attached minor patch which fixes js syntax issue in sqleditor.js


-- 
*Harshal Dhumal*
*Sr. Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 173301b..124c894 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -891,7 +891,7 @@ define([
 }
 dataView.setItems(collection, self.client_primary_key);
   },
-  fetch_next_all(cb) {
+  fetch_next_all: function(cb) {
 this.fetch_next(true, cb);
   },
   fetch_next: function(fetch_all, cb) {