feat: Add logic to merge multiple by multiple join bys #1480

ptbrowne · 2024-04-25T14:37:11Z

Some cubes should be merged on multiple dimensions. This PR makes it possible.

How to test

Start with Photovoltaikanlagen <PREVIEW_URL>/en/create/new?cube=https://energy.ld.admin.ch/sfoe/bfe_ogd84_einmalverguetung_fuer_photovoltaikanlagen/9&dataSource=Prod
Click add dataset
Median electricity per Kanton is mergeable through "year" and "kanton"
Preview shows the two columns as joined
Ability to switch between map / line charts and other charts

vercel · 2024-04-25T14:37:16Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
visualization-tool	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 30, 2024 5:28pm

bprusinowski

LGTM! 👏

Some things I noticed when checking out the deployment preview:

when merging Photovoltaikanlagen with Gebaude, the chart looks broken,
published chart looks different than chart in the edit mode.

To reproduce both issues go to https://visualization-tool-git-feat-multiple-join-by-ixt1.vercel.app/en/v/ZOYp4gfZ_gXN?dataSource=Prod, see how the chart looks like, copy and edit the chart to see that it looks different in the edit mode.

app/configurator/components/add-dataset-dialog.tsx

ptbrowne · 2024-04-30T13:33:52Z

Thanks @bprusinowski, the charts should not look broken anymore (not enough filters applied when switching from chart to chart), and the published chart should not look different anymore (bug of extracting component iris from chart config).

…'s possible for column with numerical measures to have undefined values)

- getChartFilters now take an option object - When options.joined is true, joined dimensions are deduplicated and returned through their id

…ng chart type

fix #1479

bprusinowski

LGTM! 👏

One small thing, I think it's not related to this PR but found while testing it – it looks like the join by dimension description is carried over to every dataset, while it could be dataset-based (I think right now the Jahr, in welchem die EIV ausbezahlt wurde is not really true for Gebaude dataset?) 👀

Let me know if I should create an issue or if it's intended behavior 👍

app/charts/chart-data-wrapper.tsx

app/configurator/config-form.tsx

app/configurator/configurator-state.tsx

app/utils/assert.ts

app/configurator/configurator-state.tsx

app/charts/shared/chart-helpers.spec.tsx

app/graphql/join.spec.ts

fix #1484

ptbrowne requested a review from bprusinowski as a code owner April 25, 2024 14:37

vercel bot deployed to Preview April 25, 2024 14:43 View deployment

bprusinowski approved these changes Apr 25, 2024

View reviewed changes

app/configurator/components/add-dataset-dialog.tsx Outdated Show resolved Hide resolved

vercel bot deployed to Preview April 26, 2024 10:02 View deployment

ptbrowne force-pushed the feat/multiple-join-by branch from 863d55b to 3d5e316 Compare April 26, 2024 10:03

vercel bot deployed to Preview April 26, 2024 10:09 View deployment

vercel bot deployed to Preview April 26, 2024 15:22 View deployment

vercel bot deployed to Preview April 26, 2024 16:37 View deployment

vercel bot deployed to Preview April 29, 2024 14:37 View deployment

ptbrowne force-pushed the feat/multiple-join-by branch from e0c9632 to 3b43c5a Compare April 29, 2024 15:26

vercel bot deployed to Preview April 29, 2024 15:35 View deployment

vercel bot deployed to Preview April 30, 2024 12:39 View deployment

ptbrowne requested a review from bprusinowski April 30, 2024 13:32

ptbrowne force-pushed the feat/multiple-join-by branch from cc17fd2 to 1bd9323 Compare April 30, 2024 13:32

ptbrowne added 14 commits April 30, 2024 15:36

feat: Add logic to merge multiple by multiple join bys

6991c77

docs: Update changelog

87edef5

fix: Correctly fetch table observations when join by is there

be8164d

feat: Set column label correctly when computing joined columns

2a517e1

