-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Add a way to retrieve dimensions from multiple cubes #1244
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…into feat/several-cubes-in-the-same-chart
f414fe4
to
86ff1fd
Compare
…into feat/several-cubes-in-the-same-chart
ce452cd
to
3375e3f
Compare
edde81d
to
8cb95cc
Compare
@bprusinowski can you add an additional point for |
Hi @Rdataflow, in fact we would need to switch back to Argos for screenshot testing, as we used the limit already on Percy (so not possible to check them on CI now) 😞 |
* test(e2e): ✅ fix data-testid on chart type buttons * fix(e2e): ✅ filter selector in tooltip content * fix(e2e): ✅ rework selectors for Sections for sorting tests * fix(e2e): ✅ fix selectors for ordinal-measures * fix(e2e): ✅ fix selector for color-mapping-maps * test(e2e): ✅ skip multi root hierarchy retrieval until refactored in #1244 * fix(e2e): ✅ change map NFI config to Prod * refactor(e2e): ✅ remove the config for nfi dataset * fix(e2e): ✅ fix import when slugs missing * fix(e2e): ✅ id for hierarchical filter * test: Comment out fragile e2e test * fix(e2e): ✅ skip tests that are fragile in CI --------- Co-authored-by: Bartosz Prusinowski <[email protected]>
I followed the direction and clicked around : I am getting a 405 on "Verkehrsleistung auf den Flugplätzen", when going to the table. Not sure this is related to the PR. |
@ptbrowne it looks like the payload is too large (https://vercel.com/ixt/visualization-tool/3pbpsBYim6U2gRc2bE1eDnDovxMx/logs?page=2&timeline=past30Minutes&startDate=1700148221941&endDate=1700150021941&selectedLogId=1700149691375354119628444121&selectedLogTimestamp=1700149691375&forceEndDate=1700149937685), the underlying message says it's error 413 👀 |
I will retake a look tomorrow. What I like
What I have questions on
|
a.position ?? a.value ?? undefined, | ||
b.position ?? b.value ?? undefined | ||
) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This could be done in the dimensionValuesLoader right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, this could be moved there 👍
sourceType: String! | ||
sourceUrl: String! | ||
locale: String! | ||
filters: [DataCubeFilter!]! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was wondering if cubeFilters could not be more explicit as we already use filters for value filters ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this name should be more clear :)
Thanks for reviewing @ptbrowne. I agree it would be better to separate the components queries per cube, so cube-specific queries are not re-fetched when adding another one 👍 I would be interested to learn how to do it, I guess we would need to use |
This PR adapts the application logic to support retrieving dimensions from multiple cubes at once.
GQL / SPARQL
DataCubesComponents
that supports retrieving the components from several cubes at onceComponents
query (replaced byDataCubesComponents
)ComponentsWithHierarchies
andDimensionHierarchy
queries (hierarchies are now fetched by default inDataCubesComponents
). As we generally use them in many different places across the app, it's more developer-friendly to have one source of truth (and also, it might be even more performant to have one query, instead of several different ones fired from different parts of the app). We can also see how the new version ofquery-cube-hierarchy
library improves performance, as then it might be even less worrying, or we can bring back the old way of separating hierarchies fetching to the specific places that use themDimensionValues
query (no need to have a separate query to fetch dimension values, as we can just specify one dimensionIri inDataCubesComponents
to get the same result)SearchCubes
queryDimensionMetadataFragment
type, which means we can now better typeDimension
andMeasure
, instead of mostly using the type that contains bothMisc
Context
This PR is a first part of preparations for merging the cubes. For the technical concept, see this Notion document.
DataCubesComponents
DataCubesMetadata
DataCubesObservations
How to test
Query
Variables