Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Rahul Soshte <rahulsoshte360(at)gmail(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature
Date: 2018-03-31 06:35:48
Message-ID: CAKKotZRcx4ZcQ2+gfa3zLf4nH12DHCbZOi2P+iM3bewP_38hGA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Then that's browser specific issue, please create one redmine ticket
<https://redmine.postgresql.org/projects/pgadmin4> for the issue.​

On Fri, Mar 30, 2018 at 11:36 PM, Rahul Soshte <rahulsoshte360(at)gmail(dot)com>
wrote:

> Yeah the code is present.I have attached the screenshot.
> Also I have noticed that the format combobox appears clearly in my
> Vivaldi Browser but it is not seen in my Firefox Browser.
>
> On Fri, Mar 30, 2018 at 9:59 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> ​I don't think so, Could you inspect html/css code on 'Save as' dialog
>> within your browser window and see if it's present or not?​
>>
>>
>> On Fri, Mar 30, 2018 at 8:30 PM, Rahul Soshte <rahulsoshte360(at)gmail(dot)com>
>> wrote:
>>
>>> Hi,
>>> I don't know why that combobox is not seen in my environment.I am using
>>> Ubuntu 17.10.I have attached the screenshot.
>>> Is this a bug?
>>>
>>>
>>>
>>> On Fri, Mar 30, 2018 at 7:07 PM, Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>
>>>> ++ Attaching screenshot
>>>>
>>>> On Fri, Mar 30, 2018 at 7:06 PM, Murtuza Zabuawala <
>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Rahul,
>>>>>
>>>>> When I said .sql extension, I meant selected sql option in 'Format'
>>>>> combobox (check the screenshot I've attached)
>>>>>
>>>>> For the error you've mentioned you can create Fake application context.
>>>>> Ref: ../web/pgadmin/dashboard/tests/test_dashboard_templates.py +274
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Murtuza Zabuawala
>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>>
>>>>> On Fri, Mar 30, 2018 at 6:36 PM, Rahul Soshte <
>>>>> rahulsoshte360(at)gmail(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>> I tried writing tests in the web/pgadmin/tools/sqleditor/ut
>>>>>> ils/tests/test_save_query_to_file_utils
>>>>>> for the file web/pgadmin/tools/sqleditor/ut
>>>>>> ils/tests/save_query_to_file_utils.py
>>>>>>
>>>>>> But I am getting a error,
>>>>>>
>>>>>> ERROR: runTest (pgadmin.tools.sqleditor.utils
>>>>>> .tests.test_save_query_to_file_utils.TestSaveQueryToFile)
>>>>>> When user has entered the extension .sql to the file while saving
>>>>>> ------------------------------------------------------------
>>>>>> ----------
>>>>>> Traceback (most recent call last):
>>>>>> File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/tools/sqledito
>>>>>> r/utils/tests/test_save_query_to_file_utils.py", line 42, in runTest
>>>>>> file_path_result = save_query_to_file(self.file_data)
>>>>>> File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/tools/sqledito
>>>>>> r/utils/save_query_to_file_utils.py", line 15, in save_query_to_file
>>>>>> storage_manager_path = get_storage_directory()
>>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flask_login.py",
>>>>>> line 788, in decorated_view
>>>>>> if current_app.login_manager._login_disabled:
>>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeug/local.py",
>>>>>> line 338, in __getattr__
>>>>>> return getattr(self._get_current_object(), name)
>>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeug/local.py",
>>>>>> line 297, in _get_current_object
>>>>>> return self.__local()
>>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flask/globals.py",
>>>>>> line 51, in _find_app
>>>>>> raise RuntimeError(_app_ctx_err_msg)
>>>>>> RuntimeError: Working outside of application context.
>>>>>>
>>>>>> How do I test the extracted code inside context? How do I resolve
>>>>>> this error.
>>>>>> I have attached test_save_query_to_file_utils.py
>>>>>> and save_query_to_file_utils.py
>>>>>>
>>>>>> Murtuza, Actually I didnt find any toggable button in the File Dialog
>>>>>> Box So I made it general purpose ( I guess I will have to make one then and
>>>>>> then if I select SQL all .sql files should be listed, and if I select All
>>>>>> files then every type of file is shown in the File Dialog Box,this will be
>>>>>> a new feature, wouldnt it ? )
>>>>>>
>>>>>>
>>>>>> On Fri, Mar 30, 2018 at 4:10 PM, Murtuza Zabuawala <
>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Mar 29, 2018 at 11:45 PM, Joao De Almeida Pereira <
>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>
>>>>>>>> Hi Rahul,
>>>>>>>> I see you extracted some code, that is a pretty good move :D
>>>>>>>>
>>>>>>>> We run the patch through the testing pipeline and everything is
>>>>>>>> green GJ :D
>>>>>>>> Also tested the functionality by hand and looks like it is working
>>>>>>>> except for "add the .sql extension when format is set to SQL." if
>>>>>>>> you set it to All Files the extension is also added. Not sure if this is a
>>>>>>>> big deal or not. Lets see what other people think.
>>>>>>>>
>>>>>>> ​Yes, I also think it should append .sql only if the sql extension
>>>>>>> is selected and user has not provided extension.​
>>>>>>>
>>>>>>> ​Let say If I want to save the file with .txt extension then I can
>>>>>>> use All Files. ​
>>>>>>>
>>>>>>>
>>>>>>>> Codewise here are some of my comments:
>>>>>>>> . You added the yarn-error.log file and a migration to the patch
>>>>>>>> doesn't look intentional. Can you please remove them?
>>>>>>>> . Also in the patch there are 2 file (moc_LogWindow.cpp and
>>>>>>>> ui_LogWindow.h) that look like they do not belong to the patch (Did you
>>>>>>>> rebase your branch before trying to create the patch?
>>>>>>>>
>>>>>>>> The test file: test_save_query_to_file.py is empty, it is missing
>>>>>>>> some tests there.
>>>>>>>>
>>>>>>>> As a convention we user lower case names for functions and
>>>>>>>> UpperCase for class
>>>>>>>>
>>>>>>>> Please, regenerate the patch following my previous comments.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Joao
>>>>>>>>
>>>>>>>> On Thu, Mar 29, 2018 at 12:54 PM Rahul Soshte <
>>>>>>>> rahulsoshte360(at)gmail(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>> When using save or save as feature if .sql is not provided this
>>>>>>>>> Patch appends it.
>>>>>>>>> as clearly mentioned in this link.
>>>>>>>>>
>>>>>>>>> https://redmine.postgresql.org/issues/1998
>>>>>>>>>
>>>>>>>>> I have ran pep8,regression and Jasmine tests too.
>>>>>>>>>
>>>>>>>>> I have primarily changed these files
>>>>>>>>> web/pgadmin/tools/sqleditor/__init__.py
>>>>>>>>> web/pgadmin/tools/sqleditor/static/js/sqleditor.js
>>>>>>>>> web/pgadmin/tools/sqleditor/utils/save_query_to_file.py
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Rahul Soshte (Hunter)
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2018-03-31 06:53:46 Re: [pgAdmin4][RM#3154] Update modules to latest version
Previous Message Rahul Soshte 2018-03-30 18:06:42 Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature