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

Then that's browser specific issue, please create one redmine ticket for the issue.​

On Fri, Mar 30, 2018 at 11:36 PM, Rahul Soshte <rahulsoshte360@gmail.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@gmail.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.zabuawala@enterprisedb.com> wrote:

++ Attaching screenshot

On Fri, Mar 30, 2018 at 7:06 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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@gmail.com> wrote:

Hi,
I tried writing tests in the web/pgadmin/tools/sqleditor/utils/tests/test_save_query_to_file_utils

for the file web/pgadmin/tools/sqleditor/utils/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/sqleditor/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/sqleditor/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.zabuawala@enterprisedb.com> wrote:

On Thu, Mar 29, 2018 at 11:45 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.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@gmail.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)