Re: PATCH: FTS configuration node

From: Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
To: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: FTS configuration node
Date: 2016-05-18 07:11:01
Message-ID: CA+yw=mP4jxMTjAUYwGPYrkjc3H1NOmXHOzPH_nf6NFci0rq6bw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

PFA the revised patch.
Response is lnline.

Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Tue, May 17, 2016 at 12:54 PM, Harshal Dhumal <
harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:

> Hi Sanket,
>
> Please find my review comments below:
>
> 1. In create mode it generates wrong sql.
>
> CREATE TEXT SEARCH CONFIGURATION test.asdf (
> COPY=
> );
>
> steps to reproduce:
> a] Fill up any necessary fields in general tab.
> b] On definition tab set Copy config filed and navigate to SQL tab.
> c] Come back again to definition tab clear Copy config field and set
> Parser field and again navigate to SQL tab
>
Fixed

>
> 2. Create new FTS configuration with parser throws below error.
>
> provide atleast copy config or parser.
>
> (this is because you have used key name for parser in js as "prsname" and
> in python you have applied validations on key "parser")
>
Fixed

>
> 3. When we clear dictionary for any token using cross (x) button in
> select2; JavaScript code for selecct2 fails. (see attached screenshot)
>
This occurs due to issue in select2 control. Its a generalized issue which
will be resolved later.

>
>
>
>
>
>
> --
> *Harshal Dhumal*
> *Software Engineer *
>
>
>
> EenterpriseDB <http://www.enterprisedb.com>
>
> On Mon, May 16, 2016 at 7:37 PM, Sanket Mehta <
> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Revised patch is attached with this mail.
>> My response is inline.
>>
>>
>> Regards,
>> Sanket Mehta
>> Sr Software engineer
>> Enterprisedb
>>
>> On Fri, May 13, 2016 at 6:20 PM, Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Sanket
>>>
>>> Below are my review comments:
>>>
>>>
>>> - Add button should be disabled in Tokens tab while creating.
>>>
>>> Done
>>
>>>
>>> - Unable to click on down arrow for token select control.
>>>
>>> Done
>>
>>>
>>> - Title should be change from "Create FTS-dictionaries" to "Create
>>> FTS-Configuration".
>>>
>>> Done
>>
>>>
>>> - Tree node is not getting refreshed on name change.
>>>
>>> Done
>>
>>>
>>> - Unable to rename FTS Configuration when name contains any capital
>>> letter.
>>>
>>> Done
>>
>>>
>>> - If user tries to add already existing token then respective row in
>>> the backgrid should be highlighted.
>>>
>>> Done
>>
>>>
>>> - "URL not found" when click on SQL help icon from the properties
>>> dialog.
>>>
>>> Done
>>
>>>
>>> - SQL should not be generated when remove and add the same token
>>> again. Currently it is creating two sql's one for remove and another
>>> is for add.
>>> - *Fix for the above issue is*: - Add "keys: ['token']," to
>>> TokenModel in your js file.
>>>
>>> Done
>>
>>
>>>
>>> On Fri, May 13, 2016 at 2:58 PM, Sanket Mehta <
>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi All,
>>>>
>>>> Last patch for FTS configuration does not contain node.ui.js file
>>>> Kindly ignore it.
>>>>
>>>> Here is the new revised patch attached with this mail.
>>>> Please do review it and let me know if any changes required
>>>>
>>>> Regards,
>>>> Sanket Mehta
>>>> Sr Software engineer
>>>> Enterprisedb
>>>>
>>>> On Thu, May 12, 2016 at 4:38 PM, Sanket Mehta <
>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> PFA the revised patch.
>>>>> Please do review it and let me know if anything is not proper.
>>>>>
>>>>> Regards,
>>>>> Sanket Mehta
>>>>> Sr Software engineer
>>>>> Enterprisedb
>>>>>
>>>>> On Thu, May 5, 2016 at 8:19 PM, Harshal Dhumal <
>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> + patch link
>>>>>>
>>>>>>
>>>>>> http://www.postgresql.org/message-id/CAFiP3vwkka+=1FOJ7kr2zbC4AzecoCa9eO9DZ34-Oyy_9gE92g@mail.gmail.com
>>>>>>
>>>>>> --
>>>>>> *Harshal Dhumal*
>>>>>> *Software Engineer *
>>>>>>
>>>>>>
>>>>>>
>>>>>> EenterpriseDB <http://www.enterprisedb.com>
>>>>>>
>>>>>> On Thu, May 5, 2016 at 8:18 PM, Sanket Mehta <
>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> PFA first patch for FTS configuration node.
>>>>>>>
>>>>>>> It depends upon backgrid select2cell multi select control, for which
>>>>>>> Harshal has sent the patch recently.
>>>>>>> Please do apply his patch first and then apply this patch.
>>>>>>>
>>>>>>> Please do review it and let me know if any changes are required.
>>>>>>>
>>>>>>>
>>>>>>> Regards,
>>>>>>> Sanket Mehta
>>>>>>> Sr Software engineer
>>>>>>> Enterprisedb
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Sent via pgadmin-hackers mailing list (
>>>>>>> pgadmin-hackers(at)postgresql(dot)org)
>>>>>>> To make changes to your subscription:
>>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)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*
>>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>>
>

Attachment Content-Type Size
fts_configurationv5.patch text/x-patch 71.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-18 07:12:57 Re: PATCH: FTS configuration node
Previous Message Surinder Kumar 2016-05-18 05:16:43 [pgAdmin4][Patch]: Added Filemanager library url in libraries.txt