Skip to content
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

refactor(dashboards): remove last_refresh from dashboard_tile #24004

Merged
merged 7 commits into from
Jul 26, 2024
Merged

refactor(dashboards): remove last_refresh from dashboard_tile #24004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
45a9f19
refactor(dashboards): remove last_refresh from dashboard_tile
thmsobrmlr Jul 25, 2024
15f43ee
fix tests
thmsobrmlr Jul 25, 2024
1751441
remove unused oldestRefreshed
thmsobrmlr Jul 25, 2024
19d8107
fix newest refresh
thmsobrmlr Jul 25, 2024
ae90ea3
fixes
thmsobrmlr Jul 25, 2024
61a1fd4
Update UI snapshots for `chromium` (2)
github-actions[bot] Jul 25, 2024
625731a
Update UI snapshots for `chromium` (2)
github-actions[bot] Jul 25, 2024
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
Binary file modified ..._snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 0 additions & 2 deletions frontend/src/lib/components/Cards/TextCard/TextCard.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ const makeTextTile = (body: string, color: InsightColor | null = null): Dashboar

layouts: {},
color,
last_refresh: null,
next_allowed_client_refresh: null,
}
}

Expand Down
20 changes: 2 additions & 18 deletions frontend/src/models/dashboardsModel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,10 @@ export const dashboardsModel = kea<dashboardsModelType>([
// can provide extra dashboard ids if not all listeners will choose to respond to this action
// not providing a dashboard id is a signal that only listeners in the item.dashboards array should respond
// specifying `number` not `Pick<DashboardType, 'id'> because kea typegen couldn't figure out the import in `savedInsightsLogic`
// if an update is made against an insight it will hold last_refresh, color, and filters_hash in dashboard context
updateDashboardInsight: (
insight: InsightModel,
extraDashboardIds?: number[],
updateTileOnDashboards?: [number]
) => ({
// if an update is made against an insight it will hold color in dashboard context
updateDashboardInsight: (insight: InsightModel, extraDashboardIds?: number[]) => ({
insight,
extraDashboardIds,
updateTileOnDashboards,
}),
// a side effect on this action exists in dashboardLogic so that individual refresh statuses can be bubbled up
// to dashboard items in dashboards
updateDashboardRefreshStatus: (
shortId: string | undefined | null,
refreshing: boolean | null,
last_refresh: string | null
) => ({
shortId,
refreshing,
last_refresh,
}),
pinDashboard: (id: number, source: DashboardEventSource) => ({ id, source }),
unpinDashboard: (id: number, source: DashboardEventSource) => ({ id, source }),
Expand Down
24 changes: 11 additions & 13 deletions frontend/src/scenes/dashboard/dashboardLogic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,19 @@ export function insightOnDashboard(
if (!tile.insight) {
throw new Error('tile has no insight')
}
return { ...tile.insight, dashboards: dashboardsRelation, filters: { ...tile.insight.filters, ...insight.filters } }
return {
...tile.insight,
dashboards: dashboardsRelation,
dashboard_tiles: dashboardsRelation.map((dashboardId) => ({ id: insight.id!, dashboard_id: dashboardId })),
filters: { ...tile.insight.filters, ...insight.filters },
}
}

const TEXT_TILE: DashboardTile = {
id: 4,
text: { body: 'I AM A TEXT', last_modified_at: '2021-01-01T00:00:00Z' },
layouts: {},
color: InsightColor.Blue,
last_refresh: '2021-01-01T00:00:00Z',
}

let tileId = 0
Expand All @@ -57,7 +61,6 @@ export const tileFromInsight = (insight: InsightModel, id: number = tileId++): D
layouts: {},
color: null,
insight: insight,
last_refresh: insight.last_refresh,
})

export const dashboardResult = (
Expand Down Expand Up @@ -590,22 +593,17 @@ describe('dashboardLogic', () => {

it('can respond to external update of an insight on the dashboard', async () => {
const copiedInsight = insight800()
dashboardsModel.actions.updateDashboardInsight(
{
...copiedInsight,
filters: { ...copiedInsight.filters, date_from: '-1d', interval: 'hour' },
last_refresh: '2012-04-01T00:00:00Z',
},
[],
[9]
)
dashboardsModel.actions.updateDashboardInsight({
...copiedInsight,
filters: { ...copiedInsight.filters, date_from: '-1d', interval: 'hour' },
last_refresh: '2012-04-01T00:00:00Z',
})

await expectLogic(logic).toFinishAllListeners()
expect(logic.values.dashboard?.tiles).toHaveLength(2)
expect(logic.values.insightTiles[0].insight!.filters.date_from).toEqual('-1d')
expect(logic.values.insightTiles[0].insight!.filters.interval).toEqual('hour')
expect(logic.values.textTiles[0].text!.body).toEqual('I AM A TEXT')
expect(logic.values.insightTiles[0].last_refresh).toEqual('2012-04-01T00:00:00Z')
})

it('can respond to external insight rename', async () => {
Expand Down
61 changes: 14 additions & 47 deletions frontend/src/scenes/dashboard/dashboardLogic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,10 @@ export const dashboardLogic = kea<dashboardLogicType>([
}
return state
},
[dashboardsModel.actionTypes.updateDashboardInsight]: (
state,
{ insight, extraDashboardIds, updateTileOnDashboards }
) => {
const targetDashboards = (insight.dashboards || []).concat(extraDashboardIds || [])
[dashboardsModel.actionTypes.updateDashboardInsight]: (state, { insight, extraDashboardIds }) => {
const targetDashboards = (insight.dashboard_tiles || [])
.map((tile) => tile.dashboard_id)
.concat(extraDashboardIds || [])
if (!targetDashboards.includes(props.id)) {
// this update is not for this dashboard
return state
Expand All @@ -427,11 +426,10 @@ export const dashboardLogic = kea<dashboardLogicType>([
const newTiles = state.tiles.slice()

if (tileIndex >= 0) {
if (insight.dashboards?.includes(props.id) && updateTileOnDashboards?.includes(props.id)) {
if (insight.dashboards?.includes(props.id)) {
newTiles[tileIndex] = {
...newTiles[tileIndex],
insight: insight,
last_refresh: insight.last_refresh,
}
} else if (!insight.dashboards?.includes(props.id)) {
newTiles.splice(tileIndex, 1)
Expand All @@ -452,27 +450,6 @@ export const dashboardLogic = kea<dashboardLogicType>([
[dashboardsModel.actionTypes.updateDashboardSuccess]: (state, { dashboard }) => {
return state && dashboard && state.id === dashboard.id ? dashboard : state
},
[dashboardsModel.actionTypes.updateDashboardRefreshStatus]: (
state,
{ shortId, refreshing, last_refresh }
) => {
// If not a dashboard item, don't do anything.
if (!shortId) {
return state
}
return {
...state,
items: state?.tiles.map((t) =>
!t.insight || t.insight.short_id === shortId
? {
...t,
...(refreshing != null ? { refreshing } : {}),
...(last_refresh != null ? { last_refresh } : {}),
}
: t
),
} as DashboardType
},
[insightsModel.actionTypes.renameInsightSuccess]: (state, { item }): DashboardType | null => {
const tileIndex = state?.tiles.findIndex((t) => !!t.insight && t.insight.short_id === item.short_id)
const tiles = state?.tiles.slice(0)
Expand Down Expand Up @@ -686,7 +663,9 @@ export const dashboardLogic = kea<dashboardLogicType>([
return []
}

const validDates = insightTiles.map((i) => dayjs(i.last_refresh)).filter((date) => date.isValid())
const validDates = insightTiles
.map((i) => dayjs(i.insight?.last_refresh))
.filter((date) => date.isValid())
return sortDayJsDates(validDates)
},
],
Expand All @@ -701,16 +680,6 @@ export const dashboardLogic = kea<dashboardLogicType>([
return sortedDates[sortedDates.length - 1]
},
],
oldestRefreshed: [
(s) => [s.sortedDates],
(sortedDates): Dayjs | null => {
if (!sortedDates.length) {
return null
}

return sortedDates[0]
},
],
sortedClientRefreshAllowed: [
(s) => [s.insightTiles],
(insightTiles): Dayjs[] => {
Expand Down Expand Up @@ -914,7 +883,9 @@ export const dashboardLogic = kea<dashboardLogicType>([
}
},
[dashboardsModel.actionTypes.updateDashboardInsight]: ({ insight, extraDashboardIds }) => {
const targetDashboards = (insight.dashboards || []).concat(extraDashboardIds || [])
const targetDashboards = (insight.dashboard_tiles || [])
.map((tile) => tile.dashboard_id)
.concat(extraDashboardIds || [])
if (!targetDashboards.includes(props.id)) {
// this update is not for this dashboard
return
Expand Down Expand Up @@ -1000,7 +971,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
uuid(),
'force_async'
)
dashboardsModel.actions.updateDashboardInsight(refreshedInsight, [], props.id ? [props.id] : undefined)
dashboardsModel.actions.updateDashboardInsight(refreshedInsight)
// Start polling for results
tile.insight = refreshedInsight
actions.refreshAllDashboardItems({ tiles: [tile], action: 'refresh' })
Expand Down Expand Up @@ -1063,11 +1034,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
'async',
methodOptions
)
dashboardsModel.actions.updateDashboardInsight(
polledInsight,
[],
props.id ? [props.id] : undefined
)
dashboardsModel.actions.updateDashboardInsight(polledInsight)
actions.setRefreshStatus(insight.short_id)
}
} catch (e: any) {
Expand Down Expand Up @@ -1165,7 +1132,7 @@ export const dashboardLogic = kea<dashboardLogicType>([

const dashboard = values.dashboard
const { action, dashboardQueryId, startTime, responseBytes } = values.dashboardLoadTimerData
const lastRefresh = sortDates(dashboard.tiles.map((tile) => tile.last_refresh))
const lastRefresh = sortDates(dashboard.tiles.map((tile) => tile.insight?.last_refresh || null))

const initialLoad = action === 'initial_load'
const allLoaded = false // TODO: Check this
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,7 @@ export interface Tileable {
color: InsightColor | null
}

export interface DashboardTile extends Tileable, Cacheable {
export interface DashboardTile extends Tileable {
id: number
insight?: InsightModel
text?: TextModel
Expand Down
Loading