Re: pgAdmin 4 commit: Fix a number of broken connection detection scenarios

[Neel Patel]

Thanks,
RM#3249 created for the same.

Regards,
Neel Patel

On Thu, Apr 5, 2018 at 11:27 AM, Akshay Joshi  wrote:

> Hi Neel
>
> This issue is not due to the above fix, if you can see the error it is
> showing in dashboard.js line no 454 "err = JSON.parse(xhr.responseText);"
> that code is from long time.
> I have just added an extra check of the readyState in case of server is
> down in the above commit.
> Though it is a bug can you please create a new RM for this.
>
> On Thu, Apr 5, 2018 at 10:52 AM, Neel Patel 
> wrote:
>
>> Hi Akshay,
>>
>> Due to above commit, when user "Drop" any database it gives exception in
>> browser console.
>> Check attached screenshot.
>>
>> Thanks,
>> Neel Patel
>>
>>
>>
>> On Wed, Mar 21, 2018 at 2:08 PM, Dave Page  wrote:
>>
>>> Fix a number of broken connection detection scenarios.
>>>
>>> Branch
>>> --
>>> master
>>>
>>> Details
>>> ---
>>> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdif
>>> f;h=637f3b9d1a54df8b9a1bfad9e26f27fdea407683
>>> Author: Akshay Joshi 
>>>
>>> Modified Files
>>> --
>>> web/pgadmin/dashboard/static/js/dashboard.js   | 58
>>> 
>>> web/pgadmin/static/js/sqleditor/execute_query.js   | 62
>>> +++--
>>> .../js/sqleditor/is_new_transaction_required.js|  9 
>>> web/pgadmin/tools/sqleditor/__init__.py|  4 +-
>>> web/pgadmin/tools/sqleditor/static/js/sqleditor.js | 20 ---
>>> .../tools/sqleditor/tests/test_start_query_tool.py |  2 +-
>>> .../tools/sqleditor/utils/start_running_query.py   | 63
>>> ++
>>> .../utils/tests/test_start_running_query.py| 15 +-
>>> .../user_management/static/js/user_management.js   | 13 -
>>> .../utils/driver/psycopg2/server_manager.py|  1 +
>>> .../javascript/sqleditor/execute_query_spec.js | 48
>>> -
>>> .../sqleditor/is_new_transaction_required_spec.js  | 14 ++---
>>> 12 files changed, 195 insertions(+), 114 deletions(-)
>>>
>>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

Re: [pgAdmin4]: Fix for RM #3248

[Dave Page]

Thanks, applied.

On Thu, Apr 5, 2018 at 7:49 AM, Neel Patel 
wrote:

> Hi,
>
> Please find updated patch after fixing the comments.
> Review it and let me know for comments.
>
> Thanks,
> Neel Patel
>
> On Thu, Apr 5, 2018 at 11:32 AM, Neel Patel 
> wrote:
>
>> Thanks Murtuza & Khushboo.
>> I will send updated patch.
>>
>> Thanks,
>> Neel Patel
>>
>> On Thu, Apr 5, 2018 at 11:29 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Neel,
>>>
>>> Add below code to the top (around line no 8) in
>>> alertify.pgadmin.defaults.js file will solve the problem.
>>>
>>> alertify.defaults.closable = false;
>>>
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Thu, Apr 5, 2018 at 11:13 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Neel,

 Can we make the change in wrapper instead of changing every where?
 File: ../web/pgadmin/static/js/alertify.pgadmin.defaults.js


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


 On Thu, Apr 5, 2018 at 11:04 AM, Neel Patel <
 neel.pa...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch for the fix of RM #3248 - we have modified
> alertify confirm dialog to modal dialog.
>
> Do review it and let me know for comments.
>
> Thanks,
> Neel Patel
>


>>>
>>
>


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

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

pgAdmin 4 commit: Ensure Alertify dialogues are modal to prevent them b

[Dave Page]

Ensure Alertify dialogues are modal to prevent them being closed by mis-click. 
Fixes #3248

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=659390493d08fe62d97f1fb48ff68d5e15a534fb
Author: Neel Patel 

Modified Files
--
docs/en_US/release_notes_3_0.rst   | 3 ++-
web/pgadmin/static/js/alertify.pgadmin.defaults.js | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Khushboo Vashi]

Hi Joao,

Can you please rebase the second patch?

Thanks,
Khushboo

On Tue, Apr 3, 2018 at 12:15 AM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Hackers,
>
> Attached you can find the patch that will start to decouple pgAdmin from
> ACITree library.
> This patch is intended to be merged after 3.0, because we do not want to
> cause any entropy or delay the release, but we want to start the discussion
> and show some code.
>
> This job that we started is a massive tech debt chore that will take some
> time to finalize and we would love the help of the community to do it.
>
> *Summary of the patch:*
> 0001 patch:
>  - Creates a new tree that will allow us to create a separation between
> the application and ACI Tree
>  - Creates a Fake Tree (Test double, for reference on the available test
> doubles: https://martinfowler.com/bliki/TestDouble.html) that can be used
> to inplace to replace the ACITree and also encapsulate the new tree
> behavior on our tests
>  - Adds tests for all the tree functionalities
>
> 0002 patch:
>  - Extracts, refactors, adds tests and remove dependency from ACI Tree on:
> - getTreeNodeHierarchy
> - on backup.js: menu_enabled, menu_enabled_server,
> start_backup_global_server, backup_objects
> - on datagrid.js: show_data_grid, get_panel_title, show_query_tool
> - Start using sprintf-js as Underscore.String is deprecating sprintf
> function
>
> This patch represents only 10 calls to ACITree.itemData out of 176 that
> are spread around our code
>
>
> *In Depth look on the process behind the patch:*
>
> We started writing this patch with the idea that we need to decouple
> pgAdmin4 from ACITree, because ACITree is no longer supported, the
> documentation is non existent and ACITree is no longer being actively
> developed.
>
> Our process:
> 1. We "randomly" selected a function that is part of the ACITree. From
> this point we decided to replace that function with our own version. The
> function that we choose was "itemData".
> The function gives us all the "data" that a specific node of the tree find.
> Given in order to replace the tree we would need to have a function that
> would give us the same information. We had 2 options:
>   a) Create a tree with a function called itemData
> Pros:
>  - At first view this was the simpler solution
>  - Would keep the status quo
> Cons:
>  - Not a OOP approach
>  - Not very flexible
>   b) Create a tree that would return a node given an ID and then the node
> would be responsible for giving it's data.
> Pros:
>  - OOP Approach
>  - More flexible and we do not need to bring the tree around, just a node
> Cons:
>  - Break the current status quo
>
> Given these 2 options we decided to go for a more OOP approach creating a
> Tree and a TreeNode classes, that in the future will be renamed to
> ACITreeWrapper and TreeNode.
>
> 2. After we decided on the starting point we searched for occurrences of
> the function "itemData" and we found out that there were 303 occurrences of
> "itemData" in the code and roughly 176 calls to the function itself (some
> of the hits were variable names).
>
> 3. We selected the first file on the search and found the function that
> was responsible for calling the itemData function.
>
> 4. Extracted the function to a separate file
>
> 5. Wrap this function with tests
>
> 6. Refactor the function to ES6, give more declarative names to variables
> and break the functions into smaller chunks
>
> 7. When all the tests were passing we replaced ACITree with our Tree
>
> 8. We ensured that all tests were passing
>
> 9. Remove function from the original file and use the new function
>
> 10. Ensure everything still works
>
> 11. Find the next function and execute from step 4 until all the functions
> are replaced, refactored and tested.
>
> As you can see by the process this is a pretty huge undertake, because of
> the number of calls to the function. This is just the first step on the
> direction of completely isolating the ACITree so that we can solve the
> problem with a large number of elements on the tree.
>
> *What is on our radar that we need to address:*
>  - Finish the complete decoupling of the ACITree
>  - Performance of the current tree implementation
>  - Tweak the naming of the Tree class to explicitly tell us this is to use
> only with ACITree.
>
>
> Thanks
> Joao
>
>

Re: [pgAdmin4][RM#3055] Allow user to sort the data in View data mode

[Dave Page]

Can you rebase this please?

On Wed, Mar 28, 2018 at 8:19 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find updated patch with following changes,
> - Combined Filter and Data sorting together same as pgAdmin3.
> - Extracted model into separate file
> - Change the colour of filter button from orange to blue.
> - Updated docs and screenshot.
>
> @Joao,
> Could you please provide any reference for learning more about jasmine
> test framework?
> ​
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Wed, Mar 28, 2018 at 6:07 AM, Robert Eckhardt 
> wrote:
>
>>
>>
>> On Tue, Mar 27, 2018 at 9:54 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Tue, Mar 27, 2018 at 7:06 PM, Robert Eckhardt 
>>> wrote:
>>>


 On Tue, Mar 27, 2018 at 6:25 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> On Tue, Mar 27, 2018 at 3:13 PM, Dave Page  wrote:
>
>>
>>
>> On Mon, Mar 26, 2018 at 9:26 PM, Robert Eckhardt <
>> reckha...@pivotal.io> wrote:
>>
>>>
>>>
>>> On Mon, Mar 26, 2018 at 2:07 PM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
 Hi Hackers,

 @Murtuza: The patch codewise looks good. Nice to see that we are
 using axios instead of jquery ajax calls and that there is some 
 coverage
 for the change.
 Nevertheless the Javascript testing looks a bit slim and could be
 improved. Also the DataSorting class could have some other member 
 functions
 like the model validation could be extracted out so that it is easily
 tested.


 @Hackers: This was how we tried to test this feature:
 1 - Started pgAdmin
 2 - Opened the query tool for a specific server
 3 - Executed a SQL statment
 4 - Pressed the column header to try to order, nothing happened
 5 - Right clicked the column header to see if it was there the
 option, nothing

 This is the behavior that we were expecting, not to have to open
 Data View and then press the icon that is not even near the grid in 
 order
 to sort the column. Is this really the way we want people to use the 
 grid
 in pgAdmin? Should it be more intuitive?

>>>
>>> Have we considered making the grid behave more like excel or other
>>> grids? I think that having the ascending and descending inside the 
>>> column
>>> header, we could similarly provide filtering. Something that would give
>>> users a more intuitive place to look.
>>>
>>
>> Doing the sorting via header clicks is convenient but very
>> restrictive. How do you specify multiple columns to sort by for example?
>> The current design allows you to select columns and the sort order as you
>> see fit.
>>
>
 Honestly I'm not sold on my idea, I was just proposing an alternative
 in an effort to start a discussion about the user experience. Ideally what
 I'd like to see, maybe this happened, is some user research. When we
 initial worked on refactoring the results grid we made a bunch of changes.
 One of the things we intended to do was to follow up to see how people were
 using the grid now so that we could better understand how it was now being
 used in order to design and implement features just like this. Clearly we
 haven't gotten there yet.


>
> Another reason we can't use that because w
> e have already occupied that behaviour for selecting entire column
> ​ when user clicks on header.
> As Dave suggested, I will be merging it with filter dialog meaning it
> will be accessible via direct button on toolbar & keyboard shortcut.​
> ​
>

 How are users currently interacting with that filter dialog?

>>>
>>> ​By clicking on the toolbar button as well as keyboard shortcut.
>>> ​
>>>
>>>
>>>
>>>
>>
>> Sorry I wasn't clear. My question was more along the lines of, do we
>> know if people are using the filter functionality?  What kind of filters
>> are people using?  What do they like about it? What do they wish they could
>> do above and beyond sorting, etc.
>>
> ​I have not done any data gathering from users so I can't comment on your
> queries.
> ​ but a​
> ​s far as I understood from the feature requests that most of the users
> expect to have functionality which will allow then to sort columns as it
> was in pgAdmin3.​
>
>
>
>> -- Rob
>>
>>
>>>
 What I'm suggesting is that we understand how users want to interact
 with their results, be those the results of a query or a table view, then
 we can design something that meets those needs. I agree that changing the
 column selection behavior isn't desirable, howe

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Dave Page]

