Thread: [pgAdmin][RM4144] Compound Trigger support for EPAS-v12

[pgAdmin][RM4144] Compound Trigger support for EPAS-v12

From
Akshay Joshi
Date:
Hi Hackers

I have added the support of Compound Trigger in pgAdmin4 for EPAS v12 and above. I have created a new collection node named "Compound Triggers" under the Tables node. API and RE-SQL test cases are covered and documentation is done.

Please review the attached patch. 

--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246
Attachment

Re: [pgAdmin][RM4144] Compound Trigger support for EPAS-v12

From
Dave Page
Date:
Thanks Akshay. I want to do a pass over the docs before this is committed. Otherwise, I don't see any obvious issues - however, it's a pretty big patch, so can you have one of the others on the team do a review/test as well please?


On Wed, Aug 7, 2019 at 8:08 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

I have added the support of Compound Trigger in pgAdmin4 for EPAS v12 and above. I have created a new collection node named "Compound Triggers" under the Tables node. API and RE-SQL test cases are covered and documentation is done.

Please review the attached patch. 

--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


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

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

Re: [pgAdmin][RM4144] Compound Trigger support for EPAS-v12

From
Aditya Toshniwal
Date:
I'll do.

On Wed, Aug 7, 2019 at 3:34 PM Dave Page <dpage@pgadmin.org> wrote:
Thanks Akshay. I want to do a pass over the docs before this is committed. Otherwise, I don't see any obvious issues - however, it's a pretty big patch, so can you have one of the others on the team do a review/test as well please?


On Wed, Aug 7, 2019 at 8:08 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

I have added the support of Compound Trigger in pgAdmin4 for EPAS v12 and above. I have created a new collection node named "Compound Triggers" under the Tables node. API and RE-SQL test cases are covered and documentation is done.

Please review the attached patch. 

--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


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

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


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"

Re: [pgAdmin][RM4144] Compound Trigger support for EPAS-v12

From
Aditya Toshniwal
Date:
Hi Akshay,

Everything seems to be fine. I have a few suggestions:
1) In class CompoundTriggerModule (python), "A module class for Trigger" should be "A module class for Compound Trigger"
2) We need to send a pull request to Code Mirror to highlight to make show the word - COMPOUND as a keyword.
3) Add the trigger name after END of the compound trigger definition. eg END <cp trig>;
4) Table SQL tab should show compound triggers.
5) Compound trigger disabled icon is cut. @Chethana Kumar - the icon need to be revisited.
Screenshot 2019-08-08 at 11.58.27.png
6) In edit, SQL generated (msql) is having minor spacing issues on the "FOR ..." line. Eg:
CREATE OR REPLACE TRIGGER comptrig
    FOR  INSERT  OR DELETE   OR UPDATE

You can commit the code once fixed.


On Wed, Aug 7, 2019 at 4:00 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
I'll do.

On Wed, Aug 7, 2019 at 3:34 PM Dave Page <dpage@pgadmin.org> wrote:
Thanks Akshay. I want to do a pass over the docs before this is committed. Otherwise, I don't see any obvious issues - however, it's a pretty big patch, so can you have one of the others on the team do a review/test as well please?


On Wed, Aug 7, 2019 at 8:08 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

I have added the support of Compound Trigger in pgAdmin4 for EPAS v12 and above. I have created a new collection node named "Compound Triggers" under the Tables node. API and RE-SQL test cases are covered and documentation is done.

Please review the attached patch. 

--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


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

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


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Attachment

Re: [pgAdmin][RM4144] Compound Trigger support for EPAS-v12

From
Chethana Kumar
Date:
Hi Aditya,

I am sending across the updated icons.

---
Chethana kumar

On Thu, Aug 8, 2019 at 12:19 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Akshay,

Everything seems to be fine. I have a few suggestions:
1) In class CompoundTriggerModule (python), "A module class for Trigger" should be "A module class for Compound Trigger"
2) We need to send a pull request to Code Mirror to highlight to make show the word - COMPOUND as a keyword.
3) Add the trigger name after END of the compound trigger definition. eg END <cp trig>;
4) Table SQL tab should show compound triggers.
5) Compound trigger disabled icon is cut. @Chethana Kumar - the icon need to be revisited.
Screenshot 2019-08-08 at 11.58.27.png
6) In edit, SQL generated (msql) is having minor spacing issues on the "FOR ..." line. Eg:
CREATE OR REPLACE TRIGGER comptrig
    FOR  INSERT  OR DELETE   OR UPDATE

You can commit the code once fixed.


On Wed, Aug 7, 2019 at 4:00 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
I'll do.

On Wed, Aug 7, 2019 at 3:34 PM Dave Page <dpage@pgadmin.org> wrote:
Thanks Akshay. I want to do a pass over the docs before this is committed. Otherwise, I don't see any obvious issues - however, it's a pretty big patch, so can you have one of the others on the team do a review/test as well please?


On Wed, Aug 7, 2019 at 8:08 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

I have added the support of Compound Trigger in pgAdmin4 for EPAS v12 and above. I have created a new collection node named "Compound Triggers" under the Tables node. API and RE-SQL test cases are covered and documentation is done.

Please review the attached patch. 

--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


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

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


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


--
Chethana Kumar
Principal UI/UX Designer
EnterpriseDB Corporation


The Postgres Database Company

P: +91 86981 57146
Attachment

Re: [pgAdmin][RM4144] Compound Trigger support for EPAS-v12

From
Chethana Kumar
Date:
Missed the attachment.

On Thu, Aug 8, 2019 at 2:59 PM Chethana Kumar <chethana.kumar@enterprisedb.com> wrote:
Hi Aditya,

I am sending across the updated icons.

---
Chethana kumar

On Thu, Aug 8, 2019 at 12:19 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Akshay,

Everything seems to be fine. I have a few suggestions:
1) In class CompoundTriggerModule (python), "A module class for Trigger" should be "A module class for Compound Trigger"
2) We need to send a pull request to Code Mirror to highlight to make show the word - COMPOUND as a keyword.
3) Add the trigger name after END of the compound trigger definition. eg END <cp trig>;
4) Table SQL tab should show compound triggers.
5) Compound trigger disabled icon is cut. @Chethana Kumar - the icon need to be revisited.
Screenshot 2019-08-08 at 11.58.27.png
6) In edit, SQL generated (msql) is having minor spacing issues on the "FOR ..." line. Eg:
CREATE OR REPLACE TRIGGER comptrig
    FOR  INSERT  OR DELETE   OR UPDATE

You can commit the code once fixed.


On Wed, Aug 7, 2019 at 4:00 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
I'll do.

On Wed, Aug 7, 2019 at 3:34 PM Dave Page <dpage@pgadmin.org> wrote:
Thanks Akshay. I want to do a pass over the docs before this is committed. Otherwise, I don't see any obvious issues - however, it's a pretty big patch, so can you have one of the others on the team do a review/test as well please?


On Wed, Aug 7, 2019 at 8:08 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

I have added the support of Compound Trigger in pgAdmin4 for EPAS v12 and above. I have created a new collection node named "Compound Triggers" under the Tables node. API and RE-SQL test cases are covered and documentation is done.

Please review the attached patch. 

--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


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

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


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


--
Chethana Kumar
Principal UI/UX Designer
EnterpriseDB Corporation


The Postgres Database Company

P: +91 86981 57146


--
Chethana Kumar
Principal UI/UX Designer
EnterpriseDB Corporation


The Postgres Database Company

P: +91 86981 57146
Attachment