verdnatura/hedera-web
verdnatura
/
hedera-web
0
1
Fork 3
Code Issues Pull Requests 4 Releases Wiki Activity

fix: migrate pending orders #121

Merged
jsegarra merged 7 commits from ldragan/hedera-web:taro/pending-orders into beta 2025-03-27 05:15:04 +00:00
Conversation 1 Commits 7 Files Changed 3 +71 -26
ldragan commented 2025-02-28 08:58:38 +00:00
Member
Copy Link

Needs verdnatura/salix#3506 fusionada

Needs https://gitea.verdnatura.es/verdnatura/salix/pulls/3506 *fusionada*
ldragan added 3 commits 2025-02-28 08:58:39 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
fefb3dab1f lol
ldragan changed title from WIP: taro/pending-orders to WIP: migrate pending orders 2025-02-28 09:00:04 +00:00
ldragan added 2 commits 2025-03-04 06:31:23 +00:00
ldragan added 2 commits 2025-03-04 06:35:54 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
e1e87cfcd6 rm Dockerfile-front
ldragan added 1 commit 2025-03-04 06:37:27 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
d627bdb59a revert quasar.config.js
ldragan force-pushed taro/pending-orders from d627bdb59a to 15720e5a59 2025-03-04 06:48:29 +00:00 Compare
ldragan force-pushed taro/pending-orders from 15720e5a59 to 0f0717f382 2025-03-04 06:54:11 +00:00 Compare
ldragan added 1 commit 2025-03-04 06:54:48 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
2c5473cbf3 Merge branch 'beta' into taro/pending-orders
ldragan changed title from WIP: migrate pending orders to migrate pending orders 2025-03-04 06:55:23 +00:00
ldragan requested review from jsegarra 2025-03-04 06:55:32 +00:00
ldragan changed title from migrate pending orders to refactor: migrate pending orders 2025-03-04 06:55:48 +00:00
ldragan changed title from refactor: migrate pending orders to fix: migrate pending orders 2025-03-04 06:56:03 +00:00
jsegarra reviewed 2025-03-04 10:07:42 +00:00
src/pages/Ecomerce/PendingOrders.vue Outdated
@ -66,3 +102,1 @@
onMounted(async () => {
getOrders();
});
watch(
jsegarra commented 2025-03-04 10:07:42 +00:00
Member
Copy Link

Entiendo que este watch es para cuando hacemos suplantar no¿?

Entiendo que este watch es para cuando hacemos suplantar no¿?
ldragan commented 2025-03-13 04:38:50 +00:00
Author
Member
Copy Link

Aquí me quedó pendiente poner el código de Lilium, que Willy compartió conmigo.

Aquí me quedó pendiente poner el código de Lilium, que Willy compartió conmigo.
ldragan commented 2025-03-21 03:00:20 +00:00
Author
Member
Copy Link

@jsegarra probé el código del FetchData component, pero usar un componente Vue para algo que no tienen nada visual no me parece lo ideal. A nivel patrones, para ese tipo de cosas son los data stores, hooks, etc.

Sí es cierto que, cual dijiste, esto será necesario en varios componentes (no todos), así que lo refactoricé para que sea un simple onUserId del lado de los componentes. Esa función vive en su propio archivo, src/utils/onuserId.js. La estoy usando en este PR y https://gitea.verdnatura.es/verdnatura/hedera-web/pulls/124/files.

@jsegarra probé el código del FetchData component, pero usar un componente Vue para algo que no tienen nada visual no me parece lo ideal. A nivel patrones, para ese tipo de cosas son los data stores, hooks, etc. Sí es cierto que, cual dijiste, esto será necesario en varios componentes (no todos), así que lo refactoricé para que sea un simple `onUserId` del lado de los componentes. Esa función vive en su propio archivo, `src/utils/onuserId.js`. La estoy usando en este PR y https://gitea.verdnatura.es/verdnatura/hedera-web/pulls/124/files.
jsegarra commented 2025-03-21 14:29:28 +00:00
Member
Copy Link

oki

oki
🎉 1
jsegarra requested changes 2025-03-12 22:34:38 +00:00
Dismissed
src/pages/Ecomerce/PendingOrders.vue
@ -38,0 +35,4 @@
const filter = {
where: {
clientFk,
'address.clientFk': clientFk,
jsegarra commented 2025-03-12 22:34:21 +00:00
Member
Copy Link

haces para que te traiga el objeto de address?
Quiero decir, la relacion ya está con clave primaria, por tanto no haria falta ponerlo en el where

haces para que te traiga el objeto de address? Quiero decir, la relacion ya está con clave primaria, por tanto no haria falta ponerlo en el where
ldragan commented 2025-03-13 04:49:34 +00:00
Author
Member
Copy Link

Coincido, pero así estaba la query y preferí no cambiarla. (La view myAddress tiene incluido el filtro de clientFk).

Por lo que veo en order.json, el join se hace via el address_id, y no hay una garantía real y determinística de que el address.clientFk coincida con el order.clientFk— ello depende de que nunca se haga un insert o update "incorrecto" en algún lugar del código, para alguna defición de "correcto".

Yo considero que remover esta parte del where sería un refactor, y no es estrictamente parte de la migración. Estoy a favor, pero suelo preferir el camino conservador, y encarar los dos cambios en 2 PRs separados, para simplificar el debugging en el remoto caso de que ocurra un error inesperado por alguna incongruencia en la data.

Por otro lado, lo más probable es que remover esta condición no cause ningún problema, y, aún si lo hiciera, el impacto debería ser mínimo, inmediatamente observable y fácil de resolver.

Si estás a favor de quitar el 'address.clientFk': clientFk,, entonces yo también :)

Coincido, pero así estaba la query y preferí no cambiarla. (La view `myAddress` tiene incluido el filtro de `clientFk`). Por lo que veo en `order.json`, el `join` se hace via el `address_id`, y no hay una garantía real y determinística de que el `address.clientFk` coincida con el `order.clientFk`— ello depende de que nunca se haga un `insert` o `update` "incorrecto" en algún lugar del código, para alguna defición de "correcto". Yo considero que remover esta parte del `where` sería un refactor, y no es estrictamente parte de la migración. Estoy a favor, pero suelo preferir el camino conservador, y encarar los dos cambios en 2 PRs separados, para simplificar el debugging en el remoto caso de que ocurra un error inesperado por alguna incongruencia en la data. Por otro lado, lo más probable es que remover esta condición no cause ningún problema, y, aún si lo hiciera, el impacto debería ser mínimo, inmediatamente observable y fácil de resolver. Si estás a favor de quitar el `'address.clientFk': clientFk,`, entonces yo también :)
jsegarra commented 2025-03-21 14:26:39 +00:00
Member
Copy Link

Dale con todo y quitemosla

Dale con todo y quitemosla
jsegarra commented 2025-03-21 14:29:52 +00:00
Member
Copy Link

y validamos con los tests

y validamos con los tests
ldragan commented 2025-03-22 15:10:08 +00:00
Author
Member
Copy Link

Awesome. Hecho :)