Hi

On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch which allow user to configure how many rows they wish to display
> for any pgagent jobs on statistics panel.
>

I think this is essentially good, however, I'm really not happy with the
preference name and category. In general, I'd suggest that before creating
patches in the future we should confirm naming etc on the mailing list, as
I often end up changing wording and then requiring new screenshots etc.

In this case, I really don't like that we've added another category, and
quite a specific one at that. I would suggest we move it to Browser ->
Properties and name it "Maximum job history rows" with a description of
"The maximum number of history rows to show on the Statistics tab for
pgAgent jobs."

Thoughts?

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

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

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Khushboo Vashi]

On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:

> Hi
>
> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch which allow user to configure how many rows they wish to
>> display for any pgagent jobs on statistics panel.
>>
>
> I think this is essentially good, however, I'm really not happy with the
> preference name and category. In general, I'd suggest that before creating
> patches in the future we should confirm naming etc on the mailing list, as
> I often end up changing wording and then requiring new screenshots etc.
>
> +1

> In this case, I really don't like that we've added another category, and
> quite a specific one at that. I would suggest we move it to Browser ->
> Properties and name it "Maximum job history rows" with a description of
> "The maximum number of history rows to show on the Statistics tab for
> pgAgent jobs."
>
> Thoughts?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Murtuza Zabuawala]

On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:

> Hi
>
> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch which allow user to configure how many rows they wish to
>> display for any pgagent jobs on statistics panel.
>>
>
> I think this is essentially good, however, I'm really not happy with the
> preference name and category. In general, I'd suggest that before creating
> patches in the future we should confirm naming etc on the mailing list, as
> I often end up changing wording and then requiring new screenshots etc.
>
​Ok​


>
> In this case, I really don't like that we've added another category, and
> quite a specific one at that. I would suggest we move it to Browser ->
> Properties and name it "Maximum job history rows" with a description of
> "The maximum number of history rows to show on the Statistics tab for
> pgAgent jobs."
>
I'll change it and resend the patch.


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

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

[Robert Eckhardt]

On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On Wed, Apr 4, 2018 at 8:09 PM, Dave Page  wrote:
>
>>
>>
>> On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> On Wed, Apr 4, 2018 at 5:00 PM, Dave Page  wrote:
>>>


 On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> On Wed, Apr 4, 2018 at 2:47 PM, Dave Page  wrote:
>
>>
>>
>> On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Tue, Apr 3, 2018 at 9:03 PM, Dave Page  wrote:
>>>
 Hi

 On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> Thanks Joao for reviewing.
>
> PFA updated patch.
>
> On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello,
>>
>> On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>>
>>> ​Hello,
>>>
>>> Please find updated patch,
>>>
>>> Now layout will be locked after user updates its preferences, w
>>> e have used ​
>>> templated variable in the javascript file
>>> ​ because we do not have preference module or preference cache
>>> available when the page loads and panels gets rendered,
>>> ​I
>>> ​ also
>>> made changes in JS tests as per Joao's review comments.
>>>
>> Looks like everything is working when we change the lock.
>> As a personal preferences I would prefer to see this in at least
>> 2 commits, one that is related to the preference issue and another 
>> one that
>> is related to this story.
>>
>>
>> All the tests are working, but he linter is failing:
>>
>> /tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
>>  
>> 
>> ./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
>>  
>> 
>> 1   E303 too many blank lines (2)
>>  
>> 
>>
>> 1
>>
> ​Fixed​
>
>
>>
>>
>>> @Dave/Pivotal team,
>>> The given patch is working fine for all the Tabs/Panels (all the
>>> panels from main window as well as from Query tool and Debugger) 
>>> but I'm
>>> facing an issue while handling the Browser tree section, It is a 
>>> wcDocer
>>> frame  and
>>> not a wcDocker panel
>>> . Like
>>> wcDocker panel, wcDocker frame do not provide any API so that a 
>>> developer
>>> can prevent drag-drop functionality on it.
>>>
>>
 It's not working fine for me. For example, if I put the SQL Panel
 on it's own below the properties/stats panels (so it looks like 
 pgAdmin 3
 used to by default), and then lock the layout, I can un-dock the SQL 
 panel
 into a dialogue, but then cannot re-dock it. I can do weird things 
 with the
 browser tree as well, probably because it's a frame as you say.

