Re: [GSoC][Patch] Automatic Mode Detection V1

Hi,

On Mon, Jun 24, 2019 at 10:13 PM Yosry Muhammad <yosrym93(at)gmail(dot)com> wrote:

> Hi,
>
> Please find attached a patch file with the following updates (last patch +
> updates) attached:
> - Changed the color to $color-gray-lighter and added the shortcut for the
> new button.
> - Added a preferences option to enable/disable prompting on uncommited
> transactions on exiting.
> - Changed call_render_after_poll_specs test to be in sync with code
> changes, also fixed a mix up in the test descriptions in the same file.
> - Fixed a bug with a recent patch 'Allow editing of data where a primary
> key column includes a % sign in the value.' that occurred when the primary
> key was a number.
>
> - After running python and feature tests, changes were made to nearly all
>>> the files (git status shows modifications in a ton of files), is there
>>> something I have done wrong?
>>>
>> What command did you use, can you share the screenshot of the files
>> changed?
>>
>
> I tried it again after a proper test_config.json as you mentioned and
> everything worked fine. All tests pass for this patch except for 3 feature
> tests that all fail because of a TimeoutException related to selenium.
> Please find a log file of the feature tests attached.
>
>
>>
>> - What else is missing from this patch to make it applicable ? I would
>>> like to produce a release-ready patch if possible. If so, I can continue
>>> working on the project on following patches, I just want to know what is
>>> the minimum amount of work needed to make this patch release-ready
>>> (especially that changes are being made in the master branch that require
>>> me to re-edit parts of the code that I have written before to keep things
>>> in-sync).
>>>
>> @Dave Page is the right person to answer this.
>>
>
> Waiting for his reply :D
>
> - For the bug that I reported before (generated queries in Query History
>>> appear in a distorted way for the user), to get the actual query that is
>>> being executed I can use the mogirfy() function of psycopg2 but I need
>>> access to a cursor. I can get one directly in save_changed_data() function
>>> by using conn.conn.cursor() but then I would be bypassing the wrapper
>>> Connection class. Should I modify the wrapper Connection class and add a
>>> function that can provide a cursor (or a wrapper around cursor.mogrify() )?
>>> Thoughts?
>>>
>> Could you please share the query/screenshot ? The query history just
>> stores the SQL text and fetches back to show in CodeMirror. No
>> modifications/generation of queries is done by Query History.
>>
>>>
>>>
> By 'generated queries' I meant the querie that are generated by pgAdmin to
> save changes to the data grid to the database. Here is a screenshot from
> the released version (not the version I am working on).
> [image: pg-query-history-bug.png]
> Scenario:
> - Opened View Data on a table (public.kweek)
> - Modified a cell in a column named media_url with a primary key (id = 50)
> to 'new link'
> - Instead of showing 'new link' in the query %(media_url) is shown.
>
The update queries fired internally should not go to history. Queries fired
by user only should go. That's what I think.

>
> This can be fixed in save_changed_data() function in my patch but I need
> access to a cursor as previously mentioned. Thoughts?
>
> Thanks a lot for your help!
>
>
>
> --
> *Yosry Muhammad Yosry*
>
> Computer Engineering student,
> The Faculty of Engineering,
> Cairo University (2021).
> Class representative of CMP 2021.
> https://www.linkedin.com/in/yosrym93/
>

--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"