Awesome. Hecho :)
jsegarra marked this conversation as resolved
src/pages/Ecomerce/PendingOrders.vue
@ -38,0 +37,4 @@
clientFk,
'address.clientFk': clientFk,
isConfirmed: 0,
source_app: 'WEB',
jsegarra commented 2025-03-12 22:34:36 +00:00
Member
Copy Link

este source_app no lo veo en la query origen

este source_app no lo veo en la query origen
ldragan commented 2025-03-13 04:01:21 +00:00
Author
Member
Copy Link

La query original se hacía contra myOrder , que es una view:

MariaDB [(none)]> show create view `hedera`.`myOrder`;
*************************** 1. row ***************************
                View: myOrder
         Create View: CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `hedera`.`myOrder` AS select `o`.`id` AS `id`,`o`.`date_send` AS `sent`,`o`.`customer_id` AS `clientFk`,`o`.`delivery_method_id` AS `deliveryMethodFk`,`o`.`agency_id` AS `agencyModeFk`,`o`.`address_id` AS `addressFk`,`o`.`company_id` AS `companyFk`,`o`.`note` AS `notes`,`o`.`source_app` AS `sourceApp`,`o`.`confirmed` AS `isConfirmed`,`o`.`date_make` AS `created`,`o`.`first_row_stamp` AS `firstRowStamp`,`o`.`confirm_date` AS `confirmed`,`o`.`taxableBase` AS `taxableBase`,`o`.`tax` AS `tax`,`o`.`total` AS `total` from `hedera`.`order` `o` where `o`.`customer_id` = `account`.`myUser_getId`() and `o`.`source_app` = 'WEB' and `o`.`confirmed` = 0 WITH CASCADED CHECK OPTION
