Reload saved insights if editing an insight + other fixes (#6932)

* refactor saved insights pagination

* create a system to mark logics as stale

* introduce staleness for feature flags

* reload logic results without a custom abstraction

* add test for saved insight reloads

* remove dead code

* update persons index when editing a person

* scroll to top if the page changed

* do not scroll to top if location changed via back/forward

* sync delete flags

* remove dead code

* remove dead code

* revert change

* use ?page=1

* make test pass

frontend/src/scenes/feature-flags/featureFlagLogic.tsx

@@ -239,7 +239,6 @@ export const featureFlagLogic = kea<featureFlagLogicType>({
                     closeOnClick: true,
                 }
             )
-
             featureFlagsLogic.findMounted()?.actions.updateFlag(featureFlag)
         },
         deleteFeatureFlag: async ({ featureFlag }) => {
@@ -247,6 +246,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>({
                 endpoint: `projects/${values.currentTeamId}/feature_flags`,
                 object: { name: featureFlag.name, id: featureFlag.id },
                 callback: () => {
+                    featureFlag.id && featureFlagsLogic.findMounted()?.actions.deleteFlag(featureFlag.id)
                     router.actions.push(urls.featureFlags())
                 },
             })

frontend/src/scenes/feature-flags/featureFlagsLogic.ts

@@ -12,6 +12,7 @@ export const featureFlagsLogic = kea<featureFlagsLogicType>({
     },
     actions: {
         updateFlag: (flag: FeatureFlagType) => ({ flag }),
+        deleteFlag: (id: number) => ({ id }),
         setSearchTerm: (searchTerm: string) => ({ searchTerm }),
     },
     loaders: ({ values }) => ({
@@ -55,6 +56,7 @@ export const featureFlagsLogic = kea<featureFlagsLogicType>({
                     return [flag, ...state]
                 }
             },
+            deleteFlag: (state, { id }) => state.filter((flag) => flag.id !== id),
         },
     },
     events: ({ actions }) => ({

frontend/src/scenes/insights/insightLogic.test.ts

@@ -536,4 +536,19 @@ describe('insightLogic', () => {
             filtersChanged: false,
         })
     })
+
+    test('saveInsight and updateInsight reload the saved insights list', async () => {
+        savedInsightsLogic.mount()
+        logic = insightLogic({
+            dashboardItemId: 42,
+            filters: { insight: 'FUNNELS' },
+        })
+        logic.mount()
+
+        logic.actions.saveInsight()
+        await expectLogic(savedInsightsLogic).toDispatchActions(['loadInsights'])
+
+        logic.actions.updateInsight({ filters: { insight: 'FUNNELS' } })
+        await expectLogic(savedInsightsLogic).toDispatchActions(['loadInsights'])
+    })
 })

frontend/src/scenes/insights/insightLogic.tsx

@@ -31,6 +31,7 @@ import { teamLogic } from '../teamLogic'
 import { Scene } from 'scenes/sceneTypes'
 import { userLogic } from 'scenes/userLogic'
 import { sceneLogic } from 'scenes/sceneLogic'
+import { savedInsightsLogic } from 'scenes/saved-insights/savedInsightsLogic'
 import { urls } from 'scenes/urls'
 
 const IS_TEST_MODE = process.env.NODE_ENV === 'test'
