Re: [pgAdmin4] [Patch]: Grant Wizard - Mailing list pgadmin-hackers
From | Surinder Kumar |
---|---|
Subject | Re: [pgAdmin4] [Patch]: Grant Wizard |
Date | |
Msg-id | CAM5-9D_TmnFfVEw1OXTL-f4jgtCiE+e2rQuSKa4yqPX7ZsSBNg@mail.gmail.com Whole thread Raw |
In response to | Re: [pgAdmin4] [Patch]: Grant Wizard (Dave Page <dpage@pgadmin.org>) |
Responses |
Re: [pgAdmin4] [Patch]: Grant Wizard
|
List | pgadmin-hackers |
Hi
PFA patch with resolved review comments.
On Fri, Apr 8, 2016 at 12:52 AM, Dave Page <dpage@pgadmin.org> wrote:
HiNearly there :-). Assuming no regressions, I believe we'll be ready to commit once the following issues are resolved:- The select object grid should fill the available vertical space in the dialogue - at present there's a gap below it.
Done
- When scrolling to the top of bottom of the select object grid, the dialogue jumps up or down the screen a little, if the dialogue has been resized to a larger size.
As this change is generic for all alertifyjs dialogs, I have added its style css in overrides.css. Done
- Please allow the wizard to be opened from a Database node, in which case objects from all schemas should be listed. This will also require support for schemas themselves.
Done
- When selecting privileges, each time I click on a checkbox, the row closes. Similar grids elsewhere in the app close the row when the cell loses focus, *however*, that is also the incorrect behaviour - the row should only close when the row itself loses focus (and should open when it gets focus).
I checked the row closes either when gets clicked on row or outside row, it doesn't seems to close on click on checkbox.
- The message:Please select objects from the below list.should read:Please select objects from the list below.
Done
- The message:Following query will be executed on the database server for the selected objects, and privileges. Please click on Finish to complete the process.should read:The SQL below will be executed on the database server to grant the selected privileges. Please click on <b>Finish</b> to complete the process.
Done
Thanks!On Thu, Apr 7, 2016 at 6:10 PM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:Hi Dave,Please find updated patch with above issue resolved.On Thu, Apr 7, 2016 at 7:47 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Apr 6, 2016 at 12:37 PM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:HiPFA updated patch with resolved review comments.On Tue, Apr 5, 2016 at 11:06 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:On Wed, Mar 30, 2016 at 5:14 PM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:Hi Surinder,Hi,Please find updated patch.This patch has following changes:1. Improved code commenting.2. Properly handling memory leak issues in js code.As discussed offline, here are the list of some of the review comments:* CSS should be relative to its parent element. Please make sure - whenever you makesome changes in CSS, it should not affect the existing CSS unless discussed.Done* Change class name for 'error_msg_div' as it is common name. Please name a classwith prefixed as the module name.Done* Add comments for the blow line changed in node.ui.js file. Always add logicalexplanation for a change as a comment for any changes.while(p && p.length > 0) {Done* Please make sure, we wrap the code around 80 characters for better readability.Line length should not be greater than 80 characters.Done* Put the allowed ACLs logic with server version support. We need to be flexibleenough to accommodate possible future change in ACLs.Done* Avoid using name as reference in each of the given. It will make the search fasterin the database and less prone to character conversion issue.i.e.Use schema/namespace OID instead of nspname, object OID instead of their name.Done* Use separate templates for each type of objects.Done* Use the existing functionalities as much as possible instead of introducing newone. That will make the code/results consistent across the application.i.e.Use existing 'parse_priv_to_db' method, instead of creating new one.Done* Please remove unnecessary suffixed white-spaces.DoneI get the attached error in the browser console when selecting "Grant Wizard" from the menu when the current object is a schema. I've tried all the normal refreshing/restarting.
--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
Attachment
pgadmin-hackers by date: