Re: pgAdmin Event Trigger Compatibility

Hi Dinesh

I have reviewed your event trigger patch. Following is my review comments:

- There is some problem in "refresh" logic. When I clicked on newly
created event trigger node, it gets collapsed automatically and the parent
node gets selected.
- According to the logic you have disabled the "Default Value" text box
in "New Trigger Function..." dialog, but when we select the "IN" radio
button on "Parameter" tab it gets enabled.
- There should be a status bar in "New Event Trigger" dialog. With
current implementation user will not know about which inputs are
needed/remaining to enable the "OK" button.
- In pgFunction.cpp why we are checking the typname as *"\"trigger\""*and
*"trigger"* if these the special case then should we need it for
event_trigger as well? Below is the code syntax
- "else if (typname == wxT("\"trigger\"") || typname ==
wxT("trigger") || typname == wxT("event_trigger"))"
- On "New Event Trigger" dialog help is not working. When we clicked on
Help button it will show "The URL you specified does not exist" in web
browser.

Other than the above code looks good to me.

On Mon, Jul 22, 2013 at 6:39 PM, Dinesh Kumar <dinesh(dot)kumar(at)enterprisedb(dot)com
> wrote:

> Hi Akshay,
>
> Thank you so much for correcting me on this. It's my bad to forgot the
> naming convention in the code. Now, ::OnOK() is get populating the required
> SQL as per the changes.
>
> Please find the new patch and let me know your valuable inputs.
>
>
> Dinesh
>
> --
> *Dinesh Kumar*
> Software Engineer
>
> Ph: +918087463317
> Skype ID: dinesh.kumar432
> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
> *
> Follow us on Twitter*
> @EnterpriseDB
>
> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community> and
> more <http://www.enterprisedb.com/resources-community>
>
>
> On Mon, Jul 22, 2013 at 5:58 PM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Dinesh
>>
>> I have just started reviewing your code and found on issue where any
>> changes made on Event Trigger dialog won't be saved. After looking at the
>> code, you haven't override the ::OnOK() function in dlgEventTrigger.cpp.
>> Can you please fix this and resend the new patch.
>>
>>
>> On Wed, Jul 10, 2013 at 9:46 PM, Dinesh Kumar <
>> dinesh(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>>
>>> Yes, we can change this enable check box as a radio button. But,
>>>>> "REPLICA/ALWAYS" are two enable's properties. Hence, We have implemented
>>>>> this in the proposed way. Kindly share your opinion on this.
>>>>>
>>>>
>>>> So: "Enabled Replica ( ) Enabled Always ( ) Disabled ( )" ?
>>>>
>>>
>>> Yes Dave.
>>>
>>>
>>>
>>> Dinesh
>>>
>>> --
>>> *Dinesh Kumar*
>>> Software Engineer
>>>
>>> Ph: +918087463317
>>> Skype ID: dinesh.kumar432
>>> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
>>> *
>>> Follow us on Twitter*
>>> @EnterpriseDB
>>>
>>> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community> and
>>> more <http://www.enterprisedb.com/resources-community>
>>>
>>
>>
>>
>> --
>> *Akshay Joshi
>> Senior Software Engineer
>> EnterpriseDB Corporation
>> The Enterprise PostgreSQL Company
>> Phone: +91 20-3058-9522
>> Mobile: +91 976-788-8246*
>>
>
>

--
*Akshay Joshi
Senior Software Engineer
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20-3058-9522
Mobile: +91 976-788-8246*