@@ -124,6 +125,7 @@ export const insightLogic = kea<insightLogicType>({
                     breakpoint()
                     const updatedInsight = { ...response, result: response.result || values.insight.result }
                     callback?.(updatedInsight)
+                    savedInsightsLogic.findMounted()?.actions.loadInsights()
                     return updatedInsight
                 },
                 // using values.filters, query for new insight results
@@ -532,6 +534,7 @@ export const insightLogic = kea<insightLogicType>({
                     <Link to={'/saved_insights'}>Click here to see your list of saved insights</Link>
                 </div>
             )
+            savedInsightsLogic.findMounted()?.actions.loadInsights()
         },
         loadInsightSuccess: async ({ payload, insight }) => {
             // loaded `/api/projects/:id/insights`, but it didn't have `results`, so make another query

frontend/src/scenes/persons/personsLogic.ts

@@ -25,6 +25,9 @@ export const personsLogic = kea<personsLogicType<PersonPaginatedResponse>>({
         values: [featureFlagLogic, ['featureFlags'], teamLogic, ['currentTeam']],
     },
     actions: {
+        setPerson: (person: PersonType) => ({ person }),
+        loadPerson: (id: string) => ({ id }),
+        loadPersons: (url: string | null = '') => ({ url }),
         setListFilters: (payload) => ({ payload }),
         editProperty: (key: string, newValue?: string | number | boolean | null) => ({ key, newValue }),
         setHasNewKeys: true,
@@ -57,6 +60,12 @@ export const personsLogic = kea<personsLogicType<PersonPaginatedResponse>>({
                 setSplitMergeModalShown: (_, { shown }) => shown,
             },
         ],
+        persons: {
+            setPerson: (state, { person }) => ({
+                ...state,
+                results: state.results.map((p) => (p.id === person.id ? person : p)),
+            }),
+        },
     },
     selectors: {
         exampleEmail: [
@@ -156,7 +165,7 @@ export const personsLogic = kea<personsLogicType<PersonPaginatedResponse>>({
         persons: [
             { next: null, previous: null, results: [] } as PersonPaginatedResponse,
             {
-                loadPersons: async (url: string | null = '') => {
+                loadPersons: async ({ url }) => {
                     if (!url) {
                         const qs = Object.keys(values.listFilters)
                             .filter((key) =>
@@ -180,7 +189,7 @@ export const personsLogic = kea<personsLogicType<PersonPaginatedResponse>>({
         person: [
             null as PersonType | null,
             {
-                loadPerson: async (id: string): Promise<PersonType | null> => {
+                loadPerson: async ({ id }): Promise<PersonType | null> => {
                     const response = await api.get(`api/person/?distinct_id=${id}`)
                     if (!response.results.length) {
                         router.actions.push(urls.notFound())
@@ -189,7 +198,7 @@ export const personsLogic = kea<personsLogicType<PersonPaginatedResponse>>({
                     person && actions.reportPersonDetailViewed(person)
                     return person
                 },
-                setPerson: (person: PersonType): PersonType => {
+                setPerson: ({ person }): PersonType => {
                     // Used after merging persons to update the view without an additional request
                     return person
                 },

frontend/src/scenes/saved-insights/SavedInsights.tsx

@@ -5,7 +5,7 @@ import { ObjectTags } from 'lib/components/ObjectTags'
 import { deleteWithUndo } from 'lib/utils'
 import React from 'react'
 import { DashboardItemType, LayoutView, SavedInsightsTabs, ViewType } from '~/types'
-import { savedInsightsLogic } from './savedInsightsLogic'
+import { INSIGHTS_PER_PAGE, savedInsightsLogic } from './savedInsightsLogic'
 import {
     AppstoreFilled,
     ArrowDownOutlined,
@@ -121,33 +121,17 @@ const columnSort = (direction: 'up' | 'down' | 'none'): JSX.Element => (
 )
 
 export function SavedInsights(): JSX.Element {
-    const {
-        loadInsights,
-        updateFavoritedInsight,
-        loadPaginatedInsights,
-        renameInsight,
-        duplicateInsight,
-        setSavedInsightsFilters,
-    } = useActions(savedInsightsLogic)
-    const { insights, count, offset, nextResult, previousResult, insightsLoading, filters } =
-        useValues(savedInsightsLogic)
+    const { loadInsights, updateFavoritedInsight, renameInsight, duplicateInsight, setSavedInsightsFilters } =
+        useActions(savedInsightsLogic)
+    const { insights, count, insightsLoading, filters } = useValues(savedInsightsLogic)
 
     const { hasDashboardCollaboration } = useValues(organizationLogic)
     const { currentTeamId } = useValues(teamLogic)
     const { members } = useValues(membersLogic)
-    const { tab, order, createdBy, layoutView, search, insightType, dateFrom, dateTo } = filters
+    const { tab, order, createdBy, layoutView, search, insightType, dateFrom, dateTo, page } = filters
 
-    const pageLimit = 15
-    const paginationCount = (): number => {
-        if (!previousResult) {
-            // no previous url means it's the first result set
-            return 1
-        }
-        if (nextResult) {
-            return offset - pageLimit
-        }
-        return count - (insights?.results.length || 0)
-    }
+    const startCount = (page - 1) * INSIGHTS_PER_PAGE + 1
+    const endCount = page * INSIGHTS_PER_PAGE < count ? page * INSIGHTS_PER_PAGE : count
 
     const columns: ColumnsType<DashboardItemType> = [
         {
@@ -438,7 +422,7 @@ export function SavedInsights(): JSX.Element {
             </Row>
             {insights.count > 0 && (
                 <Row className="list-or-card-layout">
-                    Showing {paginationCount()} - {nextResult ? offset : count} of {count} insights
+                    Showing {startCount} - {endCount} of {count} insights
                     <div>
                         <Radio.Group
                             onChange={(e) => setSavedInsightsFilters({ layoutView: e.target.value })}
@@ -472,21 +456,27 @@ export function SavedInsights(): JSX.Element {
                                 <Row className="footer-pagination">
                                     <span className="text-muted-alt">
                                         {insights.count > 0 &&
-                                            `Showing ${paginationCount()} - ${
-                                                nextResult ? offset : count
-                                            } of ${count} insights`}
+                                            `Showing ${startCount} - ${endCount} of ${count} insights`}
                                     </span>
                                     <LeftOutlined
                                         style={{ paddingRight: 16 }}
-                                        className={`${!previousResult ? 'paginate-disabled' : ''}`}
+                                        className={`${page === 1 ? 'paginate-disabled' : ''}`}
                                         onClick={() => {
-                                            previousResult && loadPaginatedInsights(previousResult)
+                                            if (page > 1) {
+                                                setSavedInsightsFilters({
+                                                    page: page - 1,
+                                                })
+                                            }
                                         }}
                                     />
                                     <RightOutlined
-                                        className={`${!nextResult ? 'paginate-disabled' : ''}`}
+                                        className={`${page * INSIGHTS_PER_PAGE >= count ? 'paginate-disabled' : ''}`}
                                         onClick={() => {
-                                            nextResult && loadPaginatedInsights(nextResult)
+                                            if (page * INSIGHTS_PER_PAGE < count) {
+                                                setSavedInsightsFilters({
+                                                    page: page + 1,
+                                                })
+                                            }
                                         }}
                                     />
                                 </Row>

frontend/src/scenes/saved-insights/savedInsightsLogic.ts

@@ -12,6 +12,8 @@ import { teamLogic } from '../teamLogic'
 import { urls } from 'scenes/urls'
 import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
 
+export const INSIGHTS_PER_PAGE = 15
+
 export interface InsightsResult {
     results: DashboardItemType[]
     count: number
@@ -28,8 +30,9 @@ export interface SavedInsightFilters {
     search: string
     insightType: string
     createdBy: number | 'All users'
-    dateFrom?: string | Dayjs | undefined
-    dateTo?: string | Dayjs | undefined
+    dateFrom: string | Dayjs | undefined | 'all'
+    dateTo: string | Dayjs | undefined
+    page: number
 }
 
 function cleanFilters(values: Partial<SavedInsightFilters>): SavedInsightFilters {
@@ -42,6 +45,7 @@ function cleanFilters(values: Partial<SavedInsightFilters>): SavedInsightFilters
         createdBy: (values.tab !== SavedInsightsTabs.Yours && values.createdBy) || 'All users',
         dateFrom: values.dateFrom || 'all',
         dateTo: values.dateTo || undefined,
+        page: parseInt(String(values.page)) || 1,
     }
 }
 
@@ -67,23 +71,7 @@ export const savedInsightsLogic = kea<savedInsightsLogicType<InsightsResult, Sav
                     await breakpoint(300)
                 }
                 const { filters } = values
-                const params = {
-                    order: filters.order,
-                    limit: 15,
-                    saved: true,
-                    ...(filters.tab === SavedInsightsTabs.Yours && { user: true }),
-                    ...(filters.tab === SavedInsightsTabs.Favorites && { favorited: true }),
-                    ...(filters.search && { search: filters.search }),
-                    ...(filters.insightType?.toLowerCase() !== 'all types' && {
-                        insight: filters.insightType?.toUpperCase(),
-                    }),
-                    ...(filters.createdBy !== 'All users' && { created_by: filters.createdBy }),
-                    ...(filters.dateFrom &&
-                        filters.dateFrom !== 'all' && {
-                            date_from: filters.dateFrom,
-                            date_to: filters.dateTo,
-                        }),
-                }
+                const params = values.paramsFromFilters
                 const response = await api.get(
                     `api/projects/${teamLogic.values.currentTeamId}/insights/?${toParams(params)}`
                 )
@@ -104,9 +92,13 @@ export const savedInsightsLogic = kea<savedInsightsLogicType<InsightsResult, Sav
                     }
                 }
 
+                // scroll to top if the page changed, except if changed via back/forward
+                if (router.values.lastMethod !== 'POP' && values.insights.filters?.page !== filters.page) {
+                    window.scrollTo(0, 0)
+                }
+
                 return { ...response, filters }
             },
-            loadPaginatedInsights: async (url: string) => await api.get(url),
             updateFavoritedInsight: async ({ id, favorited }) => {
                 const response = await api.update(`api/projects/${teamLogic.values.currentTeamId}/insights/${id}`, {
                     favorited,
@@ -140,20 +132,32 @@ export const savedInsightsLogic = kea<savedInsightsLogicType<InsightsResult, Sav
     },
     selectors: {
         filters: [(s) => [s.rawFilters], (rawFilters): SavedInsightFilters => cleanFilters(rawFilters || {})],
-        nextResult: [(s) => [s.insights], (insights) => insights.next],
-        previousResult: [(s) => [s.insights], (insights) => insights.previous],
         count: [(s) => [s.insights], (insights) => insights.count],
-        offset: [
-            (s) => [s.insights],
-            (insights) => {
-                const offset = new URLSearchParams(insights.next).get('offset') || '0'
-                return parseInt(offset)
-            },
-        ],
         usingFilters: [
             (s) => [s.filters],
             (filters) => !objectsEqual(cleanFilters({ ...filters, tab: SavedInsightsTabs.All }), cleanFilters({})),
         ],
+        paramsFromFilters: [
+            (s) => [s.filters],
+            (filters) => ({
+                order: filters.order,
+                limit: INSIGHTS_PER_PAGE,
+                offset: Math.max(0, (filters.page - 1) * INSIGHTS_PER_PAGE),
+                saved: true,
+                ...(filters.tab === SavedInsightsTabs.Yours && { user: true }),
+                ...(filters.tab === SavedInsightsTabs.Favorites && { favorited: true }),
+                ...(filters.search && { search: filters.search }),
+                ...(filters.insightType?.toLowerCase() !== 'all types' && {
+                    insight: filters.insightType?.toUpperCase(),
+                }),
+                ...(filters.createdBy !== 'All users' && { created_by: filters.createdBy }),
+                ...(filters.dateFrom &&
+                    filters.dateFrom !== 'all' && {
+                        date_from: filters.dateFrom,
+                        date_to: filters.dateTo,
+                    }),
+            }),
+        ],
     },
     listeners: ({ actions, values, selectors }) => ({
         addGraph: ({ type }) => {
@@ -219,20 +223,11 @@ export const savedInsightsLogic = kea<savedInsightsLogicType<InsightsResult, Sav
         },
     }),
     actionToUrl: ({ values }) => {
-        const changeUrl = ():
-            | [
-                  string,
-                  Record<string, any>,
-                  Record<string, any>,
-                  {
-                      replace: true
-                  }
-              ]
-            | void => {
+        const changeUrl = (): [string, Record<string, any>, Record<string, any>, { replace: boolean }] | void => {
             const nextValues = cleanFilters(values.filters)
             const urlValues = cleanFilters(router.values.searchParams)
             if (!objectsEqual(nextValues, urlValues)) {
-                return ['/saved_insights', objectDiffShallow(cleanFilters({}), nextValues), {}, { replace: true }]
+                return ['/saved_insights', objectDiffShallow(cleanFilters({}), nextValues), {}, { replace: false }]
             }
         }
         return {