fix: Notebook select matching (#17916)

PostHog · Oct 12, 2023 · ea4f574 · ea4f574
1 parent e47c2aa
commit ea4f574
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 117 deletions.
diff --git a/frontend/src/scenes/notebooks/Nodes/NotebookNodeFlag.tsx b/frontend/src/scenes/notebooks/Nodes/NotebookNodeFlag.tsx
@@ -123,101 +123,6 @@ const Component = ({ attributes, updateAttributes }: NotebookNodeProps<NotebookN
                         </div>
                     </>
                 ) : null}
-
-                {/* <LemonDivider className="my-0" />
-                <div className="p-2 mr-1 flex justify-end gap-2">
-                    {canCreateEarlyAccessFeature && (
-                        <LemonButton
-                            type="secondary"
-                            size="small"
-                            icon={<IconRocketLaunch />}
-                            loading={newEarlyAccessFeatureLoading}
-                            onClick={(e) => {
-                                // prevent expanding the node if it isn't expanded
-                                e.stopPropagation()
-
-                                if (!hasEarlyAccessFeatures) {
-                                    createEarlyAccessFeature()
-                                } else {
-                                    if ((featureFlag?.features?.length || 0) <= 0) {
-                                        return
-                                    }
-                                    if (!shouldDisableInsertEarlyAccessFeature(nextNode) && featureFlag.features) {
-                                        insertAfter(buildEarlyAccessFeatureContent(featureFlag.features[0].id))
-                                    }
-                                }
-                            }}
-                            disabledReason={
-                                shouldDisableInsertEarlyAccessFeature(nextNode) &&
-                                'Early access feature already exists below'
-                            }
-                        >
-                            {hasEarlyAccessFeatures ? 'View' : 'Create'} early access feature
-                        </LemonButton>
-                    )}
-                    <LemonButton
-                        type="secondary"
-                        size="small"
-                        icon={<IconSurveys />}
-                        loading={newSurveyLoading}
-                        onClick={(e) => {
-                            // prevent expanding the node if it isn't expanded
-                            e.stopPropagation()
-
-                            if (!hasSurveys) {
-                                createSurvey()
-                            } else {
-                                if ((featureFlag?.surveys?.length || 0) <= 0) {
-                                    return
-                                }
-                                if (!shouldDisableInsertSurvey(nextNode) && featureFlag.surveys) {
-                                    insertAfter(buildSurveyContent(featureFlag.surveys[0].id))
-                                }
-                            }
-                        }}
-                        disabledReason={shouldDisableInsertSurvey(nextNode) && 'Survey already exists below'}
-                    >
-                        {hasSurveys ? 'View' : 'Create'} survey
-                    </LemonButton>
-                    <LemonButton
-                        type="secondary"
-                        size="small"
-                        icon={<IconFlag />}
-                        onClick={(e) => {
-                            // prevent expanding the node if it isn't expanded
-                            e.stopPropagation()
-
-                            if (nextNode?.type.name !== NotebookNodeType.FeatureFlagCodeExample) {
-                                insertAfter(buildCodeExampleContent(id))
-                            }
-                        }}
-                        disabledReason={
-                            nextNode?.type.name === NotebookNodeType.FeatureFlagCodeExample &&
-                            'Code example already exists below'
-                        }
-                    >
-                        Show implementation
-                    </LemonButton>
-                    <LemonButton
-                        onClick={(e) => {
-                            // prevent expanding the node if it isn't expanded
-                            e.stopPropagation()
-
-                            if (nextNode?.type.name !== NotebookNodeType.RecordingPlaylist) {
-                                insertAfter(buildPlaylistContent(recordingFilterForFlag))
-                            }
-                        }}
-                        type="secondary"
-                        size="small"
-                        icon={<IconRecording />}
-                        disabledReason={
-                            nextNode?.type.name === NotebookNodeType.RecordingPlaylist &&
-                            'Recording playlist already exists below'
-                        }
-                    >
-                        View Replays
-                    </LemonButton>
-                </div> */}
             </BindLogic>
         </div>
     )

