Thanks, patch applied. Fixed some minor issues.
On Mon, Apr 6, 2020 at 3:52 PM Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote: > Hi Hackers, > > Please find the attached rebased patch. > > On Mon, Apr 6, 2020 at 3:22 PM Khushboo Vashi < > khushboo.va...@enterprisedb.com> wrote: > >> 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.toshni...@enterprisedb.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.toshni...@enterprisedb.com> wrote: >>> >>>> Hi Khushboo, >>>> >>>> On Thu, Apr 2, 2020 at 4:49 PM Khushboo Vashi < >>>> khushboo.va...@enterprisedb.com> wrote: >>>> >>>>> >>>>> >>>>> On Thu, Apr 2, 2020 at 4:30 PM Aditya Toshniwal < >>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>> >>>>>> Hi Khushboo, >>>>>> >>>>>> Thank you for reviewing. >>>>>> >>>>>> >>>>>> On Thu, Apr 2, 2020 at 4:09 PM Khushboo Vashi < >>>>>> khushboo.va...@enterprisedb.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.va...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi < >>>>>>>> akshay.jo...@enterprisedb.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.toshni...@enterprisedb.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" >>> >> > > -- > 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*
