Merge branch 'master' into surveys-reorder-questions

.eslintrc.js

@@ -42,13 +42,25 @@ module.exports = {
         },
         ecmaVersion: 2018,
         sourceType: 'module',
-        project: 'tsconfig.json'
+        project: 'tsconfig.json',
     },
-    plugins: ['prettier', 'react', 'cypress', '@typescript-eslint', 'no-only-tests', 'jest', 'compat', 'posthog'],
+    plugins: [
+        'prettier',
+        'react',
+        'cypress',
+        '@typescript-eslint',
+        'no-only-tests',
+        'jest',
+        'compat',
+        'posthog',
+        'simple-import-sort',
+    ],
     rules: {
         'no-console': ['error', { allow: ['warn', 'error'] }],
         'no-debugger': 'error',
         'no-only-tests/no-only-tests': 'error',
+        'simple-import-sort/imports': 'error',
+        'simple-import-sort/exports': 'error',
         'react/prop-types': [0],
         'react/react-in-jsx-scope': [0],
         'react/no-unescaped-entities': [0],
@@ -112,6 +124,11 @@ module.exports = {
                         importNames: ['Tooltip'],
                         message: 'Please use Tooltip from @posthog/lemon-ui instead.',
                     },
+                    {
+                        name: 'antd',
+                        importNames: ['Alert'],
+                        message: 'Please use LemonBanner from @posthog/lemon-ui instead.',
+                    },
                 ],
             },
         ],
@@ -231,6 +248,10 @@ module.exports = {
                         element: 'a',
                         message: 'use <Link> instead',
                     },
+                    {
+                        element: 'Alert',
+                        message: 'use <LemonBanner> instead',
+                    },
                 ],
             },
         ],