>>>
>>> ​That is expected behaviour ​because once you drag the panel out of
>>> the group of Panels then it becomes individual Frame, That is what the
>>> author of the wcDocker replied on my question,
>>> *"A panel must either be initialized as movable or non-movable from
>>> the beginning and never changed because it generates a different
>>> arrangement of elements depending. This feature should only ever be used
>>> within the onCreate method of the panel. I should probably have been 
>>> more
>>> clear about this limitation in the documentation."*
>>>
>>>
>> So does it become a panel again if a second panel is added to the new
>> tab group?
>>
> ​No, it stays Frame.​
>
> As far as I understand Panel needs a Frame to render itself if it is
> not attached to the main docker instance.​
>
>>
>> There must be some way we can lock a tab that's not part of a group.
>

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Joao De Almeida Pereira]

Hi Murtuza,
Generally the patch looks good passes all CI but the linter fails:

/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2
 

./pgadmin/browser/register_browser_preferences.py:11: [E302] expected
2 blank lines, found 1
 

1   E302 expected 2 blank lines, found 1
 

1


Did not test the functionality itself 

Thanks
Joao

On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find update patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi,

 PFA patch which allow user to configure how many rows they wish to
 display for any pgagent jobs on statistics panel.

>>>
>>> I think this is essentially good, however, I'm really not happy with the
>>> preference name and category. In general, I'd suggest that before creating
>>> patches in the future we should confirm naming etc on the mailing list, as
>>> I often end up changing wording and then requiring new screenshots etc.
>>>
>> ​Ok​
>>
>>
>>>
>>> In this case, I really don't like that we've added another category, and
>>> quite a specific one at that. I would suggest we move it to Browser ->
>>> Properties and name it "Maximum job history rows" with a description of
>>> "The maximum number of history rows to show on the Statistics tab for
>>> pgAgent jobs."
>>>
>> I'll change it and resend the patch.
>>
>>
>>>
>>> Thoughts?
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Dave Page]

Murtuza, please send me the screenshot as a PNG. I've been tweaking the
code and will recreate the patch with my changes.

On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Murtuza,
> Generally the patch looks good passes all CI but the linter fails:
>
> /tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2
>  
> 
> ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank 
> lines, found 1
>  
> 
> 1   E302 expected 2 blank lines, found 1
>  
> 
> 1
>
>
> Did not test the functionality itself 
>
> Thanks
> Joao
>
> On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find update patch.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala > enterprisedb.com> wrote:
>>
>>> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:
>>>
 Hi

 On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala >>> enterprisedb.com> wrote:

> Hi,
>
> PFA patch which allow user to configure how many rows they wish to
> display for any pgagent jobs on statistics panel.
>

 I think this is essentially good, however, I'm really not happy with
 the preference name and category. In general, I'd suggest that before
 creating patches in the future we should confirm naming etc on the mailing
 list, as I often end up changing wording and then requiring new screenshots
 etc.

>>> ​Ok​
>>>
>>>

 In this case, I really don't like that we've added another category,
 and quite a specific one at that. I would suggest we move it to Browser ->
 Properties and name it "Maximum job history rows" with a description of
 "The maximum number of history rows to show on the Statistics tab for
 pgAgent jobs."

>>> I'll change it and resend the patch.
>>>
>>>

 Thoughts?

 --
 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: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Dave Page]

Sorry Murtuza - I'm mixing patches up here. It's the sort/filter one I need
the image for please.

Which means this patch needs re-creating as well please.

