Skip to content

fix(fe/dashboard-list): display modifier info for Last modified data #32035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rusackas merged 5 commits into from
Feb 21, 2025
Merged

fix(fe/dashboard-list): display modifier info for Last modified data #32035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7bd9a6
fix(fe/dashboard-list): display modifier info for `Last modified` data
hainenber Jan 30, 2025
add32de
chore: update DashboardList unit test with new fetch query params
hainenber Jan 30, 2025
8f37ad2
Merge branch 'master' into fix/missing-modifier-tooltip-in-dashboard-…
hainenber Feb 21, 2025
d7f4a17
chore: re-add dashboard columns with nested field type to avoid break…
hainenber Feb 21, 2025
88fc9e0
fix(ci): refactor unit test to reflect changes in fetched dashboard c…
hainenber Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset-frontend/src/pages/DashboardList/DashboardList.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe('DashboardList', () => {
const callsD = fetchMock.calls(/dashboard\/\?q/);
expect(callsD).toHaveLength(1);
expect(callsD[0][0]).toMatchInlineSnapshot(
`"http://localhost/api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25,select_columns:!(id,dashboard_title,published,url,slug,changed_by,changed_on_delta_humanized,owners.id,owners.first_name,owners.last_name,owners,tags.id,tags.name,tags.type,status,certified_by,certification_details,changed_on))"`,
`"http://localhost/api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25,select_columns:!(id,dashboard_title,published,url,slug,changed_by,changed_by.id,changed_by.first_name,changed_by.last_name,changed_on_delta_humanized,owners,owners.id,owners.first_name,owners.last_name,tags.id,tags.name,tags.type,status,certified_by,certification_details,changed_on))"`,
);
});

Expand Down
5 changes: 4 additions & 1 deletion superset-frontend/src/pages/DashboardList/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,14 @@ const DASHBOARD_COLUMNS_TO_FETCH = [
'url',
'slug',
'changed_by',
'changed_by.id',
'changed_by.first_name',
'changed_by.last_name',
'changed_on_delta_humanized',
'owners',
'owners.id',
'owners.first_name',
'owners.last_name',
'owners',
Copy link
Member

@michael-s-molina michael-s-molina Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hainenber Can you confirm that the owners' fields, apart from id, first_name and last_name, are not being used? Same for changed_by's fields.

Copy link
Contributor Author

@hainenber hainenber Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attributes like owners.last_name are the one populating the owners field. For some reasons, you have to specify atributes for nested fields like owners and changed_by.

Copy link
Member

@michael-s-molina michael-s-molina Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you misunderstood what I mean. Previously, we had owners being declared which means that ALL attributes of owners were accessible like full_name. Now that you excluded owners from the fetched columns, if there's some component accessing full_name, it will break. I'm ok with removing the whole owners object but we need to check which attributes are in fact being used so that the projection is correct.

Copy link
Contributor Author

@hainenber hainenber Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Then in that case, I'll only add the missing attributes and not to remove the nested field names to minimize any risks.

Copy link
Contributor Author

@hainenber hainenber Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-added old nested fields. Issue is still fixed with less risks :D

image

'tags.id',
'tags.name',
'tags.type',
Expand Down
Loading