@@ -254,14 +275,22 @@ module.exports = {
             rules: {
                 // The below complains needlessly about expect(api.createInvite).toHaveBeenCalledWith(...)
                 '@typescript-eslint/unbound-method': 'off',
-            }
+            },
         },
         {
-            // disable these rules for files generated by kea-typegen
-            files: ['*Type.ts', '*Type.tsx'],
+            files: ['*Type.ts', '*Type.tsx'], // Kea typegen output
             rules: {
                 'no-restricted-imports': 'off',
-                '@typescript-eslint/ban-types': ['off'],
+                '@typescript-eslint/ban-types': 'off',
+                'simple-import-sort/imports': 'off',
+                'simple-import-sort/exports': 'off',
+            },
+        },
+        {
+            files: ['frontend/src/scenes/notebooks/Nodes/*'], // Notebooks code weirdly relies on its order of sorting
+            rules: {
+                'simple-import-sort/imports': 'off',
+                'simple-import-sort/exports': 'off',
             },
         },
         {

.github/workflows/ci-frontend.yml

@@ -2,10 +2,6 @@ name: Frontend CI
 
 on:
     pull_request:
-    # NOTE: by running on master, aside from highlight issues on master it also
-    # ensures we have e.g. node modules cached for master, which can then be
-    # used for branches. See https://github.com/actions/cache#cache-scopes for
-    # scope details.
     push:
         branches:
             - master
@@ -15,53 +11,103 @@ concurrency:
     cancel-in-progress: true
 
 jobs:
+    # Job to decide if we should run frontend ci
+    # See https://github.com/dorny/paths-filter#conditional-execution for more details
+    # we skip each step individually, so   they are still reported as success
+    # because many of them are required for CI checks to be green
+    changes:
+        runs-on: ubuntu-latest
+        timeout-minutes: 5
+        name: Determine need to run frontend checks
+        outputs:
+            frontend: ${{ steps.filter.outputs.frontend }}
+        steps:
+            # For pull requests it's not necessary to check out the code, but we
+            # also want this to run on master, so we need to check out
+            - uses: actions/checkout@v3
+
+            - uses: dorny/paths-filter@v2
+              id: filter
+              with:
+                  filters: |
+                      frontend:
+                        # Avoid running frontend tests for irrelevant changes
+                        # NOTE: we are at risk of missing a dependency here. 
+                        - 'bin/**'
+                        - 'frontend/**'
+                        # Make sure we run if someone is explicitly change the workflow
+                        - .github/workflows/ci-frontend.yml
+                        # various JS config files
+                        - .eslintrc.js
+                        - .prettier*
+                        - babel.config.js
+                        - jest.*.ts
+                        - tsconfig.json
+                        - tsconfig.*.json
+                        - webpack.config.js
+                        - postcss.config.js
+                        - stylelint*
+
     frontend-code-quality:
         name: Code quality checks
+        needs: changes
         # kea typegen and typescript:check need some more oomph
         runs-on: ubuntu-latest
         steps:
+            # we need at least one thing to run to make sure we include everything for required jobs
             - uses: actions/checkout@v3
 
             - name: Install pnpm
+              if: needs.changes.outputs.frontend == 'true'
               uses: pnpm/action-setup@v2
               with:
                   version: 8.x.x
 
             - name: Set up Node.js
-              uses: buildjet/setup-node@v3
+              if: needs.changes.outputs.frontend == 'true'
+              uses: actions/setup-node@v3
               with:
                   node-version: 18
 
             - name: Get pnpm cache directory path
+              if: needs.changes.outputs.frontend == 'true'
               id: pnpm-cache-dir
               run: echo "PNPM_STORE_PATH=$(pnpm store path)" >> $GITHUB_OUTPUT
 
             - uses: actions/cache@v3
+              if: needs.changes.outputs.frontend == 'true'
               id: pnpm-cache
               with:
                   path: ${{ steps.pnpm-cache-dir.outputs.PNPM_STORE_PATH }}
                   key: ${{ runner.os }}-pnpm-cypress-${{ hashFiles('pnpm-lock.yaml') }}
                   restore-keys: ${{ runner.os }}-pnpm-cypress-
 
             - name: Install package.json dependencies with pnpm
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm install --frozen-lockfile
 
             - name: Check formatting with prettier
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm prettier:check
 
             - name: Lint with Stylelint
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm lint:css
 
             - name: Generate logic types and run typescript with strict
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm typegen:write && pnpm typescript:check
 
             - name: Lint with ESLint
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm lint:js
 
             - name: Check if "schema.json" is up to date
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm schema:build:json && git diff --exit-code
 
             - name: Check toolbar bundle size
+              if: needs.changes.outputs.frontend == 'true'
               uses: preactjs/compressed-size-action@v2
               with:
                   build-script: 'build'
@@ -70,6 +116,7 @@ jobs:
 
     jest:
         runs-on: ubuntu-latest
+        needs: changes
         name: Jest test (${{ matrix.chunk }})
 
         strategy:
@@ -79,24 +126,29 @@ jobs:
                 chunk: [1, 2, 3]
 
         steps:
+            # we need at least one thing to run to make sure we include everything for required jobs
             - uses: actions/checkout@v3
 
             - name: Install pnpm
+              if: needs.changes.outputs.frontend == 'true'
               uses: pnpm/action-setup@v2
               with:
                   version: 8.x.x
 
             - name: Set up Node.js
-              uses: buildjet/setup-node@v3
+              if: needs.changes.outputs.frontend == 'true'
+              uses: actions/setup-node@v3
               with:
                   node-version: 18
                   cache: pnpm
 
             - name: Install package.json dependencies with pnpm
+              if: needs.changes.outputs.frontend == 'true'
               run: pnpm install --frozen-lockfile
 
             - name: Test with Jest
               # set maxWorkers or Jest only uses 1 CPU in GitHub Actions
               run: pnpm test:unit --maxWorkers=2 --shard=${{ matrix.chunk }}/3
+              if: needs.changes.outputs.frontend == 'true'
               env:
                   NODE_OPTIONS: --max-old-space-size=6144

.github/workflows/storybook-chromatic.yml

@@ -144,6 +144,7 @@ jobs:
                   HOME: /root
                   # Update snapshots for PRs on the main repo, verify on forks, which don't have access to PostHog Bot
                   VARIANT: ${{ github.event.pull_request.head.repo.full_name == github.repository && 'update' || 'verify' }}
+                  STORYBOOK_SKIP_TAGS: 'test-skip,test-skip-${{ matrix.browser }}'
               run: |
                   pnpm test:visual-regression:stories:ci:$VARIANT --browsers ${{ matrix.browser }} --shard ${{ matrix.shard }}/$SHARD_COUNT
 

.storybook/preview.tsx

@@ -8,7 +8,6 @@ import { getStorybookAppContext } from './app-context'
 import { withKea } from './decorators/withKea'
 import { withMockDate } from './decorators/withMockDate'
 import { defaultMocks } from '~/mocks/handlers'
-import { withSnapshotsDisabled } from './decorators/withSnapshotsDisabled'
 import { withFeatureFlags } from './decorators/withFeatureFlags'
 import { withTheme } from './decorators/withTheme'
 
@@ -79,7 +78,6 @@ export const parameters: Parameters = {
 
 // Setup storybook global decorators. See https://storybook.js.org/docs/react/writing-stories/decorators#global-decorators
 export const decorators: Meta['decorators'] = [
-    withSnapshotsDisabled,
     // Make sure the msw service worker is started, and reset the handlers to defaults.
     withKea,
     // Allow us to time travel to ensure our stories don't change over time.

.storybook/test-runner.ts

@@ -13,11 +13,6 @@ declare module '@storybook/types' {
         options?: any
         layout?: 'padded' | 'fullscreen' | 'centered'
         testOptions?: {
-            /**
-             * Whether the test should be a no-op (doesn't jest.skip as @storybook/test-runner doesn't allow that).
-             * @default false
-             */
-            skip?: boolean
             /**
              * Whether we should wait for all loading indicators to disappear before taking a snapshot.
              * @default true
@@ -71,19 +66,20 @@ module.exports = {
         jest.retryTimes(RETRY_TIMES, { logErrorsBeforeRetry: true })
         jest.setTimeout(JEST_TIMEOUT_MS)
     },
-    async postRender(page, context) {
+    async postVisit(page, context) {
         const browserContext = page.context()
         const storyContext = (await getStoryContext(page, context)) as StoryContext
-        const { skip = false, snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}
+        const { snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}
 
         browserContext.setDefaultTimeout(PLAYWRIGHT_TIMEOUT_MS)
-        if (!skip) {
-            const currentBrowser = browserContext.browser()!.browserType().name() as SupportedBrowserName
-            if (snapshotBrowsers.includes(currentBrowser)) {
-                await expectStoryToMatchSnapshot(page, context, storyContext, currentBrowser)
-            }
+        const currentBrowser = browserContext.browser()!.browserType().name() as SupportedBrowserName
+        if (snapshotBrowsers.includes(currentBrowser)) {
+            await expectStoryToMatchSnapshot(page, context, storyContext, currentBrowser)
         }
     },
+    tags: {
+        skip: ['test-skip'], // NOTE: This is overridden by the CI action storybook-chromatic.yml to include browser specific skipping
+    },
 } as TestRunnerConfig
 
 async function expectStoryToMatchSnapshot(

.stylelintrc.js

@@ -10,6 +10,7 @@ module.exports = {
                 // CSS Color Module Level 3 says currentColor, Level 4 candidate says currentcolor
                 // Sticking to Level 3 for now
                 camelCaseSvgKeywords: true,
+                ignoreKeywords: ['BlinkMacSystemFont'], // BlinkMacSystemFont MUST have this particular casing
             },
         ],
         // Sadly Safari only started supporting the range syntax of media queries in 2023, so let's switch to that

cypress/e2e/featureFlags.cy.ts

@@ -48,6 +48,7 @@ describe('Feature Flags', () => {
         cy.get('[data-attr=save-feature-flag]').first().click()
 
         // after save there should be a delete button
+        cy.get('[data-attr="more-button"]').click()
         cy.get('button[data-attr="delete-feature-flag"]').should('have.text', 'Delete feature flag')
 
         // make sure the data is there as expected after a page reload!
@@ -83,11 +84,13 @@ describe('Feature Flags', () => {
         cy.get('[data-attr=save-feature-flag]').first().click()
 
         // after save there should be a delete button
+        cy.get('[data-attr="more-button"]').click()
         cy.get('button[data-attr="delete-feature-flag"]').should('have.text', 'Delete feature flag')
 
         cy.clickNavMenu('featureflags')
         cy.get('[data-attr=feature-flag-table]').should('contain', name)
         cy.get(`[data-row-key=${name}]`).contains(name).click()
+        cy.get('[data-attr="more-button"]').click()
         cy.get('[data-attr=delete-feature-flag]').click()
         cy.get('.Toastify').contains('Undo').should('be.visible')
     })

cypress/e2e/insights.cy.ts

@@ -1,7 +1,8 @@
 import { urls } from 'scenes/urls'
-import { randomString } from '../support/random'
+
 import { decideResponse } from '../fixtures/api/decide'
-import { savedInsights, createInsight, insight } from '../productAnalytics'
+import { createInsight, insight, savedInsights } from '../productAnalytics'
+import { randomString } from '../support/random'
 
 // For tests related to trends please check trendsElements.js
 // insight tests were split up because Cypress was struggling with this many tests in one file🙈

ee/api/feature_flag_role_access.py

@@ -35,7 +35,11 @@ def has_permission(self, request, view):
             return True
         try:
             feature_flag: FeatureFlag = FeatureFlag.objects.get(id=view.parents_query_dict["feature_flag_id"])
-            if feature_flag.created_by.uuid == request.user.uuid:
+            if (
+                hasattr(feature_flag, "created_by")
+                and feature_flag.created_by
+                and feature_flag.created_by.uuid == request.user.uuid
+            ):
                 return True
         except FeatureFlag.DoesNotExist:
             raise exceptions.NotFound("Feature flag not found.")

ee/api/test/test_billing.py

@@ -2,9 +2,9 @@
 from typing import Any, Dict, List
 from unittest.mock import MagicMock, patch
 from uuid import uuid4
+from zoneinfo import ZoneInfo
 
 import jwt
-from zoneinfo import ZoneInfo
 from dateutil.relativedelta import relativedelta
 from django.utils.timezone import now
 from freezegun import freeze_time