-
Notifications
You must be signed in to change notification settings - Fork 16.6k
fix(home): missing key and invalid dates in Recents cards #13291
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,25 +23,43 @@ import { styled, t } from '@superset-ui/core'; | |
| import Loading from 'src/components/Loading'; | ||
| import ListViewCard from 'src/components/ListViewCard'; | ||
| import SubMenu from 'src/components/Menu/SubMenu'; | ||
| import { Chart } from 'src/types/Chart'; | ||
| import { Dashboard, SavedQueryObject } from 'src/views/CRUD/types'; | ||
| import { mq, CardStyles } from 'src/views/CRUD/utils'; | ||
|
|
||
| import { ActivityData } from './Welcome'; | ||
| import { mq, CardStyles } from '../utils'; | ||
| import EmptyState from './EmptyState'; | ||
|
|
||
| interface ActivityObjects { | ||
| action?: string; | ||
| item_title?: string; | ||
| slice_name: string; | ||
| time: string; | ||
| changed_on_utc: string; | ||
| url: string; | ||
| sql: string; | ||
| dashboard_title: string; | ||
| label: string; | ||
| id: string; | ||
| table: object; | ||
| /** | ||
| * Return result from /superset/recent_activity/{user_id} | ||
| */ | ||
| interface RecentActivity { | ||
| action: string; | ||
| item_type: 'slice' | 'dashboard'; | ||
| item_url: string; | ||
| item_title: string; | ||
| time: number; | ||
| time_delta_humanized?: string; | ||
| } | ||
|
|
||
| interface RecentSlice extends RecentActivity { | ||
| item_type: 'slice'; | ||
| } | ||
|
|
||
| interface RecentDashboard extends RecentActivity { | ||
| item_type: 'dashboard'; | ||
| } | ||
|
|
||
| /** | ||
| * Recent activity objects fetched by `getRecentAcitivtyObjs`. | ||
| */ | ||
| type ActivityObject = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ktmud Are you sure it's just these types?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, as |
||
| | RecentSlice | ||
| | RecentDashboard | ||
| | Chart | ||
| | Dashboard | ||
| | SavedQueryObject; | ||
|
|
||
| interface ActivityProps { | ||
| user: { | ||
| userId: string | number; | ||
|
|
@@ -79,31 +97,70 @@ const ActivityContainer = styled.div` | |
| } | ||
| `; | ||
|
|
||
| const UNTITLED = t('[Untitled]'); | ||
| const UNKNOWN_TIME = t('Unknown'); | ||
|
|
||
| const getEntityTitle = (entity: ActivityObject) => { | ||
| if ('dashboard_title' in entity) return entity.dashboard_title || UNTITLED; | ||
| if ('slice_name' in entity) return entity.slice_name || UNTITLED; | ||
| if ('label' in entity) return entity.label || UNTITLED; | ||
| return entity.item_title || UNTITLED; | ||
| }; | ||
|
|
||
| const getEntityIconName = (entity: ActivityObject) => { | ||
| if ('sql' in entity) return 'sql'; | ||
| const url = 'item_url' in entity ? entity.item_url : entity.url; | ||
| if (url?.includes('dashboard')) { | ||
| return 'nav-dashboard'; | ||
| } | ||
| if (url?.includes('explore')) { | ||
| return 'nav-charts'; | ||
| } | ||
| return ''; | ||
| }; | ||
|
|
||
| const getEntityUrl = (entity: ActivityObject) => { | ||
| if ('sql' in entity) return `/superset/sqllab?savedQueryId=${entity.id}`; | ||
| if ('url' in entity) return entity.url; | ||
| return entity.item_url; | ||
| }; | ||
|
|
||
| const getEntityLastActionOn = (entity: ActivityObject) => { | ||
| // translation keys for last action on | ||
| const LAST_VIEWED = `Last viewed %s`; | ||
| const LAST_MODIFIED = `Last modified %s`; | ||
|
|
||
| // for Recent viewed items | ||
| if ('time_delta_humanized' in entity) { | ||
| return t(LAST_VIEWED, entity.time_delta_humanized); | ||
| } | ||
|
|
||
| if ('changed_on_delta_humanized' in entity) { | ||
| return t(LAST_MODIFIED, entity.changed_on_delta_humanized); | ||
| } | ||
|
|
||
| let time: number | string | undefined | null; | ||
| let translationKey = LAST_MODIFIED; | ||
| if ('time' in entity) { | ||
| // eslint-disable-next-line prefer-destructuring | ||
| time = entity.time; | ||
| translationKey = LAST_VIEWED; | ||
| } | ||
| if ('changed_on' in entity) time = entity.changed_on; | ||
| if ('changed_on_utc' in entity) time = entity.changed_on_utc; | ||
|
|
||
| return t( | ||
| translationKey, | ||
| time == null ? UNKNOWN_TIME : moment(time).fromNow(), | ||
| ); | ||
| }; | ||
|
|
||
| export default function ActivityTable({ | ||
| loading, | ||
| activeChild, | ||
| setActiveChild, | ||
| activityData, | ||
| }: ActivityProps) { | ||
| const getFilterTitle = (e: ActivityObjects) => { | ||
| if (e.dashboard_title) return e.dashboard_title; | ||
| if (e.label) return e.label; | ||
| if (e.url && !e.table) return e.item_title; | ||
| if (e.item_title) return e.item_title; | ||
| return e.slice_name; | ||
| }; | ||
|
|
||
| const getIconName = (e: ActivityObjects) => { | ||
| if (e.sql) return 'sql'; | ||
| if (e.url?.includes('dashboard') || e.item_url?.includes('dashboard')) { | ||
| return 'nav-dashboard'; | ||
| } | ||
| if (e.url?.includes('explore') || e.item_url?.includes('explore')) { | ||
| return 'nav-charts'; | ||
| } | ||
| return ''; | ||
| }; | ||
|
|
||
| const tabs = [ | ||
| { | ||
| name: 'Edited', | ||
|
|
@@ -139,35 +196,30 @@ export default function ActivityTable({ | |
| }); | ||
| } | ||
|
|
||
| const renderActivity = () => { | ||
| const getRecentRef = (e: ActivityObjects) => { | ||
| if (activeChild === 'Viewed') { | ||
| return e.item_url; | ||
| } | ||
| return e.sql ? `/superset/sqllab?savedQueryId=${e.id}` : e.url; | ||
| }; | ||
| return activityData[activeChild].map((e: ActivityObjects) => ( | ||
| <CardStyles | ||
| onClick={() => { | ||
| window.location.href = getRecentRef(e); | ||
| }} | ||
| key={e.id} | ||
| > | ||
| <ListViewCard | ||
| loading={loading} | ||
| cover={<></>} | ||
| url={e.sql ? `/superset/sqllab?savedQueryId=${e.id}` : e.url} | ||
| title={getFilterTitle(e)} | ||
| description={`Last Edited: ${moment( | ||
| e.changed_on_utc, | ||
| 'MM/DD/YYYY HH:mm:ss', | ||
| )}`} | ||
| avatar={getIconName(e)} | ||
| actions={null} | ||
| /> | ||
| </CardStyles> | ||
| )); | ||
| }; | ||
| const renderActivity = () => | ||
| activityData[activeChild].map((entity: ActivityObject) => { | ||
| const url = getEntityUrl(entity); | ||
| const lastActionOn = getEntityLastActionOn(entity); | ||
| return ( | ||
| <CardStyles | ||
| onClick={() => { | ||
| window.location.href = url; | ||
| }} | ||
| key={url} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this guaranteed to be unique? The same item can't show up twice in the recent activity section?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's guaranteed. The |
||
| > | ||
| <ListViewCard | ||
| loading={loading} | ||
| cover={<></>} | ||
| url={url} | ||
| title={getEntityTitle(entity)} | ||
| description={lastActionOn} | ||
| avatar={getEntityIconName(entity)} | ||
| actions={null} | ||
| /> | ||
| </CardStyles> | ||
| ); | ||
| }); | ||
|
|
||
| if (loading) return <Loading position="inline" />; | ||
| return ( | ||
| <> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes, i don't even want to know why we have a
Sliceand aCharttype...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one has stricter typing following the backend API response; another is for temporary chart state on the client side (we should probably consolidate these two).