Re: [pgAdmin4][runtime]: Download feature in runtime - Mailing list pgadmin-hackers

From Neel Patel
Subject Re: [pgAdmin4][runtime]: Download feature in runtime
Date
Msg-id CACCA4P2bEpUNZhkGQLY1JnMiBChFhauLpqiuAgJQiYcTV6YZ7g@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin4][runtime]: Download feature in runtime  (Dave Page <dpage@pgadmin.org>)
Responses Re: [pgAdmin4][runtime]: Download feature in runtime  (Dave Page <dpage@pgadmin.org>)
List pgadmin-hackers
Hi Dave,

On Thu, Jun 30, 2016 at 7:31 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Jun 30, 2016 at 10:42 AM, Neel Patel
<neel.patel@enterprisedb.com> wrote:
> Hi,
>
> Please find attached patch file for initial version of download file in
> runtime application.

I've attached an update with some improved messages, and setting the
progress dialogue to be modal (seeing as we cannot have multiple
downloads, and it's easy to lose the dialogue).

> With this patch, we have implemented two features.
>
> Feature 1 :- Normal "Download file" from runtime application
>
> Previously "Download file" was not implemented in runtime application.
> With this patch file, we have handled Qt signal for download file properly.

This seems to work fine. I did get one crash (after I cancelled a
download, then tried it again), but I couldn't reproduce that.

Okay. I will try to reproduce the issue and also i will try to review the code again if i can find something regrading crash.
 

> Feature 2 :-   "download" attribute support for 'a' tag for client side
> download
>
> As per our knowledge, webkit has not implemented the download attribute at
> 'a' tag.
> Currently it shows under development from below link.
>
> https://bugreports.qt.io/browse/QTBUG-47732
>
> We did not found any signal in Qt for download attribute feature but to
> implement this feature in runtime application, we added one workaround to
> make it work with download CSV file.
>
> When we click on download buttons, we are getting Qt signal "urlLinkClicked"
> and in that url we are finding "data:text/csv" from encoded URL generated
> from sqleditor. Once we found that tag then we are decoding the csv data and
> writing to file.
>
> Is that right approach ? Should we add our own custom mime-type to header ?
> Let us know your thoughts on this feature.

This doesn't work so well, for a number of reasons:

1) QT Creator is complaining that your regexp contains an invalid
escape sequence (line 546).
 
I will fix.

2) The default file name seems to be the entire data blob. I would
suggest making the file name "download.csv" if we don't know anything
better. The "csv" part should be taken from the mime type (see below)

3) Should we handle all "data:" downloads in this way? Taking the file
type and default extension from the mimetype offered.

We can handle all "data:" download. We will extract the filename and extension from mime type.
As i know, Qt provides QUrlQuery class which will be useful to find the key value pair. I will test and let you know.

e.g. If we have header as below
        "data:text/csv;charset=utf-8;Content-disposition:attachment;filename=download.csv;" 

From the QurlQuery class we can query "filename" and "data:" and accordingly save the data to filename provided.

Please suggest.


4) When I change the filename the data is properly saved, but then I
get a confirmation message that still has the full data blob as the
filename.

5) It somehow seems to have let me save files with forward slashes in
the name. See attachment.
 
I think we should not ask for "Save as" dialog. If there is no key found of "filename" in encodedURI then we should create the file "download.csv" in user's download directory and save the csv data. 

6) I get all sorts of weird redrawing on the screen when downloading a
data blob. I suspect it's because the filename (which is still the
entire data blob) is shown on the progress dialogue.


I will try to fix as per above comments and submit the patch again. 
Let us know for any misunderstanding.

 
Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgadmin-hackers by date:

Previous
From: Dave Page
Date:
Subject: pgAdmin 4 commit: Beta 2.1 - a favour for our friends in QA.
Next
From: Harshal Dhumal
Date:
Subject: fix for issue rm1425 [pgAdmin4]