feat(embedded-dashboard): Share Switchboard State for Sending Events from Plugins

[sinhashubham95]

SUMMARY

This is a new feature where we can have a shared switchboard singleton instance available. The idea is to be able to send the interactions happening from the embedded dashboard back to the parent loading the iframe. Another thing is to be able to send any details or data like what filters are selected by the user back to the parent loading the iframe. Now for that we need to have a shared state maintained which can be accessed from the plugins which are actually rendered into the dashboards. For this, a singleton instance of Switchboard which can be lazily initialised has been added. Now the embedded dashboard will use this singleton instance instead of initialising its own.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Embedded dashboards should render without any issue like before.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #21319 (08d611b) into master (61bd696) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master   #21319      +/-   ##
==========================================
- Coverage   66.85%   66.83%   -0.02%     
==========================================
  Files        1798     1798              
  Lines       68827    68844      +17     
  Branches     7333     7339       +6     
==========================================
+ Hits        46011    46014       +3     
- Misses      20931    20951      +20     
+ Partials     1885     1879       -6     

Flag	Coverage Δ
javascript	53.18% <75.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/embedded/index.tsx	0.00% <0.00%> (ø)
...ackages/superset-ui-switchboard/src/switchboard.ts	100.00% <100.00%> (ø)
...rd/components/nativeFilters/FilterBar/keyValue.tsx	18.51% <0.00%> (-22.23%)	⬇️
...board/components/nativeFilters/FilterBar/index.tsx	60.43% <0.00%> (-5.76%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[sinhashubham95]

@villebro can you please check this feature request?

[sinhashubham95]

@stephenLYZ @villebro can you please check?

[sinhashubham95]

@stephenLYZ @villebro can you guys please check?

[stephenLYZ]

@sinhashubham95 Hi. I think it's a nice feature but I don't have enough context for this. So It would be great if @suddjian can help with this.
Besides, if you want to use the singleton pattern, superset-ui-core provides a makeSingleton method to use.

[sinhashubham95]

Thanks for your comment @stephenLYZ. I am aware of the makeSingleton function in the superset-ui-core package, but it still does not provide a way to share the instance across packages. That's why I took this route.

[sinhashubham95]

@suddjian @villebro can you please check this pr?

[sinhashubham95]

@stephenLYZ @villebro @suddjian @rusackas can you guys please help review this pr?

[sinhashubham95]

@stephenLYZ @villebro @suddjian @rusackas can you please check this? It has been open since long.

[jayakrishnankk]

@lilykuang

[sinhashubham95]

@suddjian @villebro @stephenLYZ @rusackas @lilykuang anyone can help with this pr? This has been open for around a month now.

[villebro]

@sinhashubham95 sorry for the delayed review. I'm not super familiar with this area, but I will review this in the coming days.

[sinhashubham95]

Thanks @villebro

[sinhashubham95]

@villebro any update?

[villebro]

LGTM with one nit. I much prefer switching to a singleton, but I would be curious to hear if there were motivations for keeping it multiton. I'll leave this open for a few days and ping some other people, and if there are no objections I'll go ahead with merging.

[villebro]

Is this change needed? I feel like the previous one was cleaner.

[srinify]

+1 to Ville's comment, I'd prefer string as well ... and rest of the file does it that way too!

[sinhashubham95]

This is added because there can be instances of logging the error even when Switchboard is not initialised, which will lead to logging undefined if this change is not made. It's just like providing a default initial value.

[cwegener]

So, it should be name: string = ''; then?

[sinhashubham95]

That's automatically detected in typescript based on the value assigned.

[villebro]

@sinhashubham95 makes sense, I hadn't thought of that 👍

[sinhashubham95]

@villebro can we merge this?