Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zip multiple exported QIF files into one for all export destinations #714

Merged
merged 3 commits into from
Aug 2, 2017

Conversation

rivaldi8
Copy link
Collaborator

@rivaldi8 rivaldi8 commented Aug 2, 2017

Fixes #696

When having transactions with multiple commodities, a QIF file for each
commodity is created. With Android's Storage Access Framework we are
limited to one file, so now we always pack QIFs into a single ZIP file.

Fixes codinguser#696
As with Storage Access Framework we are limited to one file, we checked
if there were multiple files to export and packed them into a ZIP file.
This only happened when exporting to QIF, which now packs multiple files
into a ZIP. Other export formats were already generating a single file.
So now we just copy the exported file.
@rivaldi8
Copy link
Collaborator Author

rivaldi8 commented Aug 2, 2017

I forgot to run the tests and just found some got broken. I'll look into it.

It may happen when there aren't transactions to export.
@rivaldi8
Copy link
Collaborator Author

rivaldi8 commented Aug 2, 2017

Fixed!

Copy link
Owner

@codinguser codinguser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me. Thanks!

@codinguser codinguser merged commit 011440c into codinguser:develop Aug 2, 2017
@rivaldi8 rivaldi8 deleted the bug-696-zip-qif branch August 2, 2017 21:52
@@ -283,25 +283,10 @@ private void moveExportToUri() throws Exporter.ExporterException {
if (mExportedFiles.size() > 0){
try {
OutputStream outputStream = mContext.getContentResolver().openOutputStream(exportUri);
ZipOutputStream zipOutputStream = new ZipOutputStream(outputStream);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rivaldi8 While going through the commits to create a CHANGELOG for the next release, I revisited this issue.
I think we should zip the file which we export (even if it is just a single one), because it saves bandwidth for user sync. Also, some users have really large databases which lead to large XML outputs (always exports the whole database).

One way to remedy this would be to make the GncXmlExporter always exports zipped files (without the .zip extension, only .gnca - .gnca will then be considered a zipped format). Then we do not need to worry about it here (or anywhere else).

GnuCash desktop can handle zipped XML files for export, as can our app. It will not be directly user-readable, but then again, XML is not really meant to be user-readable. And those who want to read it can just unzip it once.

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codinguser Yes, I agree with you, but I think you really mean gzipped (GZIPOutputStream, like here) instead of zipped (ZipOutputStream). That's the compression algorithm used in .gnca files, isn't it?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rivaldi8 yes, I meant gzipped, sorry :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants