verdnatura/salix
verdnatura
/
salix
3
4
Fork 0
Code Issues Pull Requests 57 Releases Wiki Activity

#6427 - SMS Recover Password #2037

Open
jsegarra wants to merge 72 commits from 6427_sms_resetPassword into dev
pull from: 6427_sms_resetPassword
merge into: verdnatura:dev
Conversation 3 Commits 72 Files Changed 33 +421 -111
jsegarra commented 2024-02-14 07:03:05 +00:00
Member
Copy Link
No description provided.
jsegarra added 3 commits 2024-02-14 07:10:33 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
c86089ea6d refs #6427 feat: front
jsegarra changed title from WIP: #6427 - SMS Recover Password to #6427 - SMS Recover Password 2024-02-14 10:58:49 +00:00
jsegarra requested review from jgallego 2024-02-14 10:58:59 +00:00
jsegarra requested review from alexm 2024-02-14 10:59:00 +00:00
jsegarra added 1 commit 2024-02-14 11:00:39 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
91c3100a6d refs #6427 feat: comments
jgallego refused to review 2024-02-16 14:28:05 +00:00
alexm requested changes 2024-02-19 08:27:27 +00:00
Dismissed
back/methods/vn-user/recover-password.js Outdated
@ -23,2 +24,3 @@
Self.recoverPassword = async function(user, app) {
const models = Self.app.models;
const usesPhone = new RegExp(/([+]\d{2})?\d{9}/, 'g').test(user);
const account = await Self.app.models.Application.rawSql(
alexm commented 2024-02-19 08:01:31 +00:00
Member
Copy Link

Tabular segun la convencion de SQL y usar JOIN

Tabular segun la convencion de SQL y usar JOIN
jsegarra marked this conversation as resolved
back/methods/vn-user/recover-password.js Outdated
@ -35,3 +33,3 @@
try {
await Self.resetPassword({email: user, emailTemplate: 'recover-password', app});
await Self.resetPassword({email, phone, emailTemplate: 'recover-password', app, usesPhone});
alexm commented 2024-02-19 08:15:32 +00:00
Member
Copy Link

Yo igual usaria un único parametro que sea userContact o algo parecido. en vez de pasar email y phone

Yo igual usaria un único parametro que sea `userContact` o algo parecido. en vez de pasar email y phone
jsegarra marked this conversation as resolved
back/models/vn-user.js Outdated
@ -119,2 +119,4 @@
for (const param in options)
params[param] = options[param];
if (info.options?.usesPhone)
await Self.app.models.Sms.send({req: {accessToken: info.accessToken}}, +info.options.phone, params.url);
alexm commented 2024-02-19 08:16:41 +00:00
Member
Copy Link

El + para que es?
Y yo igual le pondria algo mas al SMS(params.url) no solo el link

El `+` para que es? Y yo igual le pondria algo mas al SMS(params.url) no solo el link
jsegarra marked this conversation as resolved
db/dump/fixtures.before.sql Outdated
@ -3068,2 +3068,4 @@
(2,'123456N','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet',69,3,4,2,'Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet'),
(3,'123456B','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet',567,5,6,69,'Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet','Lorem ipsum dolor sit amet');
UPDATE `vn`.`client`
alexm commented 2024-02-19 08:18:53 +00:00
Member
Copy Link

Mejor que poner update, si se pude poner, es modificar el insert de client o ponerlo cerca
Si no luego es un lio ver de pq un cliente tiene x phone y no esta en el insert

Mejor que poner update, si se pude poner, es modificar el insert de `client` o ponerlo cerca Si no luego es un lio ver de pq un cliente tiene x phone y no esta en el insert
jsegarra marked this conversation as resolved
front/salix/components/recover-password/index.html Outdated
@ -1,13 +1,28 @@
<h5 class="vn-mb-md vn-mt-lg" translate>Recover password</h5>
<vn-check
alexm commented 2024-02-19 08:26:30 +00:00
Member
Copy Link

Igual pondría un radio button para que sepan que por defecto es email

Igual pondría un radio button para que sepan que por defecto es email
jsegarra commented 2024-03-26 08:43:19 +00:00
Author
Member
Copy Link

Si pones radio button tienes que añadir otra opción y gestionar el estado, entonces con el checkbox, te aseguras que es true o false, y por tanto muestra el otro método o no

Si pones radio button tienes que añadir otra opción y gestionar el estado, entonces con el checkbox, te aseguras que es true o false, y por tanto muestra el otro método o no
front/salix/components/recover-password/index.html Outdated
@ -8,3 +13,1 @@
class="text-secondary"
translate>
We will sent you an email to recover your password
<vn-textfield
alexm commented 2024-02-19 08:23:27 +00:00
Member
Copy Link

No entiendo el poner 2 vn-textfields con ifs.
Con poner `label="User, phone, or recovery email" sobraria.

Y tampoco se si deberia poder poner numeros de telefono para decir que son ellos (lo consultaria con Juan)

No entiendo el poner 2 vn-textfields con ifs. Con poner `label="User, phone, or recovery email" sobraria. Y tampoco se si deberia poder poner numeros de telefono para decir que son ellos (lo consultaria con Juan)
jsegarra commented 2024-04-10 09:44:26 +00:00
Author
Member
Copy Link

El teléfono se usa para validar la acción de recuperar la contraseña. Porque puede darse el caso que el usuario ponga su id y no le esté llegando el SMS porque en algún momento se equivocó de teléfono.

El teléfono se usa para validar la acción de recuperar la contraseña. Porque puede darse el caso que el usuario ponga su id y no le esté llegando el SMS porque en algún momento se equivocó de teléfono.
alexm commented 2024-04-22 05:26:43 +00:00
Member
Copy Link

Entonces podría poner tu id, y mi numero de teléfono y te podría cambiar la contraseña?
Lo que se hacia con el correo es apartir del correo sacar el id del usuario. Supongo que con el telefono sera igual

Entonces podría poner tu id, y mi numero de teléfono y te podría cambiar la contraseña? Lo que se hacia con el correo es apartir del correo sacar el id del usuario. Supongo que con el telefono sera igual
jsegarra commented 2024-04-22 05:54:23 +00:00
Author
Member
Copy Link

No podrías, porque tu pones el id de usuario y teléfono, y si ambos valores no existen, no te envía SMS. En local puedes probar con el userId:9 que tiene el teléfono "432978106"

El teléfono de recuperación solo lo puede cambiar quien es propietario del registro, ya que tiene una validación del id del registro contra el id del usuario logeado

Pero vamos, que yo podría estar contaminado con el desarrollo, y a lo mejor tu consigues bordear la restricción. si es así, repórtamelo, por favor.

No podrías, porque tu pones el id de usuario y teléfono, y si ambos valores no existen, no te envía SMS. En local puedes probar con el userId:9 que tiene el teléfono "432978106" El teléfono de recuperación solo lo puede cambiar quien es propietario del registro, ya que tiene una validación del id del registro contra el id del usuario logeado Pero vamos, que yo podría estar contaminado con el desarrollo, y a lo mejor tu consigues bordear la restricción. si es así, repórtamelo, por favor.
front/salix/components/recover-password/index.html Outdated
@ -11,0 +17,4 @@
vn-focus>
</vn-textfield>
<div class="text-secondary">
<span ng-if="$ctrl.sms" translate>
alexm commented 2024-02-19 08:25:40 +00:00
Member
Copy Link

Y aqui igual en vez de poner dos, pondria algo como. te enviaremos un mensaje por el tipo de envio elegido o algo asi

Y aqui igual en vez de poner dos, pondria algo como. te enviaremos un mensaje por el tipo de envio elegido o algo asi
jsegarra marked this conversation as resolved
front/salix/components/recover-password/index.js Outdated
@ -10,6 +10,11 @@ export default class Controller {
$translate,
$state
});
$scope.$watch('$ctrl.user', function(nuevoValor) {
alexm commented 2024-02-19 08:27:14 +00:00
Member
Copy Link

Esto cuando se usa??

Esto cuando se usa??
jsegarra marked this conversation as resolved
jsegarra changed title from #6427 - SMS Recover Password to WIP: #6427 - SMS Recover Password 2024-02-19 08:31:07 +00:00
jsegarra added 1 commit 2024-02-19 13:43:12 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
c06af7ac9a Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 1 commit 2024-03-07 09:45:54 +00:00
jsegarra added 3 commits 2024-03-18 13:59:21 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
6a47493180 refs #6427 feat: copy phone from vn.client
jsegarra added 4 commits 2024-03-20 12:51:48 +00:00
jsegarra added 1 commit 2024-03-20 13:11:52 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
f75bd04736 refs #6427 perf: restorePasswordSMS
jsegarra added 5 commits 2024-03-26 07:21:24 +00:00
jsegarra added 1 commit 2024-03-26 07:27:30 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
06ed466953 refs #6427 perf: minor changes
jsegarra added 1 commit 2024-03-26 07:28:29 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
fe3a97cd38 refs #6427 perf: minor changes
jsegarra added 1 commit 2024-03-26 07:29:40 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
fbd2c28a81 refs #6427 perf: minor descriptions changes
jsegarra added 1 commit 2024-03-26 08:45:35 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
928b2f5807 refs #6427 revert vn-user changes
jsegarra requested review from juan 2024-03-26 08:45:56 +00:00
jsegarra requested review from alexm 2024-03-26 08:45:57 +00:00
jsegarra changed title from WIP: #6427 - SMS Recover Password to #6427 - SMS Recover Password 2024-03-26 08:46:04 +00:00
alexm requested changes 2024-03-27 06:55:08 +00:00
Dismissed
alexm left a comment
Member
Copy Link

Faltaria añadir tests a back/methods/vn-user/recover-passwordSMS.js
PD: Como esta en estado de analisis se deberia hacer cuando pase a nueva

Faltaria añadir tests a back/methods/vn-user/recover-passwordSMS.js PD: Como esta en estado de **analisis** se deberia hacer cuando pase a nueva
back/methods/vn-user/recover-passwordSMS.js Outdated
@ -0,0 +37,4 @@
}
});
Self.recoverPasswordSMS = async function(ctx, id, phone, _otp, options) {
alexm commented 2024-03-27 06:39:01 +00:00
Member
Copy Link

pq aqui es _opt pero en accepts es opt?
Usar otp

pq aqui es `_opt` pero en accepts es `opt`? Usar `otp`
jsegarra commented 2024-03-27 07:33:24 +00:00
Author
Member
Copy Link

Correcto, el nombre de la variable está obsoleto
Gracias
Lo cambio todo por code

Correcto, el nombre de la variable está obsoleto Gracias Lo cambio todo por code
alexm commented 2024-04-22 05:27:24 +00:00
Member
Copy Link

pq aqui es _opt pero en accepts es opt?
Usar otp

pq aqui es `_opt` pero en accepts es `opt`? Usar `otp`
jsegarra marked this conversation as resolved
back/methods/vn-user/recover-passwordSMS.js Outdated
@ -0,0 +52,4 @@
where: {id, phone}
};
const user = await Self.findOne(query);
alexm commented 2024-03-27 06:37:36 +00:00
Member
Copy Link

Pondria directamente aqui el filtro

Pondria directamente aqui el filtro
👀 1
alexm commented 2024-04-22 05:27:53 +00:00
Member
Copy Link
const user = await Self.findOne({
            fields: ['id', 'phone', 'email', 'name'],
            where: {id, phone}
        });
``` const user = await Self.findOne({ fields: ['id', 'phone', 'email', 'name'], where: {id, phone} }); ```
jsegarra marked this conversation as resolved
alexm requested changes 2024-03-27 06:56:36 +00:00
Dismissed
alexm left a comment
Member
Copy Link

❗Y las tareas en analisis NO se debe poner a nadie como revisor, dado que cuando pasan al estado analizado ya se revisa por javi/juan

❗Y las tareas en analisis **NO** se debe poner a nadie como revisor, dado que cuando pasan al estado analizado ya se revisa por javi/juan
jsegarra commented 2024-03-27 07:29:35 +00:00
Author
Member
Copy Link

❗Y las tareas en analisis NO se debe poner a nadie como revisor, dado que cuando pasan al estado analizado ya se revisa por javi/juan

Cierto, porque no esta en progreso ni en espera.
De echo voy a cambiar a WIP porque todavi lo tiene que revisar Juan
Gracias @alexm

> ❗Y las tareas en analisis **NO** se debe poner a nadie como revisor, dado que cuando pasan al estado analizado ya se revisa por javi/juan Cierto, porque no esta en progreso ni en espera. De echo voy a cambiar a WIP porque todavi lo tiene que revisar Juan Gracias @alexm
jsegarra changed title from #6427 - SMS Recover Password to WIP: #6427 - SMS Recover Password 2024-03-27 07:29:40 +00:00
jsegarra requested review from alexm 2024-04-10 09:35:05 +00:00
jsegarra added 1 commit 2024-04-10 09:35:06 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
6edbada581 Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 2 commits 2024-04-10 13:06:53 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
a1a6e483cf refs #6427 feat: not update phone when is not owner
alexm commented 2024-04-18 07:30:31 +00:00
Member
Copy Link

Has solicitado revision pero esta en WIP, la reviso??

Has solicitado revision pero esta en WIP, la reviso??
jsegarra added 1 commit 2024-04-18 07:50:31 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
383433766e Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 1 commit 2024-04-18 08:04:31 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
9a7bee8ded feat(salix): refs #6427 Disable field when is not owner
jsegarra changed title from WIP: #6427 - SMS Recover Password to #6427 - SMS Recover Password 2024-04-18 08:05:38 +00:00
alexm requested changes 2024-04-22 05:37:24 +00:00
Dismissed
alexm left a comment
Member
Copy Link

Yo le daria una vuelta al codigo.
Lo que yo veo es que al querer recueperar contraseña debes poner tu usuario/correo (se podría añadir que tambien con telefono? @juan) que es con lo que te logeas.
Luego con checkbox/radio button seleccionas la opcion que quieres, telefono o email.
Y de ahi el mismo back que estaba. decide si llamar al que enviara un correo o al que te enviara un sms pero que te envie lo mismo (creo recordar que era un link, que puedes abrir igualmente en movil que en pc).
Y de ahi a cambiar la contraseña que como usa la misma funcionalidad que ya estaba no habria que tocar nada

Yo le daria una vuelta al codigo. Lo que yo veo es que al querer recueperar contraseña debes poner tu usuario/correo (se podría añadir que tambien con telefono? @juan) que es con lo que te logeas. Luego con checkbox/radio button seleccionas la opcion que quieres, telefono o email. Y de ahi el mismo back que estaba. decide si llamar al que enviara un correo o al que te enviara un sms pero que te envie lo mismo (creo recordar que era un link, que puedes abrir igualmente en movil que en pc). Y de ahi a cambiar la contraseña que como usa la misma funcionalidad que ya estaba no habria que tocar nada
jsegarra added 1 commit 2024-04-22 05:58:17 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
ef8a711ed6 feat(salix): refs #6427 updates
jsegarra added 1 commit 2024-04-22 06:00:51 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
3565e433c2 feat(salix): refs #6427 front updates
jsegarra changed title from #6427 - SMS Recover Password to WIP: #6427 - SMS Recover Password 2024-04-27 10:00:43 +00:00
jsegarra commented 2024-04-27 10:01:33 +00:00
Author
Member
Copy Link

Cambio a WIP porque voy a hacer una modificaciones visuales
@alexm El flujo para resetear por SMS está como dices, si quieres reservamos una hora y lo analizamos juntos

Cambio a WIP porque voy a hacer una modificaciones visuales @alexm El flujo para resetear por SMS está como dices, si quieres reservamos una hora y lo analizamos juntos
jsegarra added 1 commit 2024-04-27 18:11:12 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
741ccf411e Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 3 commits 2024-04-27 19:29:02 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
5914c73844 feat(salix): refs #6427 #6427 Update front
jsegarra changed title from WIP: #6427 - SMS Recover Password to #6427 - SMS Recover Password 2024-04-27 19:29:07 +00:00
jsegarra requested review from alexm 2024-04-27 19:29:11 +00:00
alexm reviewed 2024-04-29 06:05:32 +00:00
alexm left a comment
Member
Copy Link

Veo bastante raro el código, como no se si es esa la "forma" que se quiere, si puede revisar el flujo antes @juan mejor

Veo bastante raro el código, como no se si es esa la "forma" que se quiere, si puede revisar el flujo antes @juan mejor
jsegarra added 1 commit 2024-05-02 06:53:17 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
c5adc8576a Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 2 commits 2024-05-07 04:03:24 +00:00
jsegarra added 3 commits 2024-05-23 12:45:25 +00:00
jsegarra added 1 commit 2024-05-23 12:47:38 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
0f201c36de feat(salix): refs #6427 remove bad merge change
jsegarra added 1 commit 2024-05-23 13:35:22 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
4d40d825a2 feat(salix): refs #6427 recoveryPhone security
jsegarra added 3 commits 2024-05-23 19:23:14 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
d534ee423b test(salix): refs #6427 improve test
jsegarra added 1 commit 2024-05-23 20:32:36 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
1d04ca1c8a test(salix): refs #6427 improve test
jsegarra added 2 commits 2024-05-24 06:02:18 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
8201d057f3 Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 1 commit 2024-05-24 09:11:30 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
9953df2f4f perf(salix): refs #6427 imrpove
jsegarra added 2 commits 2024-05-27 07:16:28 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
faced84393 Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 1 commit 2024-05-27 09:54:22 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
dd73b960df Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 1 commit 2024-05-30 10:32:58 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
44ce536901 Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 2 commits 2024-05-30 18:44:34 +00:00
jsegarra changed title from #6427 - SMS Recover Password to WIP: #6427 - SMS Recover Password 2024-05-31 06:22:30 +00:00
jsegarra added 2 commits 2024-05-31 06:55:13 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
6e169768db feat(salix): refs #6427 #6427 handle Send sms from app
jsegarra added 1 commit 2024-05-31 07:25:18 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
0eb242ac4a Merge branch 'dev' into 6427_sms_resetPassword
jsegarra changed title from WIP: #6427 - SMS Recover Password to #6427 - SMS Recover Password 2024-05-31 07:26:12 +00:00
alexm refused to review 2024-06-06 06:37:12 +00:00
jsegarra added 1 commit 2024-06-14 09:44:11 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
57083e2f5c Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 8 commits 2024-06-14 13:54:47 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
42023c58ec Merge branch 'dev' into 6427_sms_resetPassword
jsegarra added 1 commit 2024-06-17 12:26:41 +00:00
gitea/salix/pipeline/pr-dev There was a failure building this commit Details
04520cafcf feat(Salix): refs #6427 #6427 Add loggable to models
jgallego dismissed alexm’s review 2024-08-19 11:45:14 +00:00
Reason:

que revise solo juan

juan requested changes 2024-09-18 12:59:15 +00:00
juan left a comment
Owner
Copy Link

Cuando puedas me dices y lo miramos/hablamos

Cuando puedas me dices y lo miramos/hablamos
back/methods/sms/sendSms.js Outdated
@ -0,0 +58,4 @@
} catch (e) {
console.error(e);
}
// return response;
juan commented 2024-06-11 07:15:35 +00:00
Owner
Copy Link

Eliminar linea comentada

Eliminar linea comentada
jsegarra marked this conversation as resolved
front/salix/components/recover-password/index.html Outdated
@ -11,0 +31,4 @@
ng-model="$ctrl.method"
>
</vn-radio></vn-vertical
></vn-one>
juan commented 2024-06-11 07:25:33 +00:00
Owner
Copy Link

Corregir tabulación

Corregir tabulación
jsegarra marked this conversation as resolved
modules/account/front/basic-data/index.html Outdated
@ -30,0 +32,4 @@
label="Recovery phone"
ng-model="$ctrl.user.recoveryPhone"
disabled="$root.user.id !== $ctrl.user.id"
>
juan commented 2024-06-11 07:25:45 +00:00
Owner
Copy Link

Corregir tabulación

Corregir tabulación
jsegarra marked this conversation as resolved
jsegarra added 2 commits 2024-10-02 09:28:40 +00:00
gitea/salix/pipeline/pr-dev This commit looks good Details
580c2c6f10 perf(salix): refs #6427 #6427 comments
jsegarra added 1 commit 2024-11-08 11:12:33 +00:00
jsegarra added 1 commit 2024-11-08 11:17:29 +00:00
jsegarra requested review from juan 2024-11-08 11:18:00 +00:00
This pull request has changes conflicting with the target branch.
  • loopback/locale/en.json
  • loopback/locale/es.json
Sign in to join this conversation.
Reviewers
No reviewers
juan
Labels
Clear labels   
CR / Tests passed

Unit tests and e2e passed

  
e2e

Implement e2e

  
Missing tests

Missing tests

  
Needs refactor

   
Simple

Simple check / No CR needed

No Label
CR / Tests passed
e2e
Missing tests
Needs refactor
Simple
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: verdnatura/salix#2037
No description provided.
Delete Branch "6427_sms_resetPassword"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?