fix: Handle undefined values correctly in table (with joined data, it…

e292335

…'s possible for column with numerical measures to have undefined values)

fix: Prevent table from overflowing

05c01d7

fix: Add dataset control section for table

db77908

refactor: Use aliased paths

fb1f3c7

fix: Can add correctly a filter on a dimension that is joined by

4ac0007

- getChartFilters now take an option object - When options.joined is true, joined dimensions are deduplicated and returned through their id

fix: Correctly remove filter concerning join by dimension when changi…

2608ab7

…ng chart type

refactor: Lint (use const assertions & remove unnecessary await)

78d40fc

chore: Add eslint-plugin-jest

5e6cbce

fix: Correctly apply filters after changing chart type

7517474

chore: Update yarn.lock

a17177b

ptbrowne added 8 commits April 30, 2024 15:37

refactor: Remove comment

c2e3b3c

refactor: Rename file to indicate it contains functions for join

ffa86d7

refactor: Rename hook util

1047383

refactor: Make function return more coherent, use assert

a97b6d7

refactor: Use helper to detect join by dimension iri

7e44fc1

fix: Off by one error in joinDimensions made published work fail

4386d0b

refactor: Use assert instead of !

e6ea19d

fix: Correctly infer used iris from chart config with join by

5dc3cf3

ptbrowne force-pushed the feat/multiple-join-by branch from 1bd9323 to 5dc3cf3 Compare April 30, 2024 13:37

vercel bot had a problem deploying to Preview April 30, 2024 13:44 Failure

fix: Update mock for correct typings

bc71d47

vercel bot deployed to Preview April 30, 2024 13:57 View deployment

fix: Use publish iri

3f9738e

vercel bot deployed to Preview April 30, 2024 14:13 View deployment

fix: When creator values are there, creator filter is not optional

2b603fa

fix #1479

ptbrowne force-pushed the feat/multiple-join-by branch from 59ffde4 to 2b603fa Compare April 30, 2024 14:35

bprusinowski approved these changes Apr 30, 2024

View reviewed changes

This was referenced Apr 30, 2024

Description of join by dimensions #1484

Closed

✅ Dashboard Layout: Table exceeds width of component #1481

Closed

refactor: Use functions for making join by id

1f6fc23

ptbrowne force-pushed the feat/multiple-join-by branch from 3329625 to 1f6fc23 Compare April 30, 2024 14:44

vercel bot deployed to Preview April 30, 2024 15:02 View deployment

ptbrowne added 4 commits April 30, 2024 18:47

feat: Tweak add dataset dialog styles

1fa9565

fix: Test

9dd2304

fix: Dataset control section had wrong testid

1ab4905

feat: Correct description for dimensions in metadata panel

b954117

fix #1484

ptbrowne force-pushed the feat/multiple-join-by branch from ec6b9cf to b954117 Compare April 30, 2024 17:17

ptbrowne merged commit 36a4c93 into main Apr 30, 2024
3 of 4 checks passed

ptbrowne deleted the feat/multiple-join-by branch April 30, 2024 17:23

vercel bot deployed to Preview April 30, 2024 17:28 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add logic to merge multiple by multiple join bys #1480

feat: Add logic to merge multiple by multiple join bys #1480

ptbrowne commented Apr 25, 2024 •

edited

Loading

vercel bot commented Apr 25, 2024 •

edited

Loading

bprusinowski left a comment •

edited

Loading

ptbrowne commented Apr 30, 2024

bprusinowski left a comment

feat: Add logic to merge multiple by multiple join bys #1480

feat: Add logic to merge multiple by multiple join bys #1480

Conversation

ptbrowne commented Apr 25, 2024 • edited Loading

How to test

vercel bot commented Apr 25, 2024 • edited Loading

bprusinowski left a comment • edited Loading

Choose a reason for hiding this comment

ptbrowne commented Apr 30, 2024

bprusinowski left a comment

Choose a reason for hiding this comment

ptbrowne commented Apr 25, 2024 •

edited

Loading

vercel bot commented Apr 25, 2024 •

edited

Loading

bprusinowski left a comment •

edited

Loading