fix: do not render favorite favStars and filters for anonymous user

[trepmag]

Do not render favorite favStars and filters for charts and dashboards lists for anonymous user

When browsing browsing those lists as anonymous user then 2 issues appears:

1. A flash message popup with the text: "There was an error fetching the favorite status: Internal Server Error" => fixed in fix: Make g.user attribute access safe for public users #14287
2. Favorites switches (favStar) and filter are displayed

Screenshots

Before:
See issue #14070 screenshot.

After:

TEST PLAN

Manually:

1. add some charts and dashbords to be visible by Public role
2. browse as an anonymous user to charts and dashboards lists as anonymous user
3. verify that above described issue (2) doesn't happens

Automatic test:
...

[trepmag]

Last commit 43fe481 for lint issue has 2 problems:

• impose lots of diff hunks
• trigger an exception in charts list: Unexpected error: Error: A column ID (or string accessor) is required!

Will digg into this later...

[codecov]

Codecov Report

Merging #14120 (07e6e6c) into master (37276e1) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@           Coverage Diff           @@
##           master   #14120   +/-   ##
=======================================
  Coverage   77.40%   77.40%           
=======================================
  Files         958      958           
  Lines       48324    48343   +19     
  Branches     5677     5689   +12     
=======================================
+ Hits        37403    37420   +17     
- Misses      10721    10723    +2     
  Partials      200      200           

Flag	Coverage Δ
hive	80.89% <50.00%> (-0.01%)	⬇️
javascript	72.47% <78.57%> (+<0.01%)	⬆️
mysql	81.15% <50.00%> (-0.01%)	⬇️
postgres	81.18% <50.00%> (-0.01%)	⬇️
presto	80.89% <50.00%> (+<0.01%)	⬆️
python	81.72% <50.00%> (+<0.01%)	⬆️
sqlite	80.79% <50.00%> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
...ntend/src/explore/components/ExploreChartPanel.jsx	17.14% <ø> (+1.20%)	⬆️
...nd/src/explore/components/ExploreViewContainer.jsx	2.33% <ø> (ø)
...tend/src/explore/components/ExploreChartHeader.jsx	62.50% <50.00%> (-0.66%)	⬇️
superset/views/core.py	75.48% <50.00%> (-0.04%)	⬇️
...perset-frontend/src/views/CRUD/chart/ChartList.tsx	74.10% <83.33%> (+0.22%)	⬆️
...rontend/src/views/CRUD/dashboard/DashboardList.tsx	76.00% <83.33%> (+0.16%)	⬆️
superset/examples/country_map.py	26.31% <0.00%> (ø)
superset-frontend/src/components/Alert/index.tsx	100.00% <0.00%> (ø)
superset-frontend/src/components/Badge/index.tsx	100.00% <0.00%> (ø)
...-frontend/src/datasource/ChangeDatasourceModal.tsx	85.36% <0.00%> (ø)
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37276e1...07e6e6c. Read the comment docs.

[amitmiran137]

Thank you for contributing your first PR!

[suddjian]

One nit, would be nice if it was addressed but it's non-blocking. This LGTM.

[suddjian]

Is there any way we can cleanly avoid using as Filter here? Ideally the type system would know that this object is compatible without a type assertion.

[rusackas]

@trepmag Thank you for your first PR! I just noticed it's been lingering here a while. Would you like to rebase it and consider @suddjian 's request? We'd love to get this merged - let us know if we can help!

[trepmag]

@suddjian, indeed, removing the as Filter render a compiler ERROR:

TS2322: Type '{ Header: string; id: string; urlDisplay: string; input: string; operator: FilterOperator; unfilteredLabel: string; selects: { label: string; value: boolean; }[]; }' is not assignable to type 'Filter'.
  Types of property 'input' are incompatible.
    Type 'string' is not assignable to type '"select" | "textarea" | "text" | "search" | "checkbox" | "datetime_range" | undefined'.

At this moment I can't figure a way to remove this type casting for this conditional definition...

[trepmag]

I rebased manually into one commit a fix for hiding favorite's favStars and filters for char and dashbord lists (the flash message issue was fixed in btw into another issue: #14287).

Although, I notice a related left over issue; the favstar table can be altered by anyone by alternating the following curls:

$ curl "http://localhost:9000/superset/favstar/Dashboard/1/select/"
$ curl "http://localhost:9000/superset/favstar/Dashboard/1/unselect/"

The select operation insert a row as follow:

[suddjian]

I think I see what's going on with that typing issue. Typescript is inferring a type for the temporary array that is broader than the Filter type. In the inferred type, input is a string, instead of the stricter set of valid strings specified by Filter. To fix this and keep type safety, I suggest extracting the favorite filter into its own variable that is explicitly typed, by doing:

const favoritesFilter: Filter = {
  Header: t('Favorite'),
  id: 'id',
  urlDisplay: 'favorite',
  input: 'select',
  operator: FilterOperator.dashboardIsFav,
  unfilteredLabel: t('Any'),
  selects: [
    { label: t('Yes'), value: true },
    { label: t('No'), value: false },
  ],
}

[suddjian]

Hi @trepmag, I'm wondering if you were interested in trying out my proposed fix for the Filter type. I'm happy to merge this as is if you prefer, just let me know.

[trepmag]

Hi @suddjian, isn't what I did in 07e6e6c? It set const favoritesFilter: Filter = {...} as you suggest...

Do you need I squash all commits?

[suddjian]

Oh you're absolutely right, I missed that somehow. Thanks!

No need to squash, commits will be squashed automatically when the PR is merged.