fix(dashboard): Fix missing metadata on draggable dashboard edit chart cards

[codyml]

SUMMARY

Currently, when editing a dashboard, any charts that are already in the dashboard when the Edit button is clicked but are not owned by the current user appear in the sidebar of draggable charts missing metadata. This is due to a two-step process for downloading charts:

• Charts in the dashboard are added to the Redux store's sliceEntities.slices object when the dashboard is loaded, but are missing the metadata necessary to correctly render as draggable cards.
• When the edit button is clicked, all charts owned by the current user are downloaded with necessary metadata and merged into the sliceEntities.slices store object. Because some charts added to the dashboard might not be owned by the current user, the sliceEntities.slices entries for those charts are not updated with the necessary metadata.
  This PR removes the filter from the 2nd request that limits it to charts owned by the current user, so all charts are now correctly downloaded and merged into the store object.

Note: I'm opening this PR under the assumption that this filter was present for no good reason. If there's a reason that you should only be able to add charts that you own to a dashboard, let me know and I'll find another solution.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TESTING INSTRUCTIONS

• Confirm that metadata now appears for charts that were added to the dashboard previously but that you don't own.

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 #20684 (20d10d5) into master (84d4302) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #20684   +/-   ##
=======================================
  Coverage   66.30%   66.30%           
=======================================
  Files        1756     1756           
  Lines       66734    66734           
  Branches     7049     7049           
=======================================
  Hits        44248    44248           
  Misses      20689    20689           
  Partials     1797     1797           

Flag	Coverage Δ
javascript	51.93% <ø> (ø)

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

Impacted Files	Coverage Δ
...et-frontend/src/dashboard/actions/sliceEntities.js	74.28% <ø> (ø)

[jinghua-qa]

/testenv up

[github-actions]

@jinghua-qa Ephemeral environment spinning up at http://35.166.98.73:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

[michael-s-molina]

I did some digging and this is the PR that introduced that filter. Maybe @nytai has some context on it?

[codyml]

I did some digging and this is the PR that introduced that filter. Maybe @nytai has some context on it?

@michael-s-molina Looks like that PR transitioned from another endpoint that had a similar restriction – do you know if there's any spec for what the permissions are supposed to be around adding charts to a dashboard? My thought would be that if you have view permissions on a chart, you should be able to add it to a dashboard that you have edit permissions for, but that's just off the top of my head, I don't have a great understanding of the permissions model.

[jinghua-qa]

QA team found an issue that when user add a chart (not owner) to the dashboard and then delete, user will automatically become owner of the chart. This issue has been filed in ticket and will be fixed in another pr. Other than that, LGTM~

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.

[codyml]

QA team found an issue that when user add a chart (not owner) to the dashboard and then delete, user will automatically become owner of the chart. This issue has been filed in ticket and will be fixed in another pr. Other than that, LGTM~

Linking for future documentation: #21720

[john-bodley]

@codyml in reference to your comment,

Note: I'm opening this PR under the assumption that this filter was present for no good reason. If there's a reason that you should only be able to add charts that you own to a dashboard, let me know and I'll find another solution.

I thought there was merit in sharing some user feedback—for context at Airbnb where we have a significant number of charts, i.e., in excess of 100k—I received:

To me the experience is degraded, because almost all charts I add are created by me, and there’s a clear candidate for the next chart to add, i.e., it’s the chart most recently created by me.

Of course it’s good to be able to add charts created by others, but I would think that maybe this could be visible through a toggle (default is to show charts created by me, with toggle in “on” stage for “filter to my charts”).

This change of removing the filter might be an improvement in a small company, but in a big place it’s very unlikely that charts by others are useful to me, there’s just too many.

Based on this feedback I was wondering whether:

1. There should be a toggle button (or similar) to filter between your charts (default) and all charts to help streamline the user workflow—especially in the case where exists thousands and thousands of charts.
2. The placeholder text should be updated given that "Filter your charts" is no longer accurate.

Thoughts?

[codyml]

@john-bodley Thanks for the follow-up! Those points make a lot of sense to me and it sounds like there's definitely room for improvement.

From what I remember, no one at Preset was clamoring for being able to add charts that you don't own to a dashboard via dashboard edit (cc @lauderbaugh in case I'm wrong). I think I chose this approach just because it seemed like the easiest way to resolve the missing metadata bug caused by the list trying to display charts that you don't own that were already added to the dashboard.

I like the idea of a toggle – that sounds like a best of both worlds solution. A bandaid solution could also be to revert this change and solve the missing metadata issue by just not trying to show charts added to the dashboard that you don't own in the list at all. The downside of that would be that if you remove a chart you don't own from a dashboard there would be no way to get it back. But, that was already sort of a problem before this PR: if you removed a chart you didn't own and then saved the dashboard, you wouldn't be able to add it back from the dashboard edit list.

Is changing this behavior something that you'd like to work on? Happy to discuss these or any other ideas further or do any PR reviews as helpful – let me know!