From ea4f574d017a86c745d36301597407652ea3ee3d Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 12 Oct 2023 10:36:38 +0200 Subject: [PATCH] fix: Notebook select matching (#17916) --- .../notebooks/Nodes/NotebookNodeFlag.tsx | 95 ------------------- .../notebooks/Notebook/notebookLogic.ts | 2 +- .../NotebookSelectButton.tsx | 14 ++- posthog/api/notebook.py | 9 ++ .../test/notebooks/test_notebook_filtering.py | 20 ++-- 5 files changed, 23 insertions(+), 117 deletions(-) diff --git a/frontend/src/scenes/notebooks/Nodes/NotebookNodeFlag.tsx b/frontend/src/scenes/notebooks/Nodes/NotebookNodeFlag.tsx index 86cee27b22abe..92a5646dce29e 100644 --- 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 ) : null} - - {/* -
- {canCreateEarlyAccessFeature && ( - } - 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 - - )} - } - 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 - - } - 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 - - { - // 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={} - disabledReason={ - nextNode?.type.name === NotebookNodeType.RecordingPlaylist && - 'Recording playlist already exists below' - } - > - View Replays - -
*/} ) diff --git a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts index f5659c89771d2..20da64681b722 100644 --- a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts +++ b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts @@ -491,7 +491,7 @@ export const notebookLogic = kea([ 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 index 1a48b6af9214a..b04e2a4fe4e2c 100644 --- 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 = ( } + icon={ + + + + } 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 ( - {nodeLogic ? ( - button - ) : ( - - {button} - - )} + {nodeLogic ? button : {button}} ) } diff --git a/posthog/api/notebook.py b/posthog/api/notebook.py index 01eab8dd599ec..b9a61b0141142 100644 --- 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 index 5f634de548fc7..bc4797721d013 100644 --- 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( [