Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the filter params #7051

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ui/src/components/filter/KestraFilter.vue
Original file line number Diff line number Diff line change
Expand Up @@ -599,13 +599,15 @@

const triggerSearch = () => {
if (props.searchCallback) return;
else router.push({query: encodeParams(currentFilters.value, OPTIONS)});
else {
router.push({query: encodeParams(route.path, currentFilters.value, OPTIONS)});
}
};

// Include parameters from URL directly to filter
onMounted(() => {
if (props.decode) {
const decodedParams = decodeParams(route.query, props.include, OPTIONS);
const decodedParams = decodeParams(route.path, route.query, props.include, OPTIONS);
currentFilters.value = decodedParams.map((item: any) => {
if (item.label === "absolute_date") {
return {
Expand Down Expand Up @@ -652,7 +654,7 @@
currentFilters.value.push({
label: "flow",
value: [`${params.id}`],
comparator: COMPARATORS.IS,
comparator: COMPARATORS.EQUALS,
persistent: true,
});
}
Expand Down
4 changes: 3 additions & 1 deletion ui/src/components/filter/components/Label.vue
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
const {value, label, comparator} = props.option;

if (!value.length) return;

if (label === "labels") {
MilosPaunovic marked this conversation as resolved.
Show resolved Hide resolved
return Array.isArray(value) && value.length === 1 ? value[0] : value;
}
if (label !== "absolute_date" && comparator?.label !== "between") {
return `${value.join(", ")}`;
}
Expand Down
63 changes: 30 additions & 33 deletions ui/src/components/filter/composables/useFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,25 @@ const filterItems = (items: object[], element: object) => {
return items.filter((item) => compare(item, element));
};

const buildComparator = (value: string, multiple = false): Comparator => {
return {label: value, value, multiple};
const buildComparator = (label: string, value: string, multiple: boolean = false): Comparator => {
return {label, value, multiple};
};

export function useFilters(prefix: string) {
const {t} = useI18n({useScope: "global"});

const comparator = (which: string) => `filters.comparators.${which}`;

const COMPARATORS: Record<string, Comparator> = {
IS: buildComparator(t(comparator("is"))),
IS_ONE_OF: buildComparator(t(comparator("is_one_of")), true),

IS_NOT: buildComparator(t(comparator("is_not"))),
IS_NOT_ONE_OF: buildComparator(t(comparator("is_not_one_off")), true),

CONTAINS: buildComparator(t(comparator("contains")), true),
NOT_CONTAINS: buildComparator(t(comparator("not_contains")), true),

IN: buildComparator(t(comparator("in"))),
BETWEEN: buildComparator(t(comparator("between"))),
STARTS_WITH: buildComparator(t(comparator("starts_with"))),
EQUALS: buildComparator(t(comparator("is")), "$eq"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially, might be a good idea to uniform the expressions - to not show label IS for EQUALS, but the EQUALS itself.

Although, this is more of a question for a product team, so pinging @Ben8t here for an opinion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure to understand: does it shows "IS" here? Can you give me a bit more context please?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the comparator chooser of filters:

image

For example, what is right now IS is going to be sent to backend as EQUALS when this is merged. Do we want to align the wording on that, so it's not labeled as IS anymore, but as EQUALS?

NOT_EQUALS: buildComparator(t(comparator("is_not")), "$ne"),
CONTAINS: buildComparator(t(comparator("contains")), "$contains", true),
STARTS_WITH: buildComparator(t(comparator("starts_with")), "$startsWith"),
ENDS_WITH: buildComparator(t(comparator("ends_with")), "$endsWith"),
IN: buildComparator(t(comparator("in")), "$in", true),
NOT_IN: buildComparator(t(comparator("is_not_one_off")), "$notIn", true),
BETWEEN: buildComparator(t(comparator("between")), "$between"),
GREATER_THAN: buildComparator(t(comparator("greater_than")), "$gt"),
LESS_THAN: buildComparator(t(comparator("less_than")), "$lt"),
};

const OPTIONS: Option[] = [
Expand All @@ -50,91 +47,91 @@ export function useFilters(prefix: string) {
icon: ICONS.DotsSquare,
label: t("filters.options.namespace"),
value: {label: "namespace", comparator: undefined, value: []},
comparators: [COMPARATORS.STARTS_WITH],
comparators: [COMPARATORS.STARTS_WITH, COMPARATORS.CONTAINS],
},
{
key: "state",
icon: ICONS.StateMachine,
label: t("filters.options.state"),
value: {label: "state", comparator: undefined, value: []},
comparators: [COMPARATORS.IS_ONE_OF],
comparators: [COMPARATORS.IN, COMPARATORS.NOT_IN],
},
{
key: "trigger_state",
icon: ICONS.StateMachine,
label: t("filters.options.state"),
value: {label: "trigger_state", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "scope",
icon: ICONS.FilterSettingsOutline,
label: t("filters.options.scope"),
value: {label: "scope", comparator: undefined, value: []},
comparators: [COMPARATORS.IS_ONE_OF],
comparators: [COMPARATORS.EQUALS, COMPARATORS.NOT_EQUALS],
},
{
key: "childFilter",
icon: ICONS.FilterVariantMinus,
label: t("filters.options.child"),
value: {label: "child", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS, COMPARATORS.NOT_EQUALS],
},
{
key: "level",
icon: ICONS.MathLog,
label: t("filters.options.level"),
value: {label: "level", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS, COMPARATORS.NOT_EQUALS],
},
{
key: "task",
icon: ICONS.TimelineTextOutline,
label: t("filters.options.task"),
value: {label: "task", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "metric",
icon: ICONS.ChartBar,
label: t("filters.options.metric"),
value: {label: "metric", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "user",
icon: ICONS.AccountOutline,
label: t("filters.options.user"),
value: {label: "user", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "details.cls",
icon: ICONS.FormatListBulletedType,
label: t("filters.options.type"),
value: {label: "type", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "type",
icon: ICONS.FormatListBulletedType,
label: t("filters.options.type"),
value: {label: "service_type", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "permission",
icon: ICONS.AccountCheck,
label: t("filters.options.permission"),
value: {label: "permission", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "type",
icon: ICONS.GestureTapButton,
label: t("filters.options.action"),
value: {label: "action", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "status",
Expand All @@ -148,35 +145,35 @@ export function useFilters(prefix: string) {
icon: ICONS.TagOutline,
label: t("filters.options.details"),
value: {label: "details", comparator: undefined, value: []},
comparators: [COMPARATORS.CONTAINS],
comparators: [COMPARATORS.EQUALS],
},
{
key: "aggregation",
icon: ICONS.Sigma,
label: t("filters.options.aggregation"),
value: {label: "aggregation", comparator: undefined, value: []},
comparators: [COMPARATORS.IS],
comparators: [COMPARATORS.EQUALS]
},
{
key: "timeRange",
icon: ICONS.CalendarRangeOutline,
label: t("filters.options.relative_date"),
value: {label: "relative_date", comparator: undefined, value: []},
comparators: [COMPARATORS.IN],
comparators: [COMPARATORS.EQUALS, COMPARATORS.NOT_EQUALS]
},
{
key: "date",
icon: ICONS.CalendarEndOutline,
label: t("filters.options.absolute_date"),
value: {label: "absolute_date", comparator: undefined, value: []},
comparators: [COMPARATORS.BETWEEN],
comparators: [COMPARATORS.GREATER_THAN, COMPARATORS.LESS_THAN, COMPARATORS.EQUALS]
},
{
key: "labels",
icon: ICONS.TagOutline,
label: t("filters.options.labels"),
value: {label: "labels", comparator: undefined, value: []},
comparators: [COMPARATORS.CONTAINS],
comparators: [COMPARATORS.EQUALS, COMPARATORS.NOT_EQUALS],
},
];

Expand Down
84 changes: 82 additions & 2 deletions ui/src/components/filter/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export const encodeParams = (filters, OPTIONS) => {
export const encodeParams = (path, filters, OPTIONS) => {
if(isSearchPath(path)) {return encodeSearchParams(filters, OPTIONS); }

const encode = (values, key) => {
return values
.map((v) => {
Expand Down Expand Up @@ -42,7 +44,10 @@ export const encodeParams = (filters, OPTIONS) => {
}, {});
};

export const decodeParams = (query, include, OPTIONS) => {
export const decodeParams = (path, query, include, OPTIONS) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love these two functions to be unit tested using vitest.
Create a spec file named tests/unit/filter/utils/helpers.spec.ts and add the test casses like what has been done in flowUtils and yamlUtils.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alaa asked me about tests, I've told him to skip them for now as they were not existing already, but you do make a good point here. As we're not rushing with this for the 0.21.0 release (if I'm not mistaken) we can do the test coverage, also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elevatebart @MilosPaunovic thanks for the review, i will add the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i pushed the helpers.spec.ts for testing, and i will add other case tests

if(isSearchPath(path)) {return decodeSearchParams(query, include, OPTIONS); }


let params = Object.entries(query)
.filter(
([key]) =>
Expand Down Expand Up @@ -95,3 +100,78 @@ export const decodeParams = (query, include, OPTIONS) => {
return {...p, comparator: comparator?.comparators?.[0]};
});
};


export const encodeSearchParams = (filters, OPTIONS) => {
const encode = (values, key, operation) => {
return values.reduce((acc, v) => {
if (key === "childFilter" && v === "ALL") return acc;

const encoded = encodeURIComponent(v);

if (key === "labels") {
const [labelKey, labelValue] = v.split(":");
acc[`filters[${key}][${operation}][${labelKey}]`] = encodeURIComponent(labelValue);
} else {
const paramKey = `filters[${key}][${operation}]`;
acc[paramKey] = acc[paramKey] ? `${acc[paramKey]},${encoded}` : encoded;
}
return acc;
}, {});
};

return filters.reduce((query, filter) => {
const match = OPTIONS.find((o) => o.value.label === filter.label);
const key = match ? match.key : filter.label === "text" ? "q" : null;
const operation = filter.comparator?.value || match?.comparators?.find(c => c.value === filter.operation)?.value || "$eq";

if (key) {
if (key !== "date") {
Object.assign(query, encode(filter.value, key, operation));
} else if (filter.value?.length > 0) {
const {startDate, endDate} = filter.value[0];
query["filters[startDate][$gte]"] = startDate;
query["filters[endDate][$lte]"] = endDate;
}
}
return query;
}, {});
};

export const decodeSearchParams = (query, include, OPTIONS) => {
const params = Object.entries(query)
.filter(([key]) => key.startsWith("filters[") || key === "q")
.map(([key, value]) => {
const match = key.match(/filters\[(.*?)\]\[(.*?)\](?:\[(.*?)\])?/);

if (!match) return null;

const [, field, operation, subKey] = match;

if (field === "labels" && subKey) {
return {label: field, value: `${subKey}:${decodeURIComponent(value)}`, operation};
}

const label = field === "q" ? "text" : OPTIONS.find(o => o.key === field)?.value.label || field;
const comparator = OPTIONS.find(o => o.key === field)?.comparators?.find(c => c.value === operation) || {value: operation};

return {label, value: [decodeURIComponent(value)], operation: comparator.value};
})
.filter(Boolean);

// Handle date filter
if (query["filters[startDate][$gte]"] && query["filters[endDate][$lte]"]) {
params.push({
label: "absolute_date",
value: [{
startDate: query["filters[startDate][$gte]"],
endDate: query["filters[endDate][$lte]"]
}],
operation: "$range"
});
}

return params;
};

export const isSearchPath = (path: string) =>["/admin/triggers","/dashboards/default", "/flows", "/executions", "/logs", "/dashboard"].includes(path);
5 changes: 4 additions & 1 deletion ui/src/translations/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,10 @@
"not_contains": "nicht enthält",
"is_not_one_off": "ist nicht einer von",
"is_one_of": "ist eine von",
"between": "zwischen"
"between": "zwischen",
"greater_than": "größer als",
"less_than": "weniger als",
"ends_with": "endet mit"
},
"save": {
"label": "Gespeicherte Suchen",
Expand Down
5 changes: 4 additions & 1 deletion ui/src/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,10 @@
"not_contains": "not contains",
"in": "in",
"between": "between",
"starts_with": "starts with"
"starts_with": "starts with",
"greater_than": "greater than",
"less_than": "less than",
"ends_with": "ends with"
},
"save": {
"label": "Saved searches",
Expand Down
5 changes: 4 additions & 1 deletion ui/src/translations/es.json
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,10 @@
"not_contains": "no contiene",
"is_not_one_off": "no es uno de",
"is_one_of": "es uno de",
"between": "entre"
"between": "entre",
"greater_than": "mayor que",
"less_than": "menos que",
"ends_with": "termina con"
},
"save": {
"label": "Búsquedas guardadas",
Expand Down
5 changes: 4 additions & 1 deletion ui/src/translations/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,10 @@
"not_contains": "ne contient pas",
"is_not_one_off": "n'est pas l'un de",
"is_one_of": "est l'un de",
"between": "entre"
"between": "entre",
"greater_than": "supérieur à",
"less_than": "moins de",
"ends_with": "se termine par"
},
"save": {
"label": "Recherches enregistrées",
Expand Down
5 changes: 4 additions & 1 deletion ui/src/translations/hi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,10 @@
"not_contains": "नहीं शामिल है",
"is_not_one_off": "नहीं है उनमें से एक",
"is_one_of": "में से एक है",
"between": "के बीच"
"between": "के बीच",
"greater_than": "से अधिक",
"less_than": "से कम",
"ends_with": "समाप्त होता है"
},
"save": {
"label": "सहेजे गए खोजें",
Expand Down
5 changes: 4 additions & 1 deletion ui/src/translations/it.json
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,10 @@
"not_contains": "non contiene",
"is_not_one_off": "non è uno di",
"is_one_of": "è uno di",
"between": "tra"
"between": "tra",
"greater_than": "maggiore di",
"less_than": "minore di",
"ends_with": "termina con"
},
"save": {
"label": "Ricerche salvate",
Expand Down
Loading