#7356 - Ticket filter #1530

Open
jsegarra wants to merge 4 commits from warmfix_ticketFilter into test
7 changed files with 182 additions and 24 deletions

View File

@ -1,5 +1,5 @@
<script setup>
import { ref, computed } from 'vue';
import { ref, computed, toRef } from 'vue';
import { useI18n } from 'vue-i18n';
import { useArrayData } from 'composables/useArrayData';
import toDate from 'filters/toDate';
@ -61,6 +61,14 @@ const $props = defineProps({
type: Object,
default: null,
},
validations: {
type: Array,
default: () => [],
},
excludeParams: {
type: Object,
default: null,
},
});
const emit = defineEmits([
@ -84,18 +92,40 @@ const arrayData =
const store = arrayData.store;
const userParams = ref(useFilterParams($props.dataKey).params);
const userOrders = ref(useFilterParams($props.dataKey).orders);
const isLoading = ref(false);
const excludeParams = toRef($props.excludeParams);
defineExpose({ search, params: userParams, remove });
const isLoading = ref(false);
async function search(evt) {
try {
const validations = $props.validations.every((validation) => {
return validation(userParams.value);
});
if (!validations) {
return;
}
if (Object.keys(userParams.value).length) {
excludeParams.value = null;
evt = { ...evt?.params };
} else if ($props.excludeParams) {
excludeParams.value = $props.excludeParams;
}
if (evt && $props.disableSubmitEvent) return;
store.filter.where = {};
isLoading.value = true;
const filter = { ...userParams.value, ...$props.modelValue };
let filter = { ...userParams.value, ...$props.modelValue, ...evt };
store.userParamsChanged = true;
if (excludeParams.value) {
filter = {
...filter.params,
exclude: excludeParams.value,
};
}
await arrayData.addFilter({
params: filter,
});
@ -201,6 +231,7 @@ const getLocale = (label) => {
</script>
<template>
{{ excludeParams }}
<QBtn
class="q-mt-lg q-mr-xs q-mb-lg"
round
@ -222,6 +253,7 @@ const getLocale = (label) => {
</QItemLabel>
</QItemSection>
<QItemSection top side>
{{ excludeParams }}
<QBtn
@click="clearFilters"
color="primary"

View File

@ -69,6 +69,10 @@ const props = defineProps({
type: Boolean,
default: true,
},
filterPanel: {
type: Object,
default: true,
},
});
const searchText = ref();
@ -85,6 +89,7 @@ if (props.redirect)
};
let arrayData = useArrayData(props.dataKey, arrayDataProps);
let store = arrayData.store;
const filterPanel = ref(props.filterPanel);
const to = computed(() => {
const url = { path: route.path, query: { ...(route.query ?? {}) } };
const searchUrl = arrayData.store.searchUrl;
@ -104,6 +109,12 @@ watch(
store = arrayData.store;
},
);
watch(
() => props.filterPanel,
(val) => {
filterPanel.value = val;
},
);
onMounted(() => {
const params = store.userParams;
@ -116,7 +127,10 @@ async function search() {
arrayData.resetPagination();
let filter = { params: { search: searchText.value } };
if (filterPanel?.value?.filterPanelRef) {
Review

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?
Review

no lo veo
un ejemplo?

no lo veo un ejemplo?
Review
  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
Review

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.
Review

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
filterPanel.value.filterPanelRef.search(filter);
return;
}
if (!props.searchRemoveParams || !searchText.value) {
filter = {
params: {

View File

@ -75,12 +75,13 @@ export function useArrayData(key, userOptions) {
}
}
async function fetch({ append = false, updateRouter = true }) {
async function fetch(fetchOptions) {
let { append = false, updateRouter = true } = fetchOptions ?? {};
if (!store.url) return;
cancelRequest();
canceller = new AbortController();
const { params, limit } = setCurrentFilter();
let { params, limit } = setCurrentFilter();
let exprFilter;
if (store?.exprBuilder) {
@ -98,7 +99,11 @@ export function useArrayData(key, userOptions) {
if (!params?.filter?.order?.length) delete params?.filter?.order;
params.filter = JSON.stringify(params.filter);
if (fetchOptions?.exclude) {
delete params?.exclude;
delete params?.params?.exclude;
params = { ...params, ...fetchOptions.exclude };
}
store.isLoading = true;
const response = await axios.get(store.url, {
signal: canceller.signal,
@ -150,6 +155,10 @@ export function useArrayData(key, userOptions) {
async function applyFilter({ filter, params }, fetchOptions = {}) {
if (filter) store.userFilter = filter;
store.filter = {};
if (params?.exclude) {
fetchOptions = { ...fetchOptions, exclude: params.exclude };
delete params.exclude;
}
if (params) store.userParams = { ...params };
const response = await fetch(fetchOptions);
@ -158,14 +167,19 @@ export function useArrayData(key, userOptions) {
async function addFilter({ filter, params }) {
if (filter) store.filter = filter;
let exclude = {};
if (params?.exclude) {
exclude = params.exclude;
delete params.exclude;
}
let userParams = { ...store.userParams, ...params };
userParams = sanitizerParams(userParams, store?.exprBuilder);
store.userParams = userParams;
resetPagination();
await fetch({});
await fetch({ exclude });
return { filter, params };
}

View File

@ -1,6 +1,7 @@
<script setup>
import { ref } from 'vue';
import { ref, computed } from 'vue';
import { useI18n } from 'vue-i18n';
import { useRoute } from 'vue-router';
import FetchData from 'components/FetchData.vue';
import VnFilterPanel from 'src/components/ui/VnFilterPanel.vue';
@ -8,6 +9,7 @@ import VnInput from 'src/components/common/VnInput.vue';
import VnInputDate from 'components/common/VnInputDate.vue';
import VnSelect from 'src/components/common/VnSelect.vue';
import VnSelectWorker from 'src/components/common/VnSelectWorker.vue';
import useNotify from 'src/composables/useNotify';
const { t } = useI18n();
const props = defineProps({
@ -16,13 +18,34 @@ const props = defineProps({
required: true,
},
});
const route = useRoute();
const userParams = {
from: null,
to: null,
};
const filterPanelRef = ref(null);
defineExpose({ filterPanelRef });
const provinces = ref([]);
const states = ref([]);
const agencies = ref([]);
const warehouses = ref([]);
const groupedStates = ref([]);
const { notify } = useNotify();
const from = ref(Date.vnNew());
const to = ref(Date.vnNew());
const initializeFromQuery = computed(() => {
Review

from y to no son ref

from y to no son **ref**
Review

Copy paste, pero lo modifico

Copy paste, pero lo modifico
Review

b5549dae8813235e9be27aebcddafdb07d05ad88

b5549dae8813235e9be27aebcddafdb07d05ad88
from.value.setHours(0, 0, 0, 0);
from.value.setDate(from.value.getDate() - 7);
to.value.setHours(23, 59, 0, 0);
to.value.setDate(to.value.getDate() + 1);
const query = route.query.table ? JSON.parse(route.query.table) : {};
Object.assign(userParams, {
from: query.from || from.value.toISOString(),
to: query.to || to.value.toISOString(),
});
return userParams;
});
const getGroupedStates = (data) => {
for (const state of data) {
groupedStates.value.push({
@ -32,6 +55,17 @@ const getGroupedStates = (data) => {
});
}
};
function validateDateRange(params) {
const hasFrom = 'from' in params;
const hasTo = 'to' in params;
jorgep marked this conversation as resolved
Review

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?
Review

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
if (hasFrom !== hasTo) {
notify(t(`dateRangeMustHaveBothFrom`), 'negative');
}
Review

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.
Review

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
Review

(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)**.
Review

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?
Review

(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.
return (hasFrom && hasTo) || (!hasFrom && !hasTo);
}
</script>
<template>
@ -53,7 +87,13 @@ const getGroupedStates = (data) => {
auto-load
/>
<FetchData url="Warehouses" @on-fetch="(data) => (warehouses = data)" auto-load />
<VnFilterPanel :data-key="props.dataKey" :search-button="true">
<VnFilterPanel
ref="filterPanelRef"
:data-key="props.dataKey"
:search-button="true"
:validations="[validateDateRange]"
:exclude-params="initializeFromQuery"
>
<template #tags="{ tag, formatFn }">
<div class="q-gutter-x-xs">
<strong>{{ t(`params.${tag.label}`) }}: </strong>
@ -301,6 +341,7 @@ const getGroupedStates = (data) => {
<i18n>
en:
dateRangeMustHaveBothFrom: The date range must have both 'from' and 'to'
params:
search: Contains
clientFk: Customer
@ -329,6 +370,7 @@ en:
DELIVERED: Delivered
ON_PREVIOUS: ON_PREVIOUS
es:
dateRangeMustHaveBothFrom: El rango de fechas debe tener 'desde' y 'hasta'
params:
search: Contiene
clientFk: Cliente

View File

@ -43,12 +43,8 @@ from.setDate(from.getDate() - 7);
const to = Date.vnNew();
to.setHours(23, 59, 0, 0);
to.setDate(to.getDate() + 1);
const userParams = {
from: null,
to: null,
};
onBeforeMount(() => {
initializeFromQuery();
stateStore.rightDrawer = true;
});
onMounted(async () => {
@ -79,6 +75,7 @@ const companiesOptions = ref([]);
const accountingOptions = ref([]);
const amountToReturn = ref();
const dataKey = 'TicketList';
const filterPanelRef = ref(null);
const formInitialData = ref({});
const columns = computed(() => [
@ -483,11 +480,17 @@ function setReference(data) {
:array-data-props="{
url: 'Tickets/filter',
order: ['shippedDate DESC', 'shippedHour ASC', 'zoneLanding ASC', 'id'],
filterPanel: filterPanelRef,
searchRemoveParams: true,
exprBuilder,
}"
>
<template #advanced-menu>
<TicketFilter data-key="TicketList" />
<TicketFilter
ref="filterPanelRef"
data-key="TicketList"
:excludeParams="{ ...userParams }"
/>
</template>
<template #body>
<VnTable
@ -501,7 +504,6 @@ function setReference(data) {
}"
default-mode="table"
:columns="columns"
:user-params="userParams"
:right-search="false"
redirect="ticket"
v-model:selected="selectedRows"
@ -581,7 +583,7 @@ function setReference(data) {
:options="clientsOptions"
hide-selected
required
@update:model-value="() => onClientSelected(data)"
@update:model-value="onClientSelected(data)"
:sort-by="'id ASC'"
>
<template #option="scope">

View File

@ -6,10 +6,39 @@ describe('TicketFilter', () => {
cy.visit('/#/ticket/list');
});
it('use search button', function () {
it('use search button', () => {
cy.waitForElement('.q-page');
cy.get('[data-cy="Customer ID_input"]').type('1105');
cy.intercept('GET', /\/api\/Tickets\/filter/).as('ticketFilter');
cy.searchBtnFilterPanel();
cy.location('href').should('contain', '#/ticket/15/summary');
cy.waitRequest('@ticketFilter', ({ request }) => {
const { query } = request;
expect(query).to.have.property('from');
expect(query).to.have.property('to');
});
// cy.on('uncaught:exception', () => {
// return false;
// });
cy.dataCy('From_inputDate').type(`${today()} `).type('{enter}');
cy.get('.q-notification').should(
'contain',
`The date range must have both 'from' and 'to'`,
);
cy.dataCy('To_inputDate').type(`${today()}{enter}`);
cy.intercept('GET', /\/api\/Tickets\/filter/).as('ticketFilter');
cy.searchBtnFilterPanel();
cy.wait('@ticketFilter').then(({ request }) => {
const { query } = request;
expect(query).to.have.property('from');
expect(query).to.have.property('to');
});
cy.location('href').should('contain', '#/ticket/999999');
});
});
function today(date) {
return new Intl.DateTimeFormat('es-ES', {
day: '2-digit',
month: '2-digit',
year: 'numeric',
}).format(date ?? new Date());
}

View File

@ -35,6 +35,31 @@ describe('TicketList', () => {
cy.get('.summaryBody').should('exist');
});
it('filter client and create ticket', () => {
cy.intercept('GET', /\/api\/Tickets\/filter/).as('ticketSearchbar');
searchResults();
cy.wait('@ticketSearchbar').then(({ request }) => {
Review

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
const { query } = request;
expect(query).to.have.property('from');
expect(query).to.have.property('to');
expect(query).to.not.have.property('clientFk');
});
cy.intercept('GET', /\/api\/Tickets\/filter/).as('ticketFilter');
cy.dataCy('Customer ID_input').clear('1');
cy.dataCy('Customer ID_input').type('1101{enter}');
cy.wait('@ticketFilter').then(({ request }) => {
const { query } = request;
expect(query).to.not.have.property('from');
expect(query).to.not.have.property('to');
expect(query).to.have.property('clientFk');
});
cy.get('[data-cy="vnTableCreateBtn"] > .q-btn__content > .q-icon').click();
cy.dataCy('Customer_select').should('have.value', 'Bruce Wayne');
cy.dataCy('Address_select').click();
cy.getOption().click();
cy.dataCy('Address_select').should('have.value', 'Bruce Wayne');
});
it('filter client and create ticket', () => {
cy.intercept('GET', /\/api\/Tickets\/filter/).as('ticketSearchbar');
searchResults();
@ -66,7 +91,7 @@ describe('TicketList', () => {
cy.url().should('match', /\/ticket\/\d+\/summary/);
});
it('should show the correct problems', () => {
it.only('should show the correct problems', () => {
cy.intercept('GET', '**/api/Tickets/filter*', (req) => {
req.headers['cache-control'] = 'no-cache';
req.headers['pragma'] = 'no-cache';
@ -81,7 +106,7 @@ describe('TicketList', () => {
cy.get('[data-cy="Warehouse_select"]').type('Warehouse Five');
cy.get('.q-menu .q-item').contains('Warehouse Five').click();
cy.wait('@ticket').then((interception) => {
const data = interception.response.body[1];
const data = interception.response.body.at(-1);
expect(data.hasComponentLack).to.equal(1);
expect(data.isTooLittle).to.equal(1);
expect(data.hasItemShortage).to.equal(1);