[MDS] Expose common url key for consumers #6602
Conversation
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
huyaboo
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT and
xinruiba
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6602 +/- ##
==========================================
- Coverage 67.44% 67.44% -0.01%
==========================================
Files 3442 3442
Lines 67804 67805 +1
Branches 11025 11025
==========================================
- Hits 45730 45729 -1
- Misses 19408 19409 +1
- Partials 2666 2667 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
huyaboo
force-pushed
the
urlkey
branch
2 times, most recently
from
080ad48
to
17eb0d4
Compare
|
Can we hold off on this one and include it in 2.15, given that we're close to the 2.14 code freeze date? It might look like a small change, but it requires the plugin team to go through the development cycle again to adopt this change |
Acknowledged. Moved to |
huyaboo
force-pushed
the
urlkey
branch
5 times, most recently
from
cd11e03
to
4742741
Compare
huyaboo
force-pushed
the
urlkey
branch
2 times, most recently
from
e70eb39
to
f5b717b
Compare
| @@ -110,6 +116,7 @@ export class DataSourceManagementPlugin | |||
| ui: { | |||
| DataSourceSelector: createDataSourceSelector(uiSettings, dataSource), | |||
| getDataSourceMenu: <T>() => createDataSourceMenu<T>(), | |||
| dataSourceURLKey: DATA_SOURCE_URL_KEY, | |||
There was a problem hiding this comment.
Hi Huy. Could you help me to understand what does this dataSourceURLKey do and how does it help plugin team?
There was a problem hiding this comment.
Before, plugins had to manage how they wanted to persist the selected datasource across their pages. Most opted for the datasource to be a part of the url since it makes the page bookmark-able and easier than drilling down props. However, this urlkey &dataSource= is not global, meaning different plugins can have different urlkeys. This PR will expose a common key dataSource for them to consume to increase consistency among plugin urls.
There was a problem hiding this comment.
Got it. Thanks for explaining it.
| @@ -34,6 +39,7 @@ export interface DataSourceManagementPluginSetup { | |||
| ui: { | |||
| DataSourceSelector: React.ComponentType<DataSourceSelectorProps>; | |||
| getDataSourceMenu: <T>() => React.ComponentType<DataSourceMenuProps<T>>; | |||
| dataSourceURLKey: string; | |||
There was a problem hiding this comment.
Nit: Shall we use typeof DATA_SOURCE_URL_KEY here?
There was a problem hiding this comment.
@huyaboo +1, can we use type instead of exposing it like this?
There was a problem hiding this comment.
Nit: do we have developer documentation for plugins/consumers about how to use this key? @huyaboo
Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com>
Description
This PR adds a common URL key for plugins/other consumers to consume.
Issues Resolved
Closes #6534
Screenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration