@tsteur opened this Pull Request on October 14th 2018 Member

fix #13586

@diosmosis commented on October 15th 2018 Member

Should this also remove the suffix from the filename? Could be possible w/ a Content-Disposition header I think.

@tsteur commented on October 15th 2018 Member

Not sure what you mean?

@diosmosis commented on October 15th 2018 Member

The issues says "which would be removed when the file is being downloaded via the API". I assumed that there needed to be some code to remove the extra string, maybe that's not needed? I haven't tested locally, will do that.

@tsteur commented on October 15th 2018 Member

Not sure what you mean. The file wouldn't be downloaded, it would be persisted on the file system and a user can access the report through the returned output path at any time...

We may remove the option to save to disk from the API maybe? @mattab ... this option would be only needed for sendReport()

@mattab commented on October 15th 2018 Member

I can't remember why we need to store reports on disk? but if we can get rid of that, it would be best :+1:

@diosmosis commented on October 15th 2018 Member

The file wouldn't be downloaded, it would be persisted on the file system and a user can access the report through the returned output path at any time...

I see so there's no API method like downloadReport().

@tsteur commented on October 15th 2018 Member

Done.

I see so there's no API method like downloadReport().

Nope there isn't, there is only a direct download option for which the option "save on disk" is not needed.

@diosmosis commented on November 28th 2018 Member

Looks like the MobileMessagingTest needs an update, otherwise good to merge

@tsteur commented on November 29th 2018 Member

Fixed the test 👍

This Pull Request was closed on November 29th 2018
Powered by GitHub Issue Mirror