bubblev
/
bubble-web
8
0
Bifurcation 0
Code Tickets 0 Demandes d'ajout 0 Versions 0 Wiki Activité
Étiquettes Jalons
Nouvelle demande d'ajout

#2 Add support for downloading account data

Fusionnée
jonathan a fusionné 22 révision(s) à partir de kris/download_account vers master il y a 4 ans
Discussion 3 Révisions 22 Fichiers Modifiés 6
kris a commenté il y a 4 ans
Il n'existe pas encore de contenu.
jonathan s'est vu assigner cela par kris il y a 4 ans
jonathan a commenté il y a 4 ans
Propriétaire

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 a modifié le titre de WIP: (not finished) Add support for downloading account data à Add support for downloading account data il y a 4 ans
kris a commenté il y a 4 ans
Publier

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 a requis les changements il y a 4 ans
jonathan laisser un commentaire

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 a commenté il y a 4 ans

I would call this requestAccountDownload

I would call this `requestAccountDownload`
kris a commenté il y a 4 ans

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 a commenté il y a 4 ans

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 a commenté il y a 4 ans

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 a commenté il y a 4 ans

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 a commenté il y a 4 ans

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 a commenté il y a 4 ans

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 a commenté il y a 4 ans

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

This one was already changed to `clickRequireAccountDownload`. Is that ok?
kris a commenté il y a 4 ans
Publier

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

@jonathan please check my answers here and confirm what to do...
jonathan a fermé cette pull request il y a 4 ans
jonathan a supprimé la branche kris/download_account il y a 4 ans

Relecteurs

jonathan a requis les changements il y a 4 ans
La demande de tirage a été fusionnée en 8582fc2745.
Connectez-vous pour rejoindre cette conversation.
Relecteurs
Demander une revue
Pas de relecteur
 jonathan
Étiquettes
Appliquer une étiquette
Effacer les étiquettes
Pas d'élément
Pas d'étiquette
Jalon
Affecter un jalon
Effacer le jalon
Pas d'élément
Aucun jalon
Affecté à
Assigner des utilisateurs
Supprimer les affectations
Pas d'assignataires
 jonathan
2 participants
Notifications
Échéance

Aucune échéance n'a été définie.

Dépendances

La demande de fusion n'a actuellement pas de dépendance.

Écrire Aperçu
Chargement…
Annuler
Enregistrer
Il n'existe pas encore de contenu.
Supprimer la branche 'kris/download_account'

Supprimer une branche est permanent. Cela NE PEUVENT être annulées. Continuer ?

Non
Oui