Re: [GSoC] Query History Upgrade

2019-08-16 Thread Yosry Muhammad
Thanks all for the thorough review. On Fri, Aug 16, 2019, 1:48 PM Akshay Joshi wrote: > Thanks patch applied. > > On Wed, Aug 14, 2019 at 7:24 PM Yosry Muhammad wrote: > >> Hi, >> >> On Wed, Aug 14, 2019 at 3:50 PM Yosry Muhammad >> wrote: >> >>> Please find an updated patch with the mentioned

Re: [GSoC] Query History Upgrade

2019-08-16 Thread Akshay Joshi
Thanks patch applied. On Wed, Aug 14, 2019 at 7:24 PM Yosry Muhammad wrote: > Hi, > > On Wed, Aug 14, 2019 at 3:50 PM Yosry Muhammad wrote: > >> Please find an updated patch with the mentioned issue fixed. I am sorry >> you spent so much time reviewing this and finding bugs. >> >> On Wed, Aug 1

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Yosry Muhammad
Hi, On Wed, Aug 14, 2019 at 3:50 PM Yosry Muhammad wrote: > Please find an updated patch with the mentioned issue fixed. I am sorry > you spent so much time reviewing this and finding bugs. > > On Wed, Aug 14, 2019 at 2:34 PM Akshay Joshi < > akshay.jo...@enterprisedb.com> wrote: > >> Hi Yosry >

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Akshay Joshi
Hi Yosry I am still facing following issues: - No icons for already saved(before applying your patch) query in query history. Is this expected? - Internal queries are visible even if toggle switch is set to "No". Following are the steps to reproduce: - Create a table with primar

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Akshay Joshi
On Wed, Aug 14, 2019 at 3:53 PM Yosry Muhammad wrote: > Hi, > > On Wed, Aug 14, 2019 at 12:03 PM Akshay Joshi < > akshay.jo...@enterprisedb.com> wrote: > >> Hi Yosry >> >> I have found following issues: >> >>- Jasmine test cases are failing. >>- Browser error when applying the patch and o

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Yosry Muhammad
Hi, On Wed, Aug 14, 2019 at 12:03 PM Akshay Joshi wrote: > Hi Yosry > > I have found following issues: > >- Jasmine test cases are failing. >- Browser error when applying the patch and open the query tool. >Previously saved queries are there: >- [image: Screenshot 2019-08-14 at 3

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Akshay Joshi
Hi Yosry I have found following issues: - Jasmine test cases are failing. - Browser error when applying the patch and open the query tool. Previously saved queries are there: - [image: Screenshot 2019-08-14 at 3.21.21 PM.png] - Toggle Switch should have text "Yes/No" as we already

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Yosry Muhammad
Great! thanks everyone. On Wed, Aug 14, 2019, 10:08 AM Akshay Joshi wrote: > Hi Yosry > > > On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad wrote: > >> Any more suggestions or comments on the patch? >> > > I am reviewing it. > >> >> On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal < >> adity

Re: [GSoC] Query History Upgrade

2019-08-14 Thread Akshay Joshi
Hi Yosry On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad wrote: > Any more suggestions or comments on the patch? > I am reviewing it. > > On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal < > aditya.toshni...@enterprisedb.com> wrote: > >> >> >> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad

Re: [GSoC] Query History Upgrade

2019-08-13 Thread Yosry Muhammad
Any more suggestions or comments on the patch? On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote: > > > On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad wrote: > >> Please find attached an updated patch with a small modification to the >> feature test. Thi

Re: [GSoC] Query History Upgrade

2019-08-13 Thread Aditya Toshniwal
On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad wrote: > Please find attached an updated patch with a small modification to the > feature test. This makes sure the first history element is selected and > focused before trying to move through entries using the down arrow key. > When the test fails,

Re: [GSoC] Query History Upgrade

2019-08-13 Thread Yosry Muhammad
Is it always the same error when it fails? On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote: > The test cases fails intermittently. It passes sometimes. > > On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad wrote: > >> Hi Aditya, >> >> The test passes on m

Re: [GSoC] Query History Upgrade

2019-08-13 Thread Aditya Toshniwal
The test cases fails intermittently. It passes sometimes. On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad wrote: > Hi Aditya, > > The test passes on my computer, is there anything I could try to > reproduced the failure? > > By looking at the error, I suspect clicking the down arrow key on your >

Re: [GSoC] Query History Upgrade

2019-08-13 Thread Yosry Muhammad
Hi Aditya, The test passes on my computer, is there anything I could try to reproduced the failure? By looking at the error, I suspect clicking the down arrow key on your machine did not move to the next history entry during the test. Does clicking the down arrow normally go to the next history e

Re: [GSoC] Query History Upgrade

2019-08-13 Thread Aditya Toshniwal
Hi Yosry, Everything looks fine to me. Except intermediate feature test failure. May be @committers can try on their machine. == FAIL: runTest (pgadmin.feature_tests.query_tool_journey_test.QueryToolJourneyTest) Tests the path thr

Re: [GSoC] Query History Upgrade

2019-08-12 Thread Aditya Toshniwal
Hi Yosry, On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad wrote: > Hi Aditya, > > On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal < > aditya.toshni...@enterprisedb.com> wrote: > >> Hi Yosry, >> >> Nice work there. It seems to be working fine except a few suggestions: >> 1) Fix pep8 issues >> 2)

Re: [GSoC] Query History Upgrade

2019-08-12 Thread Aditya Toshniwal
Hi Yosry, Nice work there. It seems to be working fine except a few suggestions: 1) Fix pep8 issues 2) DOM Statements like below can be avoided and html can be added directly to main template of $el instead of adding extra operations of find, prepend and append. Plus, it makes it difficult to unde