#2 Add support for downloading account data

Sammanfogat
jonathan sammanfogade 22 incheckningar från kris/download_account in i master 4 år sedan
kris kommenterad 4 år sedan
Det finns inget innehåll än.
jonathan blev tilldelad denna av kris 4 år sedan
jonathan kommenterad 4 år sedan
Ägare

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 ändrade titeln från WIP: (not finished) Add support for downloading account data till Add support for downloading account data 4 år sedan
kris kommenterad 4 år sedan
Skapare

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 begärda ändringar 4 år sedan
jonathan lämnade en kommentar

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 kommenterad 4 år sedan

I would call this requestAccountDownload

I would call this `requestAccountDownload`
kris kommenterad 4 år sedan

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 kommenterad 4 år sedan

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 kommenterad 4 år sedan

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 kommenterad 4 år sedan

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 kommenterad 4 år sedan

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 kommenterad 4 år sedan

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 kommenterad 4 år sedan

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

This one was already changed to `clickRequireAccountDownload`. Is that ok?
kris kommenterad 4 år sedan
Skapare

@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 år sedan
jonathan tog bort grenen kris/download_account 4 år sedan

Granskare

jonathan begärda ändringar 4 år sedan
Pull-förfrågan har sammanfogats som 8582fc2745.
Logga in för att delta i denna konversation.
Inga granskare
Ingen Etikett
Ingen Milsten
Ingen tilldelad
2 Deltagare
Notiser
Förfallodatum

Inget förfallodatum satt.

Beroenden

Denna pull-förfrågan har för närvarande inga beroenden.

Laddar…
Det finns inget innehåll än.