Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Export accounts as JSON or CSV #2866

Merged
merged 6 commits into from
Nov 16, 2016
Merged

Export accounts as JSON or CSV #2866

merged 6 commits into from
Nov 16, 2016

Conversation

ngotchac
Copy link
Contributor

Closes #2147

Enables the user to export its Accounts to CSV or JSON files.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M5-ui labels Oct 25, 2016
@jacogr
Copy link
Contributor

jacogr commented Oct 25, 2016

Couple of comments -

  1. Would probably just have the .json option since it should be more 'portable' between implementations
  2. We need a restore option as well when we have the backup
  3. Are we sure this is not better suited backed with personal_backupAccounts & personal_restoreAccounts APIs? @gavofyork ?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 86.133% when pulling 61f41cd on ng-accounts-backup into 2d2e9c4 on master.

@gavofyork
Copy link
Contributor

what would those APIs do? i think it's probably fine to do it in the browser from what i can see

@derhuerst
Copy link
Contributor

Would probably just have the .json option since it should be more 'portable' between implementations

👍 JSON feels a lot more robust than CSV. Beginners won't care what format it is and advanced users may fiddle with both.

@ngotchac
Copy link
Contributor Author

I guess you're right about the CSV file... I was just thinking of users who uses Excel a lot who wants to copy/paste easily a vector of addresses...

@ngotchac
Copy link
Contributor Author

Shouldn't the import be in another issue ? It needs more work (nice file upload UI at least), and would only be available for the Address Book without full export of accounts (keys)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 86.115% when pulling 1b0a87c on ng-accounts-backup into 2d2e9c4 on master.

@ngotchac
Copy link
Contributor Author

Import logged here : #2885

}
}

handleExport = () => {
Copy link
Contributor

@derhuerst derhuerst Oct 26, 2016

Choose a reason for hiding this comment

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

is this abstraction worth it? also getExtension feels like we're not going to need it for now.

? content
: JSON.stringify(content, null, 4);
const text = this.contentAsString(content, filetype);
const extension = this.getExtension(filetype);

const blob = new Blob([ text ], { type: 'text/plain;charset=utf-8' });
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the mime type corresponding to filetype be passed as type?

@ngotchac
Copy link
Contributor Author

@derhuerst Comments fixed!

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6b2cffd on ng-accounts-backup into * on master*.

@derhuerst derhuerst added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 26, 2016
@jacogr
Copy link
Contributor

jacogr commented Oct 26, 2016

Happy to have imports seperated, just would like to keep this back a bit until that is ready - since it will be first thing people ask for.

@gavofyork gavofyork added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Oct 27, 2016
@jacogr jacogr added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Oct 27, 2016
@gavofyork
Copy link
Contributor

are we doing import functionality too or is this going to stay onice forever?

@ngotchac
Copy link
Contributor Author

ngotchac commented Nov 9, 2016

Yep ; should be easy after #3279 gets merged

@gavofyork
Copy link
Contributor

was merged 5 days ago... :-)

@ngotchac
Copy link
Contributor Author

ngotchac commented Nov 14, 2016

#3433

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Nov 16, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 16, 2016
@gavofyork gavofyork merged commit b9e9544 into master Nov 16, 2016
@gavofyork gavofyork deleted the ng-accounts-backup branch November 16, 2016 03:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants