#6264 - renewToken #1824

Merged
jsegarra merged 36 commits from 6264-renewToken into dev 2023-12-20 09:22:29 +00:00
Member
No description provided.
jsegarra added 5 commits 2023-11-06 08:06:55 +00:00
jsegarra requested review from alexm 2023-11-06 08:50:31 +00:00
jsegarra requested review from juan 2023-11-06 08:50:40 +00:00
jsegarra added 4 commits 2023-11-06 09:09:02 +00:00
jsegarra changed title from WIP: 6264-renewToken to 6264-renewToken 2023-11-06 09:09:13 +00:00
jsegarra changed title from 6264-renewToken to WIP: 6264-renewToken 2023-11-06 09:46:40 +00:00
jsegarra added 1 commit 2023-11-06 09:47:53 +00:00
gitea/salix/pipeline/head This commit looks good Details
0c2b2b25b7
refs #6264 fix: remove unnecessary file
alexm requested changes 2023-11-06 09:53:53 +00:00
@ -0,0 +1,7 @@
module.exports = async(token, accessTokenConfig) => {
const now = new Date();
Member

Creo que se seria mas correcto usar vnNew()

Mas info: https://wiki.verdnatura.es/index.php/Fechas

Creo que se seria mas correcto usar vnNew() Mas info: https://wiki.verdnatura.es/index.php/Fechas
Author
Member

Okey, tomo nota.
Lo extraje de lo que hay actualmente de renew-token

Okey, tomo nota. Lo extraje de lo que hay actualmente de renew-token
jsegarra marked this conversation as resolved
@ -0,0 +1,9 @@
const DEFAULT_FIELDS = ['renewPeriod', 'courtesyTime'];
Member

Esto lo haces por hacer solo una vez la peticion?

Esto lo haces por hacer solo una vez la peticion?
Author
Member

Correcto, porque al final la config no va a cambiar y hacer tantas peticiones a la misma info... :(

Correcto, porque al final la config no va a cambiar y hacer tantas peticiones a la misma info... :(
jsegarra marked this conversation as resolved
Member

*Borrar archivo

*Borrar archivo
jsegarra marked this conversation as resolved
jsegarra added 1 commit 2023-11-09 09:09:15 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
784f5bb7f9
refs #6264 perf: replace now with vnNew
jsegarra added 1 commit 2023-11-10 12:31:50 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
bcccd1894c
refs #6264 test: init test
jsegarra changed title from WIP: 6264-renewToken to 6264-renewToken 2023-11-17 13:20:11 +00:00
jsegarra requested review from alexm 2023-11-17 13:20:16 +00:00
alexm reviewed 2023-11-20 06:39:27 +00:00
alexm added 1 commit 2023-11-20 06:39:53 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
7951b874a6
Merge branch 'dev' into 6264-renewToken
alexm requested changes 2023-11-20 06:42:03 +00:00
@ -0,0 +1,9 @@
describe('Validate Token', () => {
it('Token is not expired', async() => {
Member

Vacios?

Vacios?
jsegarra marked this conversation as resolved
jsegarra changed title from 6264-renewToken to WIP: 6264-renewToken 2023-11-24 11:35:49 +00:00
jsegarra added 3 commits 2023-11-27 08:48:35 +00:00
jsegarra added 1 commit 2023-11-27 09:24:30 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
9da5fb9a14
refs #6264 other: rename camel-case variable
jgallego requested changes 2023-11-29 08:39:22 +00:00
jgallego left a comment
Owner

Valorar usar newDate en lugar de date.vnNew

Valorar usar newDate en lugar de date.vnNew
jsegarra added 2 commits 2023-11-30 06:32:27 +00:00
jsegarra changed title from WIP: 6264-renewToken to #6264-renewToken 2023-11-30 06:55:49 +00:00
jsegarra requested review from alexm 2023-11-30 06:55:54 +00:00
jsegarra requested review from jgallego 2023-11-30 06:55:55 +00:00
jgallego approved these changes 2023-11-30 07:57:55 +00:00
Dismissed
juan requested changes 2023-12-04 13:34:34 +00:00
@ -0,0 +6,4 @@
const accessTokenConfig = await models.AccessTokenConfig.findOne({fields});
if (!accessTokenConfig) currentAccessTokenConfig = accessTokenConfig;
return accessTokenConfig;
};
Owner

Utilizar directamente await models.AccessTokenConfig.findOne() en los ficheros js que incluyan a este. En el directorio methods solo deberían haber ficheros con métodos de la API rest.

Utilizar directamente `await models.AccessTokenConfig.findOne()` en los ficheros js que incluyan a este. En el directorio methods solo deberían haber ficheros con métodos de la API rest.
jsegarra marked this conversation as resolved
@ -1,3 +1,5 @@
const isTokenValid = require('./is-token-valid');
Owner

Porque se define en un fichero externo si sólo se utiliza en este método?

Porque se define en un fichero externo si sólo se utiliza en este método?
jsegarra marked this conversation as resolved
@ -4,0 +8,4 @@
const accessToken = await models.AccessToken.findById(token);
if (!accessToken) return next();
Owner

Este código parece que no tiene ningún efecto

Este código parece que no tiene ningún efecto
jsegarra marked this conversation as resolved
jsegarra added 3 commits 2023-12-04 13:47:12 +00:00
jsegarra dismissed jgallego’s review 2023-12-04 13:47:12 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

jsegarra changed title from #6264-renewToken to #6264 - renewToken 2023-12-04 13:47:19 +00:00
jsegarra added 1 commit 2023-12-05 12:05:46 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
a5fb07bf12
refs #6264 perf remove unnecessary code
jsegarra requested review from juan 2023-12-05 12:06:02 +00:00
juan requested changes 2023-12-11 08:54:11 +00:00
Dismissed
@ -23,2 +31,2 @@
const differenceMilliseconds = now - token.created;
const differenceSeconds = Math.floor(differenceMilliseconds / 1000);
// Check if current token is valid
const isValid = await Self.validateToken(token);
Owner

El método validateToken solo se llama desde aquí en toda la aplicación, puede eliminarse y poner su código directamente aquí.

El método validateToken solo se llama desde aquí en toda la aplicación, puede eliminarse y poner su código directamente aquí.
Author
Member

Era la idea, pero no la seguí porque había un remoteMethod de validateToken y asumí que si existía seria por algo y había que aprovecharlo.

Consideras que deberíamos mover la lógica de ese método a este y eliminarlo?

Era la idea, pero no la seguí porque había un remoteMethod de validateToken y asumí que si existía seria por algo y había que aprovecharlo. Consideras que deberíamos mover la lógica de ese método a este y eliminarlo?
Owner

Si ves que sólo se llama desde aquí sí.

Si ves que sólo se llama desde aquí sí.
jsegarra marked this conversation as resolved
@ -25,3 +34,2 @@
const fields = ['renewPeriod', 'courtesyTime'];
const accessTokenConfig = await models.AccessTokenConfig.findOne({fields});
const {courtesyTime} = await models.AccessTokenConfig.findOne({fields: ['renewPeriod', 'courtesyTime']});
Owner

Para que se selecciona renewPeriod si luego no se utiliza?

Para que se selecciona `renewPeriod` si luego no se utiliza?
Author
Member

Oh, tienes razón la línea venia heredada de otro sitio

Oh, tienes razón la línea venia heredada de otro sitio
jsegarra marked this conversation as resolved
@ -1,5 +1,5 @@
module.exports = function(options) {
return function(req, res, next) {
return async function(req, res, next) {
Owner

Porque la pasas a async? No tiene ningún await dentro

Porque la pasas a async? No tiene ningún await dentro
Author
Member

Tomo nota

Tomo nota
jsegarra marked this conversation as resolved
jsegarra requested review from juan 2023-12-11 11:39:16 +00:00
jsegarra added 2 commits 2023-12-12 18:44:03 +00:00
jsegarra added 1 commit 2023-12-13 06:14:57 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
d44cdcbd13
Merge branch 'dev' into 6264-renewToken
jgallego requested changes 2023-12-13 06:32:17 +00:00
@ -1,5 +1,15 @@
const UserError = require('vn-loopback/util/user-error');
const {models} = require('vn-loopback/server/server');
const DEFAULT_COURTESY_TIME = 60;
Owner

he visto el accessTokenConfig = 60 en las fixtures, no conviene quitarlo de aquí para no tener valores en el código?

he visto el accessTokenConfig = 60 en las fixtures, no conviene quitarlo de aquí para no tener valores en el código?
Author
Member

Se utiliza como valor por defecto del parámetro courtesyTime de una línea más abajo.

Esto se hace, por si, se diese el caso que el valor en BD está a nulo.

Se utiliza como valor por defecto del parámetro courtesyTime de una línea más abajo. Esto se hace, por si, se diese el caso que el valor en BD está a nulo.
jsegarra marked this conversation as resolved
jsegarra changed title from #6264 - renewToken to WIP: #6264 - renewToken 2023-12-13 12:33:04 +00:00
jsegarra added 1 commit 2023-12-13 12:40:00 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
d364a50ec4
refs #6264 feat: remove validateToken endpoint
jsegarra added 1 commit 2023-12-13 14:39:13 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
1e89ed6cdc
Merge branch 'dev' into 6264-renewToken
jsegarra changed title from WIP: #6264 - renewToken to #6264 - renewToken 2023-12-13 14:39:47 +00:00
jsegarra requested review from jgallego 2023-12-14 06:12:29 +00:00
jsegarra added 1 commit 2023-12-14 06:22:22 +00:00
gitea/salix/pipeline/head This commit looks good Details
485968037b
Merge branch 'dev' into 6264-renewToken
jgallego requested changes 2023-12-14 07:25:51 +00:00
@ -2,1 +2,4 @@
const {models} = require('vn-loopback/server/server');
const DEFAULT_COURTESY_TIME = 60;
const handlePromiseLogout = (Self, {id}, courtesyTime = DEFAULT_COURTESY_TIME) => {
Owner

tras la conversación asumimos que courtesy estará en la tabla definido

tras la conversación asumimos que courtesy estará en la tabla definido
jsegarra marked this conversation as resolved
jsegarra added 1 commit 2023-12-14 11:04:21 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
61a323078b
refs #6264 fix: remove DEFAULT_COURTESY_TIME
jsegarra changed title from #6264 - renewToken to WIP: #6264 - renewToken 2023-12-14 11:04:56 +00:00
jsegarra added 1 commit 2023-12-14 11:05:03 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
dddc749744
Merge branch 'dev' into 6264-renewToken
jsegarra added 2 commits 2023-12-14 11:21:24 +00:00
jsegarra added 1 commit 2023-12-14 11:26:12 +00:00
gitea/salix/pipeline/head This commit looks good Details
d2461d67c9
refs #6264 test: perf renew-token.spec.js
jsegarra changed title from WIP: #6264 - renewToken to #6264 - renewToken 2023-12-14 11:26:51 +00:00
jsegarra requested review from jgallego 2023-12-14 11:26:55 +00:00
jgallego approved these changes 2023-12-14 12:17:05 +00:00
jgallego removed review request for juan 2023-12-15 09:49:28 +00:00
jgallego dismissed juan’s review 2023-12-15 09:49:34 +00:00
alexm approved these changes 2023-12-15 11:12:05 +00:00
alexm added 1 commit 2023-12-15 11:12:54 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
d47a4a7c16
Merge branch 'dev' into 6264-renewToken
juan added 1 commit 2023-12-20 09:06:01 +00:00
gitea/salix/pipeline/head This commit looks good Details
10bfafdd3c
Merge branch 'dev' into 6264-renewToken
juan approved these changes 2023-12-20 09:06:08 +00:00
jsegarra merged commit 19bd02f952 into dev 2023-12-20 09:22:29 +00:00
jsegarra deleted branch 6264-renewToken 2023-12-20 09:22:29 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
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#1824
No description provided.