Hi Below are my review comments:
- Definition box on the View dialogue not expanded as needed. It should expand on resize. - "Do Instead" on Rule node under View/M-View node not working. Unable to generate proper SQL. - I am still able to drop columns under view/mview node. - "Save" button is enable by default when user opens Materialised View dialog. On Fri, May 13, 2016 at 11:44 PM, Surinder Kumar < surinder.ku...@enterprisedb.com> wrote: > Hi, > > Please find updated patch with fixed review comments: > > Most of the issues occurred because some code was missing in tables > subnodes patch. > Now I have shared the code related to table subnodes with harshal to > integrate in tables patch. > > This patch has dependency on tables patch. > > On Tue, May 10, 2016 at 7:46 PM, Akshay Joshi < > akshay.jo...@enterprisedb.com> wrote: > >> Hi Surinder >> >> On Fri, Apr 29, 2016 at 8:07 PM, Surinder Kumar <surinder.kumar@ >> enterprisedb.com> wrote: >> >>> Hi, >>> >>> >>> PFA patch for View and Materialised View Nodes. >>> This patch is dependent on *tables node* and its subnodes patch. >>> Please test this patch once latest tables node patch is submitted. >>> >>> I have merged one other patch: >>> *"Don't show Security group of node if it is under Catalogs"* >>> >>> http://www.postgresql.org/message-id/cam5-9d8rnzxrg0ygvurqf2wqaenye-rqbhsrwcr34dnlcem...@mail.gmail.com >>> >>> Below are the fix for the review comments given by Dave: >>> >>> *Views*: >>> >>> - Please add sqlCreateHelp and sqlAlterHelp properties to all nodes. >>> *I have added these in View and Materialised View. * >>> >>> - Some of the SQL templates have inconsistent indenting (i.e. not 4 >>> chars), e.g. >>> >>> CREATE OR REPLACE VIEW pem.avail_agents >>> AS >>> ... >>> *Fixed* >>> >>> - When creating any object, the Owner, Schema and Tablespace should >>> have default values. >>> *Fixed* >>> >>> - Property labels should only have the first word capitalised, e.g. >>> >>> Security barrier >>> Check options >>> *Fixed* >>> >>> - The Definition box on the View dialogue should start at a single >>> line and expand as needed - see the Function dialogue >>> *Fixed* >>> >> >> Still reproducible. >> >>> >>> >>> - The Cancel button doesn't work on the View dialog. Please check them >>> all. >>> *In latest code pull, It **seems to be** fixed. not reproducible at my >>> end.* >>> >>> - The Save button doesn't close the View dialog, nor does it add the >>> tree node. >>> *In latest code pull, It **seems to be** fixed. not reproducible at my >>> end.* >>> >>> - Dependencies and Dependents don't show icons, and have very tall >>> rows. This issue likely comes from elsewhere as I'm seeing it on other >>> object types as well. >>> Yes, it is general, previously images were visible. It is regression of >>> some commit. I will check it. >>> >>> - Reverse engineered SQL for a column on a view is shown like: >>> >>> <html><head></head><body>-- Column: id -- ALTER TABLE pem.avail_agents >>> DROP COLUMN id; ALTER TABLE pem.avail_agents ADD COLUMN id >>> integer(4);</body></html> >>> >>> (yes, it's displaying the HTML tags) >>> *I pulled the latest code, but it is not reproducible.* >>> >>> - The ACL property should be called Privileges and be in the Security >>> group (see functions, sequences etc). >>> *Fixed* >>> >>> - The Schema should not be shown in "Properties" view. >>> *Fixed* >>> >>> - Fields in the General section should be in the order: Name, OID, >>> Owner, System xxx? (where appropriate), Comment >>> *I have checked that In view Fields (Name, Owner, Schema & Comment) are >>> in this order.* >>> *I didn't got your point. Can you please give and example, if possible.* >>> >> >> Not Fixed yet. >> > Now it is fixed. > >> >>> >>> >>> *Materialised Views:* >>> - Selecting an MV results in: >>> >>> 2016-04-14 09:34:58,863: ERROR pgadmin: Exception on >>> /browser/materialized_view/obj/1/1/24587/27424/107612 [GET] >>> Traceback (most recent call last): >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/app.py", >>> line 1817, in wsgi_app >>> response = self.full_dispatch_request() >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/app.py", >>> line 1477, in full_dispatch_request >>> rv = self.handle_user_exception(e) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/app.py", >>> line 1381, in handle_user_exception >>> reraise(exc_type, exc_value, tb) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/app.py", >>> line 1475, in full_dispatch_request >>> rv = self.dispatch_request() >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/app.py", >>> line 1461, in dispatch_request >>> return self.view_functions[rule.endpoint](**req.view_args) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/views.py", >>> line 84, in view >>> return self.dispatch_request(*args, **kwargs) >>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line >>> 233, in dispatch_request >>> return method(*args, **kwargs) >>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_ >>> groups/servers/databases/schemas/views/__init__.py", >>> line 185, in wrap >>> return f(*args, **kwargs) >>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_ >>> groups/servers/databases/schemas/views/__init__.py", >>> line 1226, in properties >>> self.conn, result, 'table') >>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_ >>> groups/servers/databases/schemas/utils.py", >>> line 357, in parse_vacuum_data >>> vacuum_fields = render_template("vacuum_ >>> settings/vacuum_fields.json") >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/templating.py", >>> line 127, in render_template >>> return _render(ctx.app.jinja_env.get_or_select_template(template_ >>> name_or_list), >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/jinja2/environment.py", >>> line 830, in get_or_select_template >>> return self.get_template(template_name_or_list, parent, globals) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/jinja2/environment.py", >>> line 791, in get_template >>> return self._load_template(name, self.make_globals(globals)) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/jinja2/environment.py", >>> line 765, in _load_template >>> template = self.loader.load(self, name, globals) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/jinja2/loaders.py", >>> line 113, in load >>> source, filename, uptodate = self.get_source(environment, name) >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site- >>> packages/flask/templating.py", >>> line 64, in get_source >>> raise TemplateNotFound(template) >>> TemplateNotFound: vacuum_settings/vacuum_fields.json >>> >>> *It seems this patch* >>> >>> http://www.postgresql.org/message-id/cam5-9d-4dygmmhfc5mzdddk++h7xnvkia5ojbbxi5sthmec...@mail.gmail.com >>> *wasn't applied properly. Now this patch is sent with tables patch.* >>> >>> - When creating an MV, an error is immediately shown telling me to >>> specify a name. We do not normally show such errors until the field >>> loses focus. >>> *Now its fixed.* >>> >> >> Not fixed. Issue is reproducible. >> >>> >>> >>> - No default owner is shown in create mode. >>> *Fixed.* >>> >>> - "Please enter function definition." is shown as an error on the >>> definition field. >>> *It triggers this error message as it is mandatory.* >>> >>> - The Definition field is missing a label. If relying on the tab title >>> to be the label, the control should fill the tab (as with Security >>> options etc). >>> *Fixed.* >>> >>> - No default tablespace is shown. >>> *Fixed* >>> >>> - If I enable custom auto-vacuum, there is no way to add a row to >>> specify options. >>> *You can enable custom auto vacuum field and add new values in the auto >>> vacuum settings.* >>> >>> - If I specify a comment on an MV, I get "ERROR: "mv_pg_tables" is not a >>> view" >>> *For some reason tablespace name was missing in generated sql.* >>> >> >> Not Fixed. Wrong sql generated "COMMENT ON VIEW ..." instead of >> "COMMENT ON MATERIALIZED VIEW ..." >> > Fixed > >> >>> >>> - Save/Cancel buttons not working as expected (like other objects) >>> *In latest code pull, It **seems to be** fixed. not reproducible at my >>> end.* >>> >> >> >> Apart from above below are my review comments >> >> *Views*:- >> >> - As per pgAdmin3 "indexes" node should not be listed under Views. >> >> Done > >> >> - SQL not generated when changing the value of "Event" and "Do >> Instead" for Rule node under View node. >> >> Done > >> >> - Changing the "Event" in Rule node not working. >> >> Done > >> >> - As per pgAdmin3 user can't be able to create columns inside View >> node. >> >> Done > >> >> - User can't be able to delete/drop columns and system generated >> Rule's, Trigger's etc.. >> >> > Done > >> >> - Found one issue when changing value of "Security Barrier" from >> "Yes" to "No" it is not reflected on GUI when we open the dialog >> again while in backend value is updated, but on GUI it is showing >> "Yes". >> >> Done > >> >> - Create -> Trigger menu is missing when any view node is selected. >> >> Done > >> >> *Materialized View:-* >> >> - *As per pgAdmin3 user can't be able to create columns inside >> Materialized View node.* >> >> Done > >> >> - >> - >> *User can't be able to delete/drop columns and system generated Rule's, >> Trigger's etc. * >> >> I have checked in pgAdmin3 that delete/drop option specific to rules has > 2 cases: > 1. If rule is system rule, user can't delete/drop it. > 2. If not system rule, it can drop dropped. > Done > >> >> - >> - Create -> Trigger/Rule/Index menu is missing when any materialized >> view node is selected. >> >> Done > >> >> - In pgAdmin3 we have two more refresh options "Refresh data" and >> "Refresh data concurrently" which is missing. >> >> Done > >> >> - "Custom Auto Vaccum" for Table and Toast Table not working. >> >> Done > >> >> >>> >>> Thanks, >>> Surinder Kumar >>> >>> >>> >>> -- >>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) >>> To make changes to your subscription: >>> http://www.postgresql.org/mailpref/pgadmin-hackers >>> >>> >> >> >> -- >> *Akshay Joshi* >> *Principal Software Engineer * >> >> >> >> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* >> > > -- *Akshay Joshi* *Principal Software Engineer * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
