#2 Add support for downloading account data

Samengevoegd
jonathan heeft 22 commits samengevoegd van kris/download_account naar master 4 jaren geleden
kris commented 4 jaren geleden
Er is nog geen inhoud.
jonathan was toegekend door kris 4 jaren geleden
jonathan commented 4 jaren geleden
Eigenaar

Note -- You might also need to make some changes to the ActionPage.vue to properly handle approvals for the account download.

Note -- You might also need to make some changes to the `ActionPage.vue` to properly handle approvals for the account download.
kris titel aangepast van WIP: (not finished) Add support for downloading account data naar Add support for downloading account data 4 jaren geleden
kris commented 4 jaren geleden
Poster

Note: before updating this lib’s refence in the main bubble repo, please review and merge corresponding PR within that one ( bubblev/bubble#8 ) as there are some dependencies (English labels) there.

Note: before updating this lib's refence in the main bubble repo, please review and merge corresponding PR within that one ( https://git.bubblev.org/bubblev/bubble/pulls/8 ) as there are some dependencies (English labels) there.
jonathan aangevraagde wijzigingen 4 jaren geleden
jonathan heeft een reactie achtergelaten

I realized I hadn’t submitted my previous review, sorry.

Overall this looks very good, just a few renaming changes and other minor things requested.

src/_services/user.service.js
@@ -12,6 +12,7 @@ export const userService = {
register,
searchAccounts,
getMe,
requireAccountDownload,
jonathan commented 4 jaren geleden

I would call this requestAccountDownload

I would call this `requestAccountDownload`
kris commented 4 jaren geleden

That was intentionally as some elements in code have Request as suffix, so I tried to make it different. Any other term to use here, or should I just change it to request? demand?
https://www.google.com/search?client=ubuntu&channel=fs&q=sysnonym+request&ie=utf-8&oe=utf-8

That was intentionally as some elements in code have `Request` as suffix, so I tried to make it different. Any other term to use here, or should I just change it to `request`? `demand`? https://www.google.com/search?client=ubuntu&channel=fs&q=sysnonym+request&ie=utf-8&oe=utf-8
jonathan commented 4 jaren geleden

Yeah just changed to request. It’s OK if the prefix and suffix are both request, they have different semantics.

Yeah just changed to `request`. It's OK if the prefix and suffix are both `request`, they have different semantics.
src/_store/account.module.js
@@ -181,1 +183,4 @@
);
},
requireAccountDownload({ commit }, {messages, errors}) {
commit('requiringAccountDownloadRequest');
jonathan commented 4 jaren geleden

the convention is for the commit methods to be named methodNameRequest, methodNameSuccess and methodNameFailure, so these should be requestAccountDownloadRequest, requestAccountDownloadSuccess and requestAccountDownloadFailure

the convention is for the commit methods to be named `methodNameRequest`, `methodNameSuccess` and `methodNameFailure`, so these should be `requestAccountDownloadRequest`, `requestAccountDownloadSuccess` and `requestAccountDownloadFailure`
kris commented 4 jaren geleden

Yes, that’s the one - request...Request. Do we need another haters will hate like comment here? :)

Yes, that's the one - `request...Request`. Do we need another `haters will hate` like comment here? :)
jonathan commented 4 jaren geleden

Yeah just changed to request. It’s OK if the prefix and suffix are both request, they have different semantics.

Yeah just changed to `request`. It's OK if the prefix and suffix are both `request`, they have different semantics.
src/account/profile/PolicyPage.vue
@@ -519,6 +530,11 @@
errors: this.errors
});
},
downloadAccount() {
jonathan commented 4 jaren geleden

since this isn’t actually downloading the account data, but requesting an account data download, perhaps call this method initiateAccountDownloadRequest or something like that

since this isn't actually downloading the account data, but requesting an account data download, perhaps call this method `initiateAccountDownloadRequest` or something like that
kris commented 4 jaren geleden

This one was already changed to clickRequireAccountDownload. Is that ok?

This one was already changed to `clickRequireAccountDownload`. Is that ok?
kris commented 4 jaren geleden
Poster

@jonathan please check my answers here and confirm what to do...

@jonathan please check my answers here and confirm what to do...
jonathan heeft deze pull request gesloten 4 jaren geleden
jonathan heeft 4 jaren geleden de branch kris/download_account verwijderd.

Reviewers

jonathan aangevraagde wijzigingen 4 jaren geleden
The pull request has been merged as 8582fc2745.
Log in om deel te nemen aan deze discussie.
Geen beoordelaars
Geen label
Geen mijlpaal
Niet toegewezen
2 deelnemers
Notificaties
Vervaldatum

Geen vervaldatum ingesteld.

Afhankelijkheden

Deze pull-aanvraag heeft momenteel geen afhankelijkheden.

Laden…
Er is nog geen inhoud.