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

Make queryUI on ViewV2 something you can write on its own, and query gets filled in for you #14818

Merged
merged 27 commits into from
Oct 23, 2024

Conversation

samwho
Copy link
Collaborator

@samwho samwho commented Oct 17, 2024

Description

This PR is on request of @aptkingston, it makes it such that the various view APIs allow the frontend to only work with queryUI and can ignore query.

Here's broadly what's happening:

  • On view create/update, if you set queryUI it will generate query for you (regardless what you specify for query, it will get overwritten because queryUI is the source of truth).
  • On view fetch (either directly or through the table), if there is a query but no queryUI, queryUI will get generated from query*.

*This only happens if query is of type LegacyFilter[], which is true of all ViewV2 in prod on Budicloud today (we checked). When we convert queryUI to query, we don't convert to a LegacyFilter[] but instead to a SearchFilters. There's currently no function to convert between SearchFilters and UISearchFilter (renamed from SearchFilterGroup) because it's technically lossy, and in theory we don't have to because from now on the frontend will always set queryUI.

I'm sorry for how confusing this all is. There's a lot of history around this part of the codebase and I'm doing the best I can to slot in new requirements to an already complex part of the code.

Feature branch env

Feature Branch Link

Copy link

qa-wolf bot commented Oct 17, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/xl labels Oct 17, 2024
@samwho samwho marked this pull request as ready for review October 21, 2024 12:50
@samwho samwho requested a review from a team as a code owner October 21, 2024 12:50
@samwho samwho requested review from mike12345567 and removed request for a team October 21, 2024 12:50
@samwho samwho added the feature-branch Release this PR code into a feature branch label Oct 21, 2024
packages/server/src/sdk/app/tables/getters.ts Outdated Show resolved Hide resolved
packages/shared-core/src/utils.ts Outdated Show resolved Hide resolved
packages/shared-core/src/filters.ts Show resolved Hide resolved
packages/server/src/sdk/app/views/utils.ts Show resolved Hide resolved
packages/types/src/api/web/searchFilter.ts Show resolved Hide resolved
Copy link
Collaborator

@mike12345567 mike12345567 left a comment

Choose a reason for hiding this comment

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

LGTM - very nice work, great to get all the tests updated as well!

@samwho samwho changed the base branch from master to v3-ui October 23, 2024 12:56
@samwho samwho merged commit ad475be into v3-ui Oct 23, 2024
11 checks passed
@samwho samwho deleted the queryui-default branch October 23, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-branch Release this PR code into a feature branch firestorm Data/Infra/Revenue Team size/xl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants