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

From: Rahul Soshte <rahulsoshte360(at)gmail(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>
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-04-10 22:49:22
Message-ID: CAKyzeV3ciT_L9OoOG8XcxQPNsCokvwgduhGM0Cha0FtXW7damA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

I am trying to access the value in the format combobox for selected file
type to put it here..

This snippet happens to be in
web/pgadmin/tools/sqleditor/static/js/sqleditor.js

How do I get access to that format combobox value to determine whether it
is 'sql' or 'All Files' ?
Also What is e here ?

This passed as to sqleditor's save_file method as seen from the code here.

This snippet happens to be in web/pgadmin/tools/sqleditor/__init__.py

On Sat, Mar 31, 2018 at 12:05 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

>
> 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(dot)zabuawala(at)enterprisedb(dot)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 Fahar Abbas 2018-04-11 04:51:59 Re: pgAdmin 3.0 builds
Previous Message Rahul Soshte 2018-04-10 19:50:57 Ignoring web/yarn-error.log and runtime/moc_LogWindow.cpp