#2 Add support for downloading account data

Merge aplicado
jonathan aplicou merge dos 22 commits de kris/download_account em master 4 anos atrás
kris comentou 4 anos atrás
Ainda não há conteúdo.
jonathan foi atribuído por kris 4 anos atrás
jonathan comentou 4 anos atrás
Proprietário

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 alterou o título de WIP: (not finished) Add support for downloading account data para Add support for downloading account data 4 anos atrás
kris comentou 4 anos atrás
Autor

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 alterações solicitadas 4 anos atrás
jonathan left a comment

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 comentou 4 anos atrás

I would call this requestAccountDownload

I would call this `requestAccountDownload`
kris comentou 4 anos atrás

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 comentou 4 anos atrás

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 comentou 4 anos atrás

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 comentou 4 anos atrás

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 comentou 4 anos atrás

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 comentou 4 anos atrás

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 comentou 4 anos atrás

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

This one was already changed to `clickRequireAccountDownload`. Is that ok?
kris comentou 4 anos atrás
Autor

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

@jonathan please check my answers here and confirm what to do...
jonathan closed this pull request 4 anos atrás
jonathan excluiu branch kris/download_account 4 anos atrás

Revisores

jonathan alterações solicitadas 4 anos atrás
O pull request teve merge aplicado como 8582fc2745.
Acesse para participar desta conversação.
No reviewers
Sem etiqueta
Sem marco
Sem responsável
2 participante(s)
Notificações
Data limite

Data limite não informada.

Dependências

Atualmente este pull request não tem dependências.

Carregando…
Ainda não há conteúdo.