bubblev
/
bubble-web
8
0
Креирај огранак 0
Код Дискусије 0 Захтеви за спајање 0 Издања 0 Вики Activity
Лабеле Фазе
Нови захтев за спајање

#2 Add support for downloading account data

Спојено
jonathan споји(ла) 22 комит(е) из kris/download_account у master пре 4 година
Дискусија 3 Комити 22 Files Changed 6
kris коментирира пре 4 година
Још нема садржаја.
jonathan was assigned by kris пре 4 година
jonathan коментирира пре 4 година
Власник

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 changed title from WIP: (not finished) Add support for downloading account data to Add support for downloading account data пре 4 година
kris коментирира пре 4 година
Аутор

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 requested changes пре 4 година
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 коментирира пре 4 година

I would call this requestAccountDownload

I would call this `requestAccountDownload`
kris коментирира пре 4 година

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 коментирира пре 4 година

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 коментирира пре 4 година

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 коментирира пре 4 година

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 коментирира пре 4 година

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 коментирира пре 4 година

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 коментирира пре 4 година

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

This one was already changed to `clickRequireAccountDownload`. Is that ok?
kris коментирира пре 4 година
Аутор

@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 година
jonathan deleted branch kris/download_account пре 4 година

Reviewers

jonathan requested changes пре 4 година
The pull request has been merged as 8582fc2745.
Пријавите се да се прикључе у овом разговору.
Reviewers
Request review
No reviewers
 jonathan
Лавеле
Apply labels
Уклони лабеле
No items
Нема лабеле
Фаза
Set milestone
Уклони фазу
No items
Нема фазе
Assignees
Assign users
Clear assignees
No Assignees
 jonathan
2 учесника
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Write Preview
Loading…
Откажи
Сачувај
Још нема садржаја.
Delete Branch 'kris/download_account'

Deleting a branch is permanent. It CANNOT be undone. Continue?

Не
Да