Re: [pgAdmin4][Patch]: RM1728 - Properties are not refreshing after objects are edited

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM1728 - Properties are not refreshing after objects are edited
Date: 2016-10-17 06:48:05
Message-ID: CAM5-9D9Yjg-oG3j7FyzSqifYcUMhwaFN_gWv7-9gss0Nkor25Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Sun, Oct 16, 2016 at 7:29 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> I just found a case where this patch is broken - if you update the comment
> on a type, it looks like it tried to lookup the schema ID using the type
> name, which a) isn't in the posted data so gives a 500 response, and b)
> wouldn't be safe anyway, if there were types with the same name in multiple
> schemas.
>
​I have fixed this issue. Now it will lookup the schema ID against the type
id instead of type name.​

>
> Actually, it looks like that's an issue when creating a type too - that is
> also using an unsafe schema lookup.
>
> Please fix this ASAP (i.e. Monday) and double check to ensure we're not
> doing any more unsafe lookups like this.
>
​It looks good to me in other nodes.
Please find attached patch and review.​

>
> Thanks.
>
>
> On Friday, October 14, 2016, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Thanks, applied.
>>
>> On Friday, October 14, 2016, Surinder Kumar <
>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>> Hi
>>>
>>> *Following are the issues fixed in nodes:*
>>>
>>> 1) If we create/update a node with non-default schema, It should return
>>> selected schema id in return response. but default schema id is returned
>>> every time due to which it throws error in properties panel.
>>> Fixed in Domains, Collation, Types, Views & Table node.
>>>
>>> 2) Incorrect parent id of object node is returned from *nodes method*
>>> due to which wrong parent id is passed while updating object and
>>> thus node didn't get refreshed.
>>> Fixed in FTS Configuration, FTS Parser nodes.
>>>
>>> Also, I have kept changes of first patch which are essential to refresh
>>> node every time. Without that patch nodes properties panel updates only
>>> sometimes.
>>>
>>> Please find attached patch. Please review and let me know for comments.
>>>
>>> Thanks
>>> Surinder Kumar
>>>
>>>
>>>
>>> On Fri, Sep 23, 2016 at 6:00 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Umm, no it wasn't - sorry.
>>>>
>>>> I see the same issue with Types. Can you fix that, and check all other
>>>> nodes as well please?
>>>>
>>>> Thanks.
>>>>
>>>> On Fri, Sep 23, 2016 at 1:29 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>> > Thanks, applied.
>>>> >
>>>> > On Fri, Sep 23, 2016 at 12:05 PM, Surinder Kumar
>>>> > <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>> >> Hi,
>>>> >>
>>>> >> Please find updated patch with changes:
>>>> >> 1) On debugging through JS files, the issue was in synonym update
>>>> method
>>>> >> which wasn't returning node object.
>>>> >> 2) retrieving schema name in node.sql for creating node object in
>>>> update
>>>> >> method.
>>>> >>
>>>> >> Please review and let me know for comments.
>>>> >>
>>>> >> On Fri, Sep 23, 2016 at 2:44 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>> wrote:
>>>> >>>
>>>> >>> Hi
>>>> >>>
>>>> >>> On Fri, Sep 23, 2016 at 7:39 AM, Surinder Kumar
>>>> >>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>> >>> > Hi
>>>> >>> >
>>>> >>> > Issue:
>>>> >>> > on updating node, we deselect and then again select the node
>>>> updated to
>>>> >>> > refresh the panel. but it needs some delay of few milliseconds
>>>> between
>>>> >>> > deselect and select to fix this issue.
>>>> >>> >
>>>> >>> > Please find attached patch and review.
>>>> >>>
>>>> >>> This does not resolve the issue for me. I tested using a synonym to
>>>> a
>>>> >>> package on EPAS 9.5, by changing the target package name.
>>>> >>>
>>>> >>>
>>>> >>> --
>>>> >>> 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
>>>>
>>>
>>>
>>
>> --
>> 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 Content-Type Size
fix_unsafe_schema_lookup_issue_in_type.patch application/octet-stream 2.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Surinder Kumar 2016-10-17 07:32:05 Re: [pgAdmin4][Patch]: Listing of files/folders not sorted alphabetically in Storage Manager
Previous Message Surinder Kumar 2016-10-17 04:41:03 Re: pgadmin4 saving query