On Thu, Apr 5, 2018 at 3:03 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find attached PNG.
>
> On Thu, Apr 5, 2018 at 7:25 PM, Dave Page  wrote:
>
>> Murtuza, please send me the screenshot as a PNG. I've been tweaking the
>> code and will recreate the patch with my changes.
>>
>> On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Murtuza,
>>> Generally the patch looks good passes all CI but the linter fails:
>>>
>>> /tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2
>>>  
>>> 
>>> ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 
>>> blank lines, found 1
>>>  
>>> 
>>> 1   E302 expected 2 blank lines, found 1
>>>  
>>> 
>>> 1
>>>
>>>
>>> Did not test the functionality itself 
>>>
>> ​@Joao,
> I have included the test for Stats call which will cover the newly added
> functionality.​
>
>
>> Thanks
>>> Joao
>>>
>>> On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 Please find update patch.

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


 On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch which allow user to configure how many rows they wish to
>>> display for any pgagent jobs on statistics panel.
>>>
>>
>> I think this is essentially good, however, I'm really not happy with
>> the preference name and category. In general, I'd suggest that before
>> creating patches in the future we should confirm naming etc on the 
>> mailing
>> list, as I often end up changing wording and then requiring new 
>> screenshots
>> etc.
>>
> ​Ok​
>
>
>>
>> In this case, I really don't like that we've added another category,
>> and quite a specific one at that. I would suggest we move it to Browser 
>> ->
>> Properties and name it "Maximum job history rows" with a description of
>> "The maximum number of history rows to show on the Statistics tab for
>> pgAgent jobs."
>>
> I'll change it and resend the patch.
>
>
>>
>> Thoughts?
>>
>> --
>> 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
>>
>
>


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

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

pgAdmin 4 commit: Allow sorting when viewing/editing data. Fixes #1894

[Dave Page]

Allow sorting when viewing/editing data. Fixes #1894

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=fa1854bd85502ea9db2716f18d5edf0bd79cacdc
Author: Murtuza Zabuawala 

Modified Files
--
docs/en_US/editgrid.rst|  23 ++
docs/en_US/images/editgrid_filter_dialog.png   | Bin 0 -> 68460 bytes
docs/en_US/release_notes_3_0.rst   |   2 +-
web/pgadmin/static/js/sqleditor/filter_dialog.js   | 243 +
.../static/js/sqleditor/filter_dialog_model.js | 133 +++
.../tools/datagrid/templates/datagrid/index.html   |  33 +--
web/pgadmin/tools/sqleditor/__init__.py| 115 --
web/pgadmin/tools/sqleditor/command.py | 187 ++--
.../tools/sqleditor/static/css/sqleditor.css   |  23 ++
web/pgadmin/tools/sqleditor/static/js/sqleditor.js |  85 +--
.../sqleditor/sql/default/get_columns.sql  |   9 +
.../sqleditor/sql/default/objectquery.sql  |   6 +-
web/pgadmin/tools/sqleditor/utils/filter_dialog.py |  95 
.../utils/tests/test_filter_dialog_callbacks.py| 103 +
.../javascript/sqleditor/filter_dialog_specs.js|  31 +++
15 files changed, 894 insertions(+), 194 deletions(-)

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Dave Page]

Hi

The description for the Preferences field doesn't seem to be there in the
code or the screenshot. I suggested below using: "The maximum number of
history rows to show on the Statistics tab for pgAgent jobs."

Thanks.

On Thu, Apr 5, 2018 at 1:09 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find update patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi,

 PFA patch which allow user to configure how many rows they wish to
 display for any pgagent jobs on statistics panel.

>>>
>>> I think this is essentially good, however, I'm really not happy with the
>>> preference name and category. In general, I'd suggest that before creating
>>> patches in the future we should confirm naming etc on the mailing list, as
>>> I often end up changing wording and then requiring new screenshots etc.
>>>
>> ​Ok​
>>
>>
>>>
>>> In this case, I really don't like that we've added another category, and
>>> quite a specific one at that. I would suggest we move it to Browser ->
>>> Properties and name it "Maximum job history rows" with a description of
>>> "The maximum number of history rows to show on the Statistics tab for
>>> pgAgent jobs."
>>>
>> I'll change it and resend the patch.
>>
>>
>>>
>>> Thoughts?
>>>
>>> --
>>> 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