character_set_client: utf8mb4
collation_connection: utf8mb4_unicode_ci
1 row in set (0.156 sec)

La view incluye el source_app y otros filtros:

 where `o`.`customer_id` = `account`.`myUser_getId`() and `o`.`source_app` = 'WEB' and `o`.`confirmed` = 0
La query original se hacía contra `myOrder` , que es una view: ``` MariaDB [(none)]> show create view `hedera`.`myOrder`; *************************** 1. row *************************** View: myOrder Create View: CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `hedera`.`myOrder` AS select `o`.`id` AS `id`,`o`.`date_send` AS `sent`,`o`.`customer_id` AS `clientFk`,`o`.`delivery_method_id` AS `deliveryMethodFk`,`o`.`agency_id` AS `agencyModeFk`,`o`.`address_id` AS `addressFk`,`o`.`company_id` AS `companyFk`,`o`.`note` AS `notes`,`o`.`source_app` AS `sourceApp`,`o`.`confirmed` AS `isConfirmed`,`o`.`date_make` AS `created`,`o`.`first_row_stamp` AS `firstRowStamp`,`o`.`confirm_date` AS `confirmed`,`o`.`taxableBase` AS `taxableBase`,`o`.`tax` AS `tax`,`o`.`total` AS `total` from `hedera`.`order` `o` where `o`.`customer_id` = `account`.`myUser_getId`() and `o`.`source_app` = 'WEB' and `o`.`confirmed` = 0 WITH CASCADED CHECK OPTION character_set_client: utf8mb4 collation_connection: utf8mb4_unicode_ci 1 row in set (0.156 sec) ``` La view incluye el `source_app` y otros filtros: ``` where `o`.`customer_id` = `account`.`myUser_getId`() and `o`.`source_app` = 'WEB' and `o`.`confirmed` = 0 ```
jsegarra commented 2025-03-13 08:09:42 +00:00
Member
Copy Link

Comentado por @alexm
Puedes definirte una vista como un modelo normal
modules/worker/back/models/worker-department.json por ejemplo, es una vista

Comentado por @alexm Puedes definirte una vista como un modelo normal modules/worker/back/models/worker-department.json por ejemplo, es una vista
ldragan commented 2025-03-22 15:15:50 +00:00
Author
Member
Copy Link

For the record, esto lo charlamos en la daily, y quedamos en ver si al equipo le parece bien dejar esto así, o hacer un cambio mayor, para agregar un modelo loopback sobre una view.

Mi argumento a favor de dejar esto así es que, al largo plazo, lo mejor sería directamente ya no usar las views, y usar completamente lo que ofrece loopback (hacerlo "the loopback way", digamos).

Por otro lado, la view incluye la llamada al myUser_getId(), el antiguo método de autorización, que, considero, deberíamos apuntar a eliminar completamente. Usar la view implica usar una transacción de la DB y ejecutar un 'CALL account.myUser_loginWithName((SELECT name FROM account.user WHERE id = ?))',, como lo hace VnModel.rawSql. Perdemos casi todos los beneficios de loopback.

