Re: [pgAdmin][RM2172] Search Objects Functionality

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Subject: Re: [pgAdmin][RM2172] Search Objects Functionality
Date: 2020-04-06 09:52:17
Message-ID: CAFOhELfufmwyYW_CkCtAxVo-KKf7CHst9187Nq-TrbmYr3HXFw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Aditya,

Please resend the rebased patch, it does not apply.

Thanks,
Khushboo

On Fri, Apr 3, 2020 at 2:44 PM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi Hackers,
>
> Attached is the updated patch.
> With this,
> 1) I've displayed the rows count detail at the bottom of the dialog. This
> will help in both cases, when there are rows and when there are none.
> 2) As discussed, a user can now apply object types dropdown filter on
> already loaded data.
> 3) I've not made changes for the multilevel partition icon because it
> would be too much to do for an icon. We're already showing the type name in
> the grid. Adding extra SQL joins and making the query slower for the icon
> is not desirable.
> 4) Fixed some gettext issues as mentioned in the review.
>
> Please review.
>
>
> On Thu, Apr 2, 2020 at 5:54 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Khushboo,
>>
>> On Thu, Apr 2, 2020 at 4:49 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Thu, Apr 2, 2020 at 4:30 PM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Khushboo,
>>>>
>>>> Thank you for reviewing.
>>>>
>>>>
>>>> On Thu, Apr 2, 2020 at 4:09 PM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Aditya,
>>>>>
>>>>> Review comments:
>>>>>
>>>>> *UI:*
>>>>>
>>>>> 1. When no object is found, the default message should be given,
>>>>> currently no message displayed.
>>>>> 2. Can we have a tooltip on the row "Double click to locate the object
>>>>> in the browser" ?
>>>>> 3. Full stop is missing in the message column objects are disabled in
>>>>> the browser. You can enable them in the preferences dialog ( :D )
>>>>> and also, we should start the statement with the capital letter.
>>>>> 4. If possible, use the multilevel partition table symbol same as the
>>>>> browser tree.
>>>>> 5. gettext is missing from the search grid header.
>>>>>
>>>> I'll fix all above.
>>>>
>>>>> 6. Suggestion: The search button should be at the end (after type
>>>>> combobox). The current position of the controls suggest that search for
>>>>> the objects and then filter it out but that's not the case.
>>>>>
>>>> I've actually kept the most frequently used controls together. The
>>>> probability of using the types filter is less and a user would generally go
>>>> for full search. This is how even we generally do. We search first and then
>>>> apply filter if required
>>>>
>>> Right, so type based search on slickgrid data would be useful.
>>>
>> 👍
>>
>>> After changing the type, we have to click on the search button.
>>>>> In the current positioning, we should fetch all the records from the
>>>>> backend and then filter those out depending on the type at the client side
>>>>> only, so that will reduce the server requests and slickgrid is efficient it
>>>>> do so.
>>>>>
>>>> I'll look into this. My only concern is the data may be outdated, but I
>>>> agree to filter in slickgrid on type change. The user can hit search again
>>>> if required.
>>>>
>>>
>>>>> *Backend:*
>>>>>
>>>>> 1. We do have the list of blueprint, so we can use that list instead
>>>>> of taking the hard coe list in the init method of SearchObjectsHelper
>>>>> class.
>>>>>
>>>> The reason is, we do not support all objects for search objects. Only
>>>> objects under a database are supported. The probability of node type change
>>>> is very less.
>>>>
>>> True but we can maintain the skip list (which would be less) and we do
>>> have bluprint start with NODE, so it will be easier to fetch.
>>>
>> I would prefer the "in" list rather than "skip" list. Each time a new
>> node is added to pgAdmin, we will have to update the skip list in search
>> objects. With the "in" list, search objects has better control.
>>
>>> 2. While searching the object, we create an object of SearchObjectsHelper
>>>>> on each request. We can create it once while initializing and utilize it on
>>>>> every search.
>>>>>
>>>> The intention is to keep SearchObjectsHelper stateless. The object is
>>>> created based on the request data and it is easier to maintain
>>>> independently.
>>>>
>>>>>
>>>>> Note: The functionality is working fine.
>>>>>
>>>> Great. Thanks.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Apr 2, 2020 at 9:31 AM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Khushboo,
>>>>>>>
>>>>>>> Can you please review it.
>>>>>>>
>>>>>> I am on it.
>>>>>>
>>>>>>>
>>>>>>> On Mon, Mar 30, 2020 at 2:39 PM Aditya Toshniwal <
>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Hackers,
>>>>>>>>
>>>>>>>> Attached is the patch to implement search objects functionality in
>>>>>>>> pgadmin.
>>>>>>>> The feature will allow a user to search for any object in a
>>>>>>>> database.
>>>>>>>> Highlights of the feature:
>>>>>>>> 1) Search any object with user input text with at least 3
>>>>>>>> characters.
>>>>>>>> 2) Search can be done on a specific object type by selecting from
>>>>>>>> the types dropdown.
>>>>>>>> 3) The search results grid will show object name, object type and
>>>>>>>> the object path on the browser tree. On double clicking the record, it will
>>>>>>>> locate that object on the browser tree. The columns object name and type
>>>>>>>> are sortable.
>>>>>>>> 4) The object nodes which are disabled (hidden) using preferences
>>>>>>>> will not be visible in the types dropdown. However, in the case of all
>>>>>>>> types, the search records will be visible for those types greyed out.
>>>>>>>> 5) You can also access search objects dialog using the button on
>>>>>>>> the browser toolbar.
>>>>>>>>
>>>>>>>> Python and JS test cases added. Docs updated.
>>>>>>>> Please review.
>>>>>>>>
>>>>>>>> --
>>>>>>>> Thanks and Regards,
>>>>>>>> Aditya Toshniwal
>>>>>>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Thanks & Regards*
>>>>>>> *Akshay Joshi*
>>>>>>>
>>>>>>> *Sr. Software Architect*
>>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>
>>>>>>
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-04-06 09:56:41 Re: [pgAdmin][RM4206]: Grant wizard does not close when we press ESC key
Previous Message Pradip Parkale 2020-04-06 08:50:36 [pgAdmin][RM4206]: Grant wizard does not close when we press ESC key