Re: [pgAdmin4][runtime]: Download feature in runtime

Hi Dave,

On Thu, Jun 30, 2016 at 7:31 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Thu, Jun 30, 2016 at 10:42 AM, Neel Patel
> <neel(dot)patel(at)enterprisedb(dot)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
>