diff --git a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts
@@ -491,7 +491,7 @@ export const notebookLogic = kea<notebookLogicType>([
 
         exportJSON: () => {
             const file = new File(
-                [JSON.stringify(values.editor?.getJSON())],
+                [JSON.stringify(values.editor?.getJSON(), null, 2)],
                 `${slugify(values.title ?? 'untitled')}.ph-notebook.json`,
                 { type: 'application/json' }
             )

diff --git a/frontend/src/scenes/notebooks/NotebookSelectButton/NotebookSelectButton.tsx b/frontend/src/scenes/notebooks/NotebookSelectButton/NotebookSelectButton.tsx
@@ -204,7 +204,11 @@ export function NotebookSelectButton({ children, ...props }: NotebookSelectButto
 
     const button = (
         <LemonButton
-            icon={<IconJournalPlus />}
+            icon={
+                <IconWithCount count={notebooksContainingResource.length ?? 0} showZero={false}>
+                    <IconJournalPlus />
+                </IconWithCount>
+            }
             data-attr={nodeLogic ? 'notebooks-add-button-in-a-notebook' : 'notebooks-add-button'}
             sideIcon={null}
             {...props}
@@ -224,13 +228,7 @@ export function NotebookSelectButton({ children, ...props }: NotebookSelectButto
 
     return (
         <FlaggedFeature flag={FEATURE_FLAGS.NOTEBOOKS} match>
-            {nodeLogic ? (
-                button
-            ) : (
-                <IconWithCount count={notebooksContainingResource.length ?? 0} showZero={false}>
-                    <NotebookSelectPopover {...props}>{button}</NotebookSelectPopover>
-                </IconWithCount>
-            )}
+            {nodeLogic ? button : <NotebookSelectPopover {...props}>{button}</NotebookSelectPopover>}
         </FlaggedFeature>
     )
 }
diff --git a/posthog/api/notebook.py b/posthog/api/notebook.py
@@ -276,6 +276,15 @@ def _filter_request(self, request: request.Request, queryset: QuerySet) -> Query
                         nested_structure = basic_structure | List[Dict[str, basic_structure]]
 
                         presence_match_structure: basic_structure | nested_structure = [{"type": f"ph-{target}"}]
+
+                        try:
+                            # We try to parse the match as a number, as query params are always strings,
+                            # but an id could be an integer and wouldn't match
+                            if isinstance(match, str):  # because mypy
+                                match = int(match)
+                        except (ValueError, TypeError):
+                            pass
+
                         id_match_structure: basic_structure | nested_structure = [{"attrs": {"id": match}}]
                         if target == "replay-timestamp":
                             # replay timestamps are not at the top level, they're one-level down in a content array

diff --git a/posthog/api/test/notebooks/test_notebook_filtering.py b/posthog/api/test/notebooks/test_notebook_filtering.py
@@ -17,7 +17,7 @@
 FEATURE_FLAG_CONTENT = lambda id: {
     "type": "ph-feature-flag",
     "attrs": {
-        "id": id or "feature_flag_id",
+        "id": id or 1,
     },
 }
 
@@ -134,7 +134,7 @@ def test_filters_based_on_params(self) -> None:
     def test_filtering_by_types(self) -> None:
         playlist_content_notebook = self._create_notebook_with_content([PLAYLIST_CONTENT()])
         insight_content_notebook = self._create_notebook_with_content([INSIGHT_COMMENT("insight_id")])
-        feature_flag_content_notebook = self._create_notebook_with_content([FEATURE_FLAG_CONTENT("feature_flag_id")])
+        feature_flag_content_notebook = self._create_notebook_with_content([FEATURE_FLAG_CONTENT(1)])
         person_content_notebook = self._create_notebook_with_content([PERSON_CONTENT("person_id")])
         recording_comment_notebook = self._create_notebook_with_content(
             [RECORDING_COMMENT_CONTENT("session_recording_id", None)]
@@ -178,7 +178,7 @@ def test_filtering_by_types(self) -> None:
     def test_filtering_by_abscence_of_types(self) -> None:
         playlist_content_notebook = self._create_notebook_with_content([PLAYLIST_CONTENT()])
         insight_content_notebook = self._create_notebook_with_content([INSIGHT_COMMENT("insight_id")])
-        feature_flag_content_notebook = self._create_notebook_with_content([FEATURE_FLAG_CONTENT("feature_flag_id")])
+        feature_flag_content_notebook = self._create_notebook_with_content([FEATURE_FLAG_CONTENT(1)])
         person_content_notebook = self._create_notebook_with_content([PERSON_CONTENT("person_id")])
         recording_comment_notebook = self._create_notebook_with_content(
             [RECORDING_COMMENT_CONTENT("session_recording_id", None)]
@@ -270,9 +270,7 @@ def test_filtering_by_abscence_of_types(self) -> None:
     @parameterized.expand([["insight"], ["insights"]])
     def test_filtering_by_just_the_target_name_is_truthy(self, target_name: str) -> None:
         insight_content_notebook_one = self._create_notebook_with_content([INSIGHT_COMMENT("insight_id_one")])
-        _feature_flag_content_notebook_one = self._create_notebook_with_content(
-            [FEATURE_FLAG_CONTENT("feature_flag_id_one")]
-        )
+        _feature_flag_content_notebook_one = self._create_notebook_with_content([FEATURE_FLAG_CONTENT(1)])
         filter_response = self.client.get(f"/api/projects/{self.team.id}/notebooks?contains={target_name}")
         assert sorted([n["id"] for n in filter_response.json()["results"]]) == sorted(
             [
@@ -286,12 +284,8 @@ def test_filtering_by_id_of_types(self) -> None:
         insight_content_notebook_one = self._create_notebook_with_content([INSIGHT_COMMENT("insight_id_one")])
         _insight_content_notebook_two = self._create_notebook_with_content([INSIGHT_COMMENT("insight_id_two")])
 
-        feature_flag_content_notebook_one = self._create_notebook_with_content(
-            [FEATURE_FLAG_CONTENT("feature_flag_id_one")]
-        )
-        _feature_flag_content_notebook_two = self._create_notebook_with_content(
-            [FEATURE_FLAG_CONTENT("feature_flag_id_two")]
-        )
+        feature_flag_content_notebook_one = self._create_notebook_with_content([FEATURE_FLAG_CONTENT(1)])
+        _feature_flag_content_notebook_two = self._create_notebook_with_content([FEATURE_FLAG_CONTENT(2)])
 
         person_content_notebook_one = self._create_notebook_with_content([PERSON_CONTENT("person_id_one")])
         _person_content_notebook_two = self._create_notebook_with_content([PERSON_CONTENT("person_id_two")])
@@ -326,7 +320,7 @@ def test_filtering_by_id_of_types(self) -> None:
 
         # filter by feature flag
         feature_flag_filter_response = self.client.get(
-            f"/api/projects/{self.team.id}/notebooks?contains=feature-flag:feature_flag_id_one"
+            f"/api/projects/{self.team.id}/notebooks?contains=feature-flag:1"
         )
         assert sorted([n["id"] for n in feature_flag_filter_response.json()["results"]]) == sorted(
             [