Skip to content

Commit

Permalink
feat: Better Notebook refreshing (#18980)
Browse files Browse the repository at this point in the history
  • Loading branch information
@benjackwhite
benjackwhite authored Dec 4, 2023
1 parent 21ef324 commit 763f652
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 90 deletions.
16 changes: 13 additions & 3 deletions frontend/src/lib/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { decompressSync, strFromU8 } from 'fflate'
import { encodeParams } from 'kea-router'
import { ActivityLogProps } from 'lib/components/ActivityLog/ActivityLog'
import { ActivityLogItem, ActivityScope } from 'lib/components/ActivityLog/humanizeActivity'
import { toParams } from 'lib/utils'
import { objectClean, toParams } from 'lib/utils'
import posthog from 'posthog-js'
import { SavedSessionRecordingPlaylistsResult } from 'scenes/session-recordings/saved-playlists/savedSessionRecordingPlaylistsLogic'

Expand Down Expand Up @@ -102,6 +102,7 @@ export interface ActivityLogPaginatedResponse<T> extends PaginatedResponse<T> {

export interface ApiMethodOptions {
signal?: AbortSignal
headers?: Record<string, any>
}

const CSRF_COOKIE_NAME = 'posthog_csrftoken'
Expand Down Expand Up @@ -1519,8 +1520,14 @@ const api = {
},

notebooks: {
async get(notebookId: NotebookType['short_id']): Promise<NotebookType> {
return await new ApiRequest().notebook(notebookId).get()
async get(
notebookId: NotebookType['short_id'],
params: Record<string, any> = {},
headers: Record<string, any> = {}
): Promise<NotebookType> {
return await new ApiRequest().notebook(notebookId).withQueryString(toParams(params)).get({
headers,
})
},
async update(
notebookId: NotebookType['short_id'],
Expand Down Expand Up @@ -1842,6 +1849,7 @@ const api = {
response = await fetch(url, {
signal: options?.signal,
headers: {
...objectClean(options?.headers ?? {}),
...(getSessionId() ? { 'X-POSTHOG-SESSION-ID': getSessionId() } : {}),
},
})
Expand All @@ -1865,6 +1873,7 @@ const api = {
const response = await fetch(url, {
method: 'PATCH',
headers: {
...objectClean(options?.headers ?? {}),
...(isFormData ? {} : { 'Content-Type': 'application/json' }),
'X-CSRFToken': getCookie(CSRF_COOKIE_NAME) || '',
...(getSessionId() ? { 'X-POSTHOG-SESSION-ID': getSessionId() } : {}),
Expand Down Expand Up @@ -1897,6 +1906,7 @@ const api = {
const response = await fetch(url, {
method: 'POST',
headers: {
...objectClean(options?.headers ?? {}),
...(isFormData ? {} : { 'Content-Type': 'application/json' }),
'X-CSRFToken': getCookie(CSRF_COOKIE_NAME) || '',
...(getSessionId() ? { 'X-POSTHOG-SESSION-ID': getSessionId() } : {}),
Expand Down
108 changes: 53 additions & 55 deletions frontend/src/scenes/notebooks/Notebook/Notebook.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,64 +86,62 @@ export function Notebook({
setContainerSize(size as 'small' | 'medium')
}, [size])

// TODO - Render a special state if the notebook is empty

if (conflictWarningVisible) {
return <NotebookConflictWarning />
} else if (!notebook && notebookLoading) {
return <NotebookLoadingState />
} else if (notebookMissing) {
return <NotFound object="notebook" />
}

return (
<BindLogic logic={notebookLogic} props={logicProps}>
<div
className={clsx(
'Notebook',
!isExpanded && 'Notebook--compact',
mode && `Notebook--${mode}`,
size === 'small' && `Notebook--single-column`,
isEditable && 'Notebook--editable'
)}
ref={ref}
>
{isTemplate && (
<LemonBanner
type="info"
className="my-4"
action={{
onClick: duplicateNotebook,
children: 'Create copy',
}}
>
<b>This is a template.</b> You can create a copy of it to edit and use as your own.
</LemonBanner>
)}

<NotebookHistoryWarning />
{shortId === SCRATCHPAD_NOTEBOOK.short_id ? (
<LemonBanner
type="info"
className="my-4"
action={{
children: 'Convert to Notebook',
onClick: duplicateNotebook,
}}
>
This is your scratchpad. It is only visible to you and is persisted only in this browser. It's a
great place to gather ideas before turning into a saved Notebook!
</LemonBanner>
) : null}

<div className="flex flex-1 justify-center">
<NotebookColumnLeft />
<ErrorBoundary>
<Editor />
</ErrorBoundary>
<NotebookColumnRight />
{conflictWarningVisible ? (
<NotebookConflictWarning />
) : !notebook && notebookLoading ? (
<NotebookLoadingState />
) : notebookMissing ? (
<NotFound object="notebook" />
) : (
<div
className={clsx(
'Notebook',
!isExpanded && 'Notebook--compact',
mode && `Notebook--${mode}`,
size === 'small' && `Notebook--single-column`,
isEditable && 'Notebook--editable'
)}
ref={ref}
>
{isTemplate && (
<LemonBanner
type="info"
className="my-4"
action={{
onClick: duplicateNotebook,
children: 'Create copy',
}}
>
<b>This is a template.</b> You can create a copy of it to edit and use as your own.
</LemonBanner>
)}

<NotebookHistoryWarning />
{shortId === SCRATCHPAD_NOTEBOOK.short_id ? (
<LemonBanner
type="info"
className="my-4"
action={{
children: 'Convert to Notebook',
onClick: duplicateNotebook,
}}
>
This is your scratchpad. It is only visible to you and is persisted only in this browser.
It's a great place to gather ideas before turning into a saved Notebook!
</LemonBanner>
) : null}

<div className="flex flex-1 justify-center">
<NotebookColumnLeft />
<ErrorBoundary>
<Editor />
</ErrorBoundary>
<NotebookColumnRight />
</div>
</div>
</div>
)}
</BindLogic>
)
}
63 changes: 35 additions & 28 deletions frontend/src/scenes/notebooks/Notebook/notebookLogic.ts
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,8 @@
import { lemonToast } from '@posthog/lemon-ui'
import {
actions,
beforeUnmount,
BuiltLogic,
connect,
kea,
key,
listeners,
path,
props,
reducers,
selectors,
sharedListeners,
} from 'kea'
import { actions, beforeUnmount, BuiltLogic, connect, kea, key, listeners, path, props, reducers, selectors } from 'kea'
import { loaders } from 'kea-loaders'
import { router, urlToAction } from 'kea-router'
import { subscriptions } from 'kea-subscriptions'
import api from 'lib/api'
import { downloadFile, slugify } from 'lib/utils'
import posthog from 'posthog-js'
Expand All @@ -34,6 +22,7 @@ import type { notebookLogicType } from './notebookLogicType'
import { EditorRange, JSONContent, NotebookEditor } from './utils'

const SYNC_DELAY = 1000
const NOTEBOOK_REFRESH_MS = window.location.origin === 'http://localhost:8000' ? 5000 : 30000

export type NotebookLogicMode = 'notebook' | 'canvas'

Expand Down Expand Up @@ -81,6 +70,7 @@ export const notebookLogic = kea<notebookLogicType>([
setPreviewContent: (jsonContent: JSONContent) => ({ jsonContent }),
clearPreviewContent: true,
loadNotebook: true,
scheduleNotebookRefresh: true,
saveNotebook: (notebook: Pick<NotebookType, 'content' | 'title'>) => ({ notebook }),
renameNotebook: (title: string) => ({ title }),
setEditingNodeId: (editingNodeId: string | null) => ({ editingNodeId }),
Expand Down Expand Up @@ -221,8 +211,14 @@ export const notebookLogic = kea<notebookLogicType>([
}
} else {
try {
response = await api.notebooks.get(props.shortId)
response = await api.notebooks.get(props.shortId, undefined, {
'If-None-Match': values.notebook?.version,
})
} catch (e: any) {
if (e.status === 304) {
// Indicates nothing has changed
return values.notebook
}
if (e.status === 404) {
return null
}
Expand All @@ -232,7 +228,7 @@ export const notebookLogic = kea<notebookLogicType>([

const notebook = migrate(response)

if (!values.notebook && notebook.content) {
if (notebook.content && (!values.notebook || values.notebook.version !== notebook.version)) {
// If this is the first load we need to override the content fully
values.editor?.setContent(notebook.content)
}
Expand Down Expand Up @@ -421,15 +417,7 @@ export const notebookLogic = kea<notebookLogicType>([
(shouldBeEditable, previewContent) => shouldBeEditable && !previewContent,
],
}),
sharedListeners(({ values, actions }) => ({
onNotebookChange: () => {
// Keep the list logic up to date with any changes
if (values.notebook && values.notebook.short_id !== SCRATCHPAD_NOTEBOOK.short_id) {
actions.receiveNotebookUpdate(values.notebook)
}
},
})),
listeners(({ values, actions, sharedListeners, cache }) => ({
listeners(({ values, actions, cache }) => ({
insertAfterLastNode: async ({ content }) => {
await runWhenEditorIsReady(
() => !!values.editor,
Expand Down Expand Up @@ -560,8 +548,8 @@ export const notebookLogic = kea<notebookLogicType>([
values.editor?.setContent(values.content)
},

saveNotebookSuccess: sharedListeners.onNotebookChange,
loadNotebookSuccess: sharedListeners.onNotebookChange,
saveNotebookSuccess: actions.scheduleNotebookRefresh,
loadNotebookSuccess: actions.scheduleNotebookRefresh,

exportJSON: () => {
const file = new File(
Expand Down Expand Up @@ -592,6 +580,24 @@ export const notebookLogic = kea<notebookLogicType>([
values.editor?.setTextSelection(selection)
})
},

scheduleNotebookRefresh: () => {
clearTimeout(cache.refreshTimeout)
cache.refreshTimeout = setTimeout(() => {
actions.loadNotebook()
}, NOTEBOOK_REFRESH_MS)
},
})),

subscriptions(({ actions }) => ({
notebook: (notebook?: NotebookType) => {
// Keep the list logic up to date with any changes
if (notebook && notebook.short_id !== SCRATCHPAD_NOTEBOOK.short_id) {
actions.receiveNotebookUpdate(notebook)
}
// If the notebook ever changes, we want to reset the scheduled refresh
actions.scheduleNotebookRefresh()
},
})),

urlToAction(({ values, actions, cache }) => ({
Expand All @@ -606,7 +612,8 @@ export const notebookLogic = kea<notebookLogicType>([
},
})),

beforeUnmount(() => {
beforeUnmount(({ cache }) => {
clearTimeout(cache.refreshTimeout)
const hashParams = router.values.currentLocation.hashParams
delete hashParams['🦔']
router.actions.replace(
Expand Down
19 changes: 15 additions & 4 deletions posthog/api/notebook.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
extend_schema_view,
OpenApiExample,
)
from rest_framework import request, serializers, viewsets
from rest_framework import serializers, viewsets
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.decorators import action
from rest_framework.permissions import IsAuthenticated

Expand Down Expand Up @@ -253,7 +255,7 @@ def get_queryset(self) -> QuerySet:

return queryset

def _filter_request(self, request: request.Request, queryset: QuerySet) -> QuerySet:
def _filter_request(self, request: Request, queryset: QuerySet) -> QuerySet:
filters = request.GET.dict()

for key in filters:
Expand Down Expand Up @@ -329,16 +331,25 @@ def _filter_request(self, request: request.Request, queryset: QuerySet) -> Query

return queryset

def retrieve(self, request: Request, *args: Any, **kwargs: Any) -> Response:
instance = self.get_object()
serializer = self.get_serializer(instance)

if str(request.headers.get("If-None-Match")) == str(instance.version):
return Response(None, 304)

return Response(serializer.data)

@action(methods=["GET"], url_path="activity", detail=False)
def all_activity(self, request: request.Request, **kwargs):
def all_activity(self, request: Request, **kwargs):
limit = int(request.query_params.get("limit", "10"))
page = int(request.query_params.get("page", "1"))

activity_page = load_activity(scope="Notebook", team_id=self.team_id, limit=limit, page=page)
return activity_page_response(activity_page, limit, page, request)

@action(methods=["GET"], url_path="activity", detail=True)
def activity(self, request: request.Request, **kwargs):
def activity(self, request: Request, **kwargs):
notebook = self.get_object()
limit = int(request.query_params.get("limit", "10"))
page = int(request.query_params.get("page", "1"))
Expand Down
14 changes: 14 additions & 0 deletions posthog/api/test/notebooks/test_notebook.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,3 +229,17 @@ def test_patching_does_not_leak_between_teams(self) -> None:
data={"content": {"something": "here"}},
)
assert response.status_code == status.HTTP_403_FORBIDDEN

def test_responds_not_modified_if_versions_match(self) -> None:
response = self.client.post(
f"/api/projects/{self.team.id}/notebooks",
data={"content": {}, "text_content": ""},
)
assert response.status_code == status.HTTP_201_CREATED

response = self.client.get(
f"/api/projects/{self.team.id}/notebooks/{response.json()['short_id']}",
HTTP_IF_NONE_MATCH=response.json()["version"],
)

assert response.status_code == status.HTTP_304_NOT_MODIFIED

0 comments on commit 763f652

Please sign in to comment.