verdnatura/hedera-web
verdnatura
/
hedera-web
0
1
Fork 2
Code Issues Pull Requests 6 Releases Wiki Activity

WIP: refactor: migrate fetchUser API call from PHP to Salix #111

Draft
ldragan wants to merge 5 commits from ldragan/hedera-web:taro/migrate-fetchUser into beta
pull from: ldragan/hedera-web:taro/migrate-fetchUser
merge into: verdnatura:beta
Conversation 0 Commits 5 Files Changed 1 +4 -5
ldragan commented 2025-02-18 03:42:04 +00:00
First-time contributor
Copy Link

WIP: bloqueado por verdnatura/salix#3488

WIP: bloqueado por https://gitea.verdnatura.es/verdnatura/salix/pulls/3488
ldragan added 1 commit 2025-02-18 03:42:05 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
29c42d7f34 refactor: migrate `fetchUser` API call from PHP to Salix
ldragan requested review from jsegarra 2025-02-18 03:47:05 +00:00
jsegarra reviewed 2025-02-18 08:04:41 +00:00
src/stores/user.js Outdated
@ -253,3 +251,1 @@
);
if (userType === 'user') mainUser.value = userData;
else supplantedUser.value = userData;
const _token = getToken();
jsegarra commented 2025-02-18 08:04:41 +00:00
Member
Copy Link

duda: porque el _?

duda: porque el _?
ldragan marked this conversation as resolved
jsegarra requested changes 2025-02-18 08:07:05 +00:00
src/stores/user.js
@ -254,2 +251,2 @@
if (userType === 'user') mainUser.value = userData;
else supplantedUser.value = userData;
const _token = getToken();
const userData = await api.get('VnUsers/getCurrentUserData', {
jsegarra commented 2025-02-18 08:07:03 +00:00
Member
Copy Link

El token no es necesario porque está definido que se coja en boot/axios
De esta manera evitamos que hacer getToken y pasarlo como parametro de la petición.
Prueba a quitarlo y me dices si obtienes el mismo resultado. Si no comentamos

El token no es necesario porque está definido que se coja en boot/axios De esta manera evitamos que hacer getToken y pasarlo como parametro de la petición. Prueba a quitarlo y me dices si obtienes el mismo resultado. Si no comentamos
ldragan commented 2025-02-19 05:14:23 +00:00
Author
First-time contributor
Copy Link

Tenés razón. Probé quitarlo y anda perfecto.

En renewToken se pasa de forma explícita, pero también se autentica con el _tokenMultimedia. Supongo que en ese caso tiene sentido, incluso si redundante, para esclarecer posibles dudas comunicando intención deliberada:

        const _token = getToken();
        const tokenData = await api.post('VnUsers/renewToken', {
            headers: { Authorization: _token }
        });

        const _tokenMultimedia = getTokenMultimedia();
        const tokenMultimedia = await api.post('VnUsers/renewToken', {
            headers: { Authorization: _tokenMultimedia }
        });
Tenés razón. Probé quitarlo y anda perfecto. En `renewToken` se pasa de forma explícita, pero también se autentica con el `_tokenMultimedia`. Supongo que en ese caso tiene sentido, incluso si redundante, para esclarecer posibles dudas comunicando intención deliberada: ```js const _token = getToken(); const tokenData = await api.post('VnUsers/renewToken', { headers: { Authorization: _token } }); const _tokenMultimedia = getTokenMultimedia(); const tokenMultimedia = await api.post('VnUsers/renewToken', { headers: { Authorization: _tokenMultimedia } }); ```
ldragan marked this conversation as resolved
ldragan added 1 commit 2025-02-19 05:06:11 +00:00
ldragan added 1 commit 2025-02-19 05:17:16 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
93c481aadf handle possible error case
jsegarra changed title from WIP: refactor: migrate `fetchUser` API call from PHP to Salix to refactor: migrate `fetchUser` API call from PHP to Salix 2025-02-19 08:01:16 +00:00
jsegarra requested changes 2025-02-19 08:04:36 +00:00
src/stores/user.js Outdated
@ -255,1 +251,3 @@
else supplantedUser.value = userData;
const userData = await api.get('VnUsers/getCurrentUserData');
if (!userData?.data) {
jsegarra commented 2025-02-19 08:04:34 +00:00
Member
Copy Link

necesitaré mas info sobre esto. porque no tenemos console.error hechos de esta manera sino que estan dentro de trycatch
Axios ya tiene manejador para los errores

necesitaré mas info sobre esto. porque no tenemos console.error hechos de esta manera sino que estan dentro de trycatch Axios ya tiene manejador para los errores
ldragan commented 2025-02-20 07:55:42 +00:00
Author
First-time contributor
Copy Link

Aquí estoy siguiendo el patrón de fetchTokenConfig, que, cuando el server retorna un algo falsey, simplemente termina temprano:

    const fetchTokenConfig = async () => {
        try {
            const { data } = await api.get('AccessTokenConfigs/findOne', {
                filter: { fields: ['renewInterval', 'renewPeriod'] }
            });
            if (!data) return; // <--- AQUI
            tokenConfig.value = data;
            storage.value.setItem('renewPeriod', data.renewPeriod);
            return data;
        } catch (error) {
            notify('errors.tokenConfig', 'negative');
            console.error('Error fetching token config:', error);
        }
    };

Pero le agrego un console.error para no perder completamente el registro. Si tienen Sentry o afines, podrán ver ese log en producción. De lo contrario, solo servirá para debugging.

Por otro lado, por lo que estoy viendo, parece ser imposible que !userData, y !userData.data es técnicamente posible (Axios declara T = any), pero eso implicaría que

  • el server retornó algo que no es un objeto JSON, en cuyo caso, asumo, Axios tiraría error,
  • se configuró Axios con algún transformador post-respuesta que retorna algo falsey, lo cual, probablemente, sería un bug

Así que este chequeo parece redundante. Lo quitaré.

En el caso que tomé de ejemplo, además de las otras razones, esto es una llamada a AccessTokenConfigs/findOne sin filtros, y entiendo que nunca ocurrirá que no exista ningún solo registro en esa tabla. Así que, imagino, deberíamos poder quitar esa verificación en ese caso. Pero bueno, eso sería para otro PR.

Aquí estoy siguiendo el patrón de `fetchTokenConfig`, que, cuando el server retorna un algo falsey, simplemente termina temprano: ```js const fetchTokenConfig = async () => { try { const { data } = await api.get('AccessTokenConfigs/findOne', { filter: { fields: ['renewInterval', 'renewPeriod'] } }); if (!data) return; // <--- AQUI tokenConfig.value = data; storage.value.setItem('renewPeriod', data.renewPeriod); return data; } catch (error) { notify('errors.tokenConfig', 'negative'); console.error('Error fetching token config:', error); } }; ``` Pero le agrego un `console.error` para no perder completamente el registro. Si tienen Sentry o afines, podrán ver ese log en producción. De lo contrario, solo servirá para debugging. --- Por otro lado, por lo que estoy viendo, parece ser imposible que `!userData`, y `!userData.data` es técnicamente posible (Axios declara `T = any`), pero eso implicaría que - el server retornó algo que no es un objeto JSON, en cuyo caso, _asumo_, Axios tiraría error, - se configuró Axios con algún transformador post-respuesta que retorna algo falsey, lo cual, probablemente, sería un bug Así que este chequeo parece redundante. Lo quitaré. --- En el caso que tomé de ejemplo, además de las otras razones, esto es una llamada a `AccessTokenConfigs/findOne` sin filtros, y entiendo que nunca ocurrirá que no exista ningún solo registro en esa tabla. Así que, imagino, deberíamos poder quitar esa verificación en ese caso. Pero bueno, eso sería para otro PR.
ldragan marked this conversation as resolved
ldragan added 1 commit 2025-02-20 07:57:35 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
4ad4cb17e8 refactor: remove unnecessary verification
ldragan changed title from refactor: migrate `fetchUser` API call from PHP to Salix to WIP: refactor: migrate `fetchUser` API call from PHP to Salix 2025-02-24 08:53:31 +00:00
ldragan added 1 commit 2025-02-24 14:16:21 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
52fec5d087 Merge branch 'beta' into taro/migrate-fetchUser
All checks were successful
gitea/hedera-web/pipeline/pr-beta This commit looks good
Required
Details
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
Sign in to join this conversation.
Reviewers
No reviewers
jsegarra
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 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/hedera-web#111
No description provided.
Delete Branch "ldragan/hedera-web:taro/migrate-fetchUser"

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?