#7356 - Ticket filter #1530

Open
jsegarra wants to merge 4 commits from warmfix_ticketFilter into test
Member

He traído la lógica del filtrado de la PR #1405

He traído la lógica del filtrado de la PR https://gitea.verdnatura.es/verdnatura/salix-front/pulls/1405
jsegarra added 2 commits 2025-03-02 22:25:00 +00:00
gitea/salix-front/pipeline/pr-test This commit looks good Details
547426cf87
fix: arrayData chip
jsegarra requested review from alexm 2025-03-02 22:25:09 +00:00
jsegarra requested review from jorgep 2025-03-02 22:25:09 +00:00
jorgep requested changes 2025-03-05 08:56:50 +00:00
@ -85,2 +93,4 @@
const userParams = ref(useFilterParams($props.dataKey).params);
const userOrders = ref(useFilterParams($props.dataKey).orders);
const isLoading = ref(false);
const excludeParams = ref($props.excludeParams);
Member

Usar toRef .

Copilot dice:
Usar ref directamente en $props no es la mejor práctica porque $props ya es reactivo por defecto en Vue 3. Si necesitas una referencia reactiva a una propiedad específica de $props, toRef es más adecuado. Sin embargo, si decides usar ref, tendrías que hacer una copia de la propiedad para que sea reactiva, lo cual no es necesario con toRef.

Usar **toRef** . Copilot dice: _Usar ref directamente en $props no es la mejor práctica porque $props ya es reactivo por defecto en Vue 3. Si necesitas una referencia reactiva a una propiedad específica de $props, toRef es más adecuado. Sin embargo, si decides usar ref, tendrías que hacer una copia de la propiedad para que sea reactiva, lo cual no es necesario con toRef._
Author
Member

b5549dae8813235e9be27aebcddafdb07d05ad88

b5549dae8813235e9be27aebcddafdb07d05ad88
@ -95,3 +116,3 @@
store.filter.where = {};
isLoading.value = true;
const filter = { ...userParams.value, ...$props.modelValue };
const filter = { ...userParams.value, ...$props.modelValue, ...evt };
Member

No veo donde usas el evt.

No veo donde usas el **evt**.
jorgep marked this conversation as resolved
@ -117,3 +128,3 @@
let filter = { params: { search: searchText.value } };
if (filterPanel?.value?.filterPanelRef) {
Member

No sería más fácil que crearas un composable useFilterPanelSearch ? De todas maneras, creo que lo ideal sería que el searchbar y el filterPanel usen la misma lógica(respecto a arrayData) ya que ambos llaman al mismo back. Incluso crearía un solo composable para ambos. @alexm ¿cómo lo ves?

No sería más fácil que crearas un composable **useFilterPanelSearch** ? De todas maneras, creo que lo ideal sería que el searchbar y el filterPanel usen la misma lógica(respecto a arrayData) ya que ambos llaman al mismo back. Incluso crearía un solo composable para ambos. @alexm ¿cómo lo ves?
Author
Member

no lo veo
un ejemplo?

no lo veo un ejemplo?
Author
Member
  1. VnSarchbar y VnFilterPanel, son componentes independientes que sus funciones de search no se parecen entre si, por lo que tener un composable que tenga la función search valida para ambos lo veo, complicado, peligroso e innecesario.
  2. Si lo que se quiere hacer es un refactor, esta tarea no tiene ese objetivo
  3. "creo que lo ideal sería que el searchbar y el filterPanel usen la misma lógica(respecto a arrayData) ya que ambos llaman al mismo back"...nos sentamos a revisarlo juntos porque searchbar está llamando a su lógica y solo cuando se le indica, casos específicos, que use filterpanel lo hace, que es el caso de TicketFilter.
  4. Por otra parte, searchbar no se le está modificando el comportamiento con respecto a arrayData
1. VnSarchbar y VnFilterPanel, son componentes independientes que sus funciones de search no se parecen entre si, por lo que tener un composable que tenga la función search valida para ambos lo veo, complicado, peligroso e innecesario. 2. Si lo que se quiere hacer es un refactor, esta tarea no tiene ese objetivo 3. "creo que lo ideal sería que el searchbar y el filterPanel usen la misma lógica(respecto a arrayData) ya que ambos llaman al mismo back"...nos sentamos a revisarlo juntos porque searchbar está llamando a su lógica y solo cuando se le indica, casos específicos, que use filterpanel lo hace, que es el caso de TicketFilter. 4. *Por otra parte*, searchbar no se le está modificando el comportamiento con respecto a arrayData
Member

No veo lo de pasar el componente como propiedad, que te diga @alexm su opinión.

No veo lo de pasar el componente como propiedad, que te diga @alexm su opinión.
Author
Member

Ni yo, pero por el ciclo de vida de VnSection, provide/inject no se puede hacer
Vamos a hacer de meter el TicketFilter dentro de VnSection

Ni yo, pero por el ciclo de vida de VnSection, provide/inject no se puede hacer Vamos a hacer de meter el TicketFilter dentro de VnSection
@ -160,2 +169,3 @@
if (filter) store.filter = filter;
let exclude = {};
if (params?.params?.exclude) {
Member

Por qué params.params?

Por qué params.params?
Author
Member

SOLVED en b5549dae8813235e9be27aebcddafdb07d05ad88

SOLVED en b5549dae8813235e9be27aebcddafdb07d05ad88
@ -26,0 +34,4 @@
const { notify } = useNotify();
const initializeFromQuery = computed(() => {
const query = route.query.table ? JSON.parse(route.query.table) : {};
from.value = query.from || from.toISOString();
Member

from y to no son ref

from y to no son **ref**
Author
Member

Copy paste, pero lo modifico

Copy paste, pero lo modifico
Author
Member

b5549dae8813235e9be27aebcddafdb07d05ad88

b5549dae8813235e9be27aebcddafdb07d05ad88
@ -35,0 +58,4 @@
const hasFrom = 'from' in params;
const hasTo = 'to' in params;
if (hasFrom !== hasTo) {
Member

Comentario, no sería mejor lanzar error? y así en caso de ser erróneo el usuario puede abrir un cau?

Comentario, no sería mejor lanzar error? y así en caso de ser erróneo el usuario puede abrir un cau?
Author
Member

Pero no es un error para abrir un cau sino es una validación previa a llamar a search
La función se llama validate

Pero no es un error para abrir un cau sino es una validación previa a llamar a search La función se llama validate
jorgep marked this conversation as resolved
@ -35,0 +62,4 @@
notify(t(`dateRangeMustHaveBothFrom`), 'negative');
}
return (hasFrom && hasTo) || (!hasFrom && !hasTo);
Member

return hasFrom === hasTo , yo lo guardaría en una variable y así lo puedes usar en el if también.

**return hasFrom === hasTo** , yo lo guardaría en una variable y así lo puedes usar en el if también.
Author
Member

Son 3 validaciones distintas

  1. Cuando son diferentes
  2. Cuando son iguales
  3. Cuando son null las 2
Son 3 validaciones distintas 1. Cuando son diferentes 2. Cuando son iguales 3. Cuando son null las 2
Member

(hasFrom && hasTo) || (!hasFrom && !hasTo) es lo mismo que hasFrom === hasTo y hasFrom !== hasTo también es lo mismo que !(hasFrom === hasTo).

**(hasFrom && hasTo) || (!hasFrom && !hasTo)** es lo mismo que **hasFrom === hasTo** y **hasFrom !== hasTo** también es lo mismo que **!(hasFrom === hasTo)**.
Author
Member

has es booleano
comparamos booleanos? o se entiende mejor de esta manera?
En un if no haces hasFrom === true, no?

has es booleano comparamos booleanos? o se entiende mejor de esta manera? En un if no haces hasFrom === true, no?
@ -543,3 +538,3 @@
hide-selected
required
@update:model-value="(client) => onClientSelected(data)"
@update:model-value="() => onClientSelected(data)"
Member

Es posible no tener que usar la fn de flecha¿?

Es posible no tener que usar la fn de flecha¿?
jsegarra marked this conversation as resolved
@ -0,0 +16,4 @@
expect(query).to.have.property('from');
expect(query).to.have.property('to');
});
cy.on('uncaught:exception', () => {
Member

Estas ignorando un error no capturado? porque?

Estas ignorando un error no capturado? porque?
jsegarra marked this conversation as resolved
@ -38,6 +38,31 @@ describe('TicketList', () => {
cy.get('.summaryBody').should('exist');
});
it('filter client and create ticket', () => {
Member

Veo bien que compruebes los parametros que se envian en la solicitud, pero también creo que es importante comprobar que en el DOM se ha actualizado con los valores correctos. Ej. ver solo los tickets del 1 al 30 de febrero

Veo bien que compruebes los parametros que se envian en la solicitud, pero también creo que es importante comprobar que en el DOM se ha actualizado con los valores correctos. Ej. ver solo los tickets del 1 al 30 de febrero
jsegarra added 2 commits 2025-03-06 15:59:38 +00:00
gitea/salix-front/pipeline/pr-test This commit is unstable Details
dcca9c9d34
fix: comments
jsegarra requested review from jorgep 2025-03-06 23:37:54 +00:00
jorgep requested changes 2025-03-07 08:23:05 +00:00
@ -35,0 +63,4 @@
if (hasFrom !== hasTo) {
notify(t(`dateRangeMustHaveBothFrom`), 'negative');
}
Member

(hasFrom && hasTo) || (!hasFrom && !hasTo) es lo mismo que esto: hasFrom === hasTo?

GitHub Copilot
Sí, la expresión (hasFrom && hasTo) || (!hasFrom && !hasTo) es lógicamente equivalente a hasFrom === hasTo.

Ambas expresiones verifican si hasFrom y hasTo son ambos verdaderos o ambos falsos. Aquí está la explicación:

(hasFrom && hasTo): Esto es verdadero si ambos hasFrom y hasTo son verdaderos.
(!hasFrom && !hasTo): Esto es verdadero si ambos hasFrom y hasTo son falsos.
La expresión completa (hasFrom && hasTo) || (!hasFrom && !hasTo) es verdadera si ambos son verdaderos o ambos son falsos.
La expresión hasFrom === hasTo también es verdadera si ambos son verdaderos o ambos son falsos, ya que compara directamente los valores de hasFrom y hasTo.

Por lo tanto, puedes simplificar (hasFrom && hasTo) || (!hasFrom && !hasTo) a hasFrom === hasTo.

(hasFrom && hasTo) || (!hasFrom && !hasTo) es lo mismo que esto: hasFrom === hasTo? GitHub Copilot Sí, la expresión (hasFrom && hasTo) || (!hasFrom && !hasTo) es lógicamente equivalente a hasFrom === hasTo. Ambas expresiones verifican si hasFrom y hasTo son ambos verdaderos o ambos falsos. Aquí está la explicación: (hasFrom && hasTo): Esto es verdadero si ambos hasFrom y hasTo son verdaderos. (!hasFrom && !hasTo): Esto es verdadero si ambos hasFrom y hasTo son falsos. La expresión completa (hasFrom && hasTo) || (!hasFrom && !hasTo) es verdadera si ambos son verdaderos o ambos son falsos. La expresión hasFrom === hasTo también es verdadera si ambos son verdaderos o ambos son falsos, ya que compara directamente los valores de hasFrom y hasTo. Por lo tanto, puedes simplificar (hasFrom && hasTo) || (!hasFrom && !hasTo) a hasFrom === hasTo.
Some checks reported warnings
gitea/salix-front/pipeline/pr-test This commit is unstable
Required
Details
This pull request has changes conflicting with the target branch.
  • src/components/ui/VnFilterPanel.vue
  • src/composables/useArrayData.js
  • src/pages/Ticket/TicketFilter.vue
  • src/pages/Ticket/TicketList.vue
  • test/cypress/integration/ticket/ticketList.spec.js
Sign in to join this conversation.
No reviewers
No Milestone
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/salix-front#1530
No description provided.