Skip to content

Conversation

@NikkiAung
Copy link
Contributor

Summary & Motivation

Fixed a bug where clicking a partition in AssetPartitionList placed default_range before partition in the URL, causing the purple overlay to expand incorrectly. The URL parameter order wasn't controlled, so default_range could appear before partition. This broke the overlay behavior. The fix ensures partition always comes before default_range in the URL, restoring correct overlay behavior.

How I Tested These Changes

I tested the changes locally by running both the backend and frontend. For the backend, I launched the Dagster GraphQL server using time_based_partitioning.py, static_partitioning.py and files like those using dagster dev -f time_based_partitioning.py -p 3333 for example. For the frontend, I ran the Dagster UI using make dev_webapp. And the Verified Outcomes:

Fixed a bug where clicking a partition in the list put default_range before partition in the URL, which made the purple overlay expand incorrectly. The issue was that the code didn't control the order of URL parameters. Added logic to always place partition before default_range when building URLs, so the overlay behaves correctly.

Don't worry about code format, I did yarn lint, yarn ts and yarn jest within ui-core to pick up lint fixes

fixBug/partition-before-default-range](fix: partition before default-range URL order)

Resolves "View Partitions" features at asset sidebar #32796

@NikkiAung
Copy link
Contributor Author

Hello @cmpadden, @hellendag, and @salazarm, mind having a look at the changes I made for the issue?

@hellendag
Copy link
Member

Thanks for giving this a try. The order of URL query params actually shouldn't matter at all in general, and the root of the bug lies in the order in which we extract the values and copy them to another object.

I have a change that should take care of the issue. I'll share it here tomorrow morning and let you try testing it to verify that it fixes things on your side.

@hellendag
Copy link
Member

Take a look at #32804 and see if that resolves the issue for you.

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