For the record, esto lo charlamos en la daily, y quedamos en ver si al equipo le parece bien dejar esto así, o hacer un cambio mayor, para agregar un modelo loopback sobre una view. Mi argumento a favor de dejar esto así es que, al largo plazo, lo mejor sería directamente ya no usar las views, y usar completamente lo que ofrece loopback (hacerlo "the loopback way", digamos). Por otro lado, la view incluye la llamada al `myUser_getId()`, el antiguo método de autorización, que, considero, deberíamos apuntar a eliminar completamente. Usar la view implica usar una transacción de la DB y ejecutar un `'CALL account.myUser_loginWithName((SELECT name FROM account.user WHERE id = ?))',`, como lo hace `VnModel.rawSql`. Perdemos casi todos los beneficios de loopback.
ldragan added 1 commit 2025-03-21 02:55:58 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
afbbbace4b refactor: onUserId
ldragan added 1 commit 2025-03-22 15:09:44 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
89de158514 refactor: remove redundant `where` filter clause
ldragan added 1 commit 2025-03-22 15:27:37 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
64582e99c7 Merge branch 'beta' into taro/pending-orders
ldragan requested review from jsegarra 2025-03-22 15:28:07 +00:00
jsegarra requested changes 2025-03-23 22:22:57 +00:00
Dismissed
jsegarra left a comment
Member
Copy Link

Lo veo bien, pero me surge la duda en la linea 88...mantenemos una SQL? No deberiamos usar la llamada axios a salix?

Lo veo bien, pero me surge la duda en la linea 88...mantenemos una SQL? No deberiamos usar la llamada axios a salix?
ldragan added 1 commit 2025-03-25 07:08:50 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
859ffbd1fd fix(PendingOrders): migrate removeOrder, too
ldragan commented 2025-03-25 07:10:42 +00:00
Author
Member
Copy Link

Lo veo bien, pero me surge la duda en la linea 88...mantenemos una SQL? No deberiamos usar la llamada axios a salix?

Done ✅

> Lo veo bien, pero me surge la duda en la linea 88...mantenemos una SQL? No deberiamos usar la llamada axios a salix? Done ✅
jsegarra reviewed 2025-03-25 08:21:54 +00:00
src/pages/Ecomerce/PendingOrders.vue
@ -52,3 +78,2 @@
}
);
await api.delete(`Orders/${id}`);
orders.value.splice(index, 1);
jsegarra commented 2025-03-25 08:21:54 +00:00
Member
Copy Link

Lo veo bien, pero cuando tengamos un respiro miraria de hacer algo para que refresque haciendo un refetch por ejemplo. https://gitea.verdnatura.es/verdnatura/salix-front/src/branch/dev/src/components/VnTable/VnTable.vue#L311

Lo veo bien, pero cuando tengamos un respiro miraria de hacer algo para que refresque haciendo un refetch por ejemplo. https://gitea.verdnatura.es/verdnatura/salix-front/src/branch/dev/src/components/VnTable/VnTable.vue#L311
jsegarra requested changes 2025-03-25 08:29:12 +00:00
Dismissed
src/pages/Ecomerce/PendingOrders.vue Outdated
@ -38,0 +32,4 @@
const filter = {
where: {
clientFk,
isConfirmed: 0,
jsegarra commented 2025-03-25 08:28:51 +00:00
Member
Copy Link

a nivel de BD funciona bien, pero como lo consultamos desde el front, mejor aplicamos del standard Booleano true/false

a nivel de BD funciona bien, pero como lo consultamos desde el front, mejor aplicamos del standard Booleano true/false
src/utils/onUserId.js Outdated
@ -0,0 +7,4 @@
export const onUserId = (cb) => watch(
() => userStore?.user?.id,
async userId => {
if (userId) {
jsegarra commented 2025-03-25 08:24:15 +00:00
Member
Copy Link

Usando clausulas de guarda podemos quitar un nivel de indexación
if (!userId) return;

Usando clausulas de guarda podemos quitar un nivel de indexación ` if (!userId) return;`
❤️ 2
ldragan added 1 commit 2025-03-25 08:50:16 +00:00
gitea/hedera-web/pipeline/pr-beta This commit looks good Details
512b0b5e9c refactor(PendingOrders): address PR comments
ldragan requested review from jsegarra 2025-03-27 03:35:20 +00:00
jsegarra approved these changes 2025-03-27 05:14:19 +00:00
jsegarra merged commit bae2de2478 into beta 2025-03-27 05:15:04 +00:00
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#121
No description provided.
Delete Branch "ldragan/hedera-web:taro/pending-orders"

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?