Skip to content

Range utility functions and column filter changes#1709

Merged
heswell merged 8 commits intofinos:mainfrom
anandBalachandar:task/range-filter-utility-WIP
Sep 8, 2025
Merged

Range utility functions and column filter changes#1709
heswell merged 8 commits intofinos:mainfrom
anandBalachandar:task/range-filter-utility-WIP

Conversation

@anandBalachandar
Copy link
Copy Markdown
Contributor

Changes to address circular dependency issue see
Updated examples to use ColumnFilterStore (not complete)

@netlify
Copy link
Copy Markdown

netlify bot commented Sep 5, 2025

Deploy Preview for papaya-valkyrie-395400 canceled.

Name Link
🔨 Latest commit 6ce9b72
🔍 Latest deploy log https://app.netlify.com/projects/papaya-valkyrie-395400/deploys/68beb851bbe756000827d0d5

@heswell heswell marked this pull request as ready for review September 6, 2025 15:33
@@ -0,0 +1,116 @@
import { describe, expect, it, vi } from "vitest";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the vite tests live in vuu-utils/test/filters

.map(([column, descriptor]) => {
const colDesc = this.#columns.get(column);
if (colDesc)
buildColumnFilterString(
Copy link
Copy Markdown
Contributor

@heswell heswell Sep 6, 2025

Choose a reason for hiding this comment

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

you are not returning a value from the map function, so you build a list of undefined

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

last minute change related to a type issue..causing the tests to fail!

it("resets all filters", () => {
const store = new ColumnFilterStore();
store.addFilter(toColumnDescriptor("ric"), "=", "AAOQ.OQ");
store.addFilter(toColumnDescriptor("price"), ">", 100);
Copy link
Copy Markdown
Contributor

@heswell heswell Sep 6, 2025

Choose a reason for hiding this comment

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

you are going to have to hard code the columnDescriptor here, it needs serverDataType: 'double'
otw it defaults to string and value will be quoted

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will we have columnDescriptor (serverDataType) when we are loading the store from a saved filter (query)?

Copy link
Copy Markdown
Contributor Author

@anandBalachandar anandBalachandar Sep 7, 2025

Choose a reason for hiding this comment

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

perhaps something we will have to maintain - a list of columnDescriptors used by the ColumnFilter on the page and inject it into the ColumnFilterStore constructor along with the query?

Copy link
Copy Markdown
Contributor

@heswell heswell left a comment

Choose a reason for hiding this comment

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

couple of minor things, which should fix the etsts

const { serverDataType = "string" } = column;
const typedValue = Array.isArray(value)
? value
: (getTypedValue(value, serverDataType, true) as ColumnFilterValue); //Check getTypedValue
Copy link
Copy Markdown
Contributor

@heswell heswell Sep 6, 2025

Choose a reason for hiding this comment

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

to fix the type error, force this to a string... maybe

getTypedValue(`${value}`)

Updated examples to use ColumnFilterStore
Added test coverage

Pls note: WIP - Looking at conversion of values - TimeColumnPicker/DatePicker examples may have issues because of this.
Added test coverage
PR feedback implemented

Todo
Validation of range filters (numeric/date/time)
Allowing column descriptors to be injected through the column filter store class
@anandBalachandar anandBalachandar force-pushed the task/range-filter-utility-WIP branch from a895870 to a4540bb Compare September 8, 2025 10:12
Fixed failing test due to timezone
@heswell heswell merged commit fe5c478 into finos:main Sep 8, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants