Skip to content

Commit

Permalink
feat: remove most frontend references to available_features (#22337)
Browse files Browse the repository at this point in the history
* remove most frontend references to available_features

* some more

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* adding back some items, fix tests

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* fix test

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
xrdt and github-actions[bot] authored May 20, 2024
1 parent 2a0aa98 commit 008698a
Show file tree
Hide file tree
Showing 27 changed files with 96 additions and 74 deletions.
21 changes: 16 additions & 5 deletions ee/api/test/test_billing.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ def test_license_is_updated_on_billing_load(self, mock_request):
assert license.valid_until == datetime(2022, 1, 31, 12, 0, 0, tzinfo=ZoneInfo("UTC"))

@patch("ee.api.billing.requests.get")
def test_organization_available_features_updated_if_different(self, mock_request):
def test_organization_available_product_features_updated_if_different(self, mock_request):
def mock_implementation(url: str, headers: Any = None, params: Any = None) -> MagicMock:
mock = MagicMock()
mock.status_code = 404
Expand All @@ -659,20 +659,31 @@ def mock_implementation(url: str, headers: Any = None, params: Any = None) -> Ma
elif "api/billing" in url:
mock.status_code = 200
mock.json.return_value = create_billing_response(
customer=create_billing_customer(available_features=["feature1", "feature2"])
customer=create_billing_customer(
available_product_features=[
{"key": "feature1", "name": "feature1"},
{"key": "feature2", "name": "feature2"},
]
)
)

return mock

mock_request.side_effect = mock_implementation

self.organization.available_features = []
self.organization.available_product_features = []
self.organization.save()

assert self.organization.available_features == []
assert self.organization.available_product_features == []
self.client.get("/api/billing-v2")
self.organization.refresh_from_db()
assert self.organization.available_features == ["feature1", "feature2"]
assert self.organization.available_product_features == [
{
"key": "feature1",
"name": "feature1",
},
{"key": "feature2", "name": "feature2"},
]

@patch("ee.api.billing.requests.get")
def test_organization_usage_update(self, mock_request):
Expand Down
5 changes: 0 additions & 5 deletions ee/billing/billing_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,6 @@ def update_org_details(self, organization: Organization, billing_status: Billing
org_modified = True
sync_org_quota_limits(organization)

available_features = data.get("available_features", None)
if available_features and available_features != organization.available_features:
organization.available_features = data["available_features"]
org_modified = True

available_product_features = data.get("available_product_features", None)
if available_product_features and available_product_features != organization.available_product_features:
organization.available_product_features = data["available_product_features"]
Expand Down
21 changes: 11 additions & 10 deletions ee/billing/billing_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,23 @@ class UsageSummary(TypedDict):
usage: Optional[int]


class ProductFeature(TypedDict):
key: str
name: str
description: str
unit: Optional[str]
limit: Optional[int]
note: Optional[str]
is_plan_default: bool


class CustomerInfo(TypedDict):
customer_id: Optional[str]
deactivated: bool
has_active_subscription: bool
billing_period: BillingPeriod
available_features: list[AvailableFeature]
available_product_features: list[ProductFeature]
current_total_amount_usd: Optional[str]
current_total_amount_usd_after_discount: Optional[str]
products: Optional[list[CustomerProduct]]
Expand All @@ -103,16 +114,6 @@ class BillingStatus(TypedDict):
customer: CustomerInfo


class ProductFeature(TypedDict):
key: str
name: str
description: str
unit: Optional[str]
limit: Optional[int]
note: Optional[str]
is_plan_default: bool


class ProductPlan(TypedDict):
"""
A plan for a product that a customer can upgrade/downgrade to.
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion frontend/src/lib/api.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ export const MOCK_DEFAULT_ORGANIZATION: OrganizationType = {
plugins_access_level: PluginsAccessLevel.Root,
enforce_2fa: false,
teams: [MOCK_DEFAULT_TEAM],
available_features: [],
is_member_join_email_enabled: true,
metadata: {},
available_product_features: [],
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/components/Sharing/sharingLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ export const sharingLogic = kea<sharingLogicType>([
selectors({
siteUrl: [() => [preflightLogic.selectors.preflight], (preflight) => preflight?.site_url],
whitelabelAvailable: [
() => [userLogic.selectors.user],
(user) => (user?.organization?.available_features || []).includes(AvailableFeature.WHITE_LABELLING),
() => [userLogic.selectors.hasAvailableFeature],
(hasAvailableFeature) => hasAvailableFeature(AvailableFeature.WHITE_LABELLING),
],

params: [
Expand Down
11 changes: 8 additions & 3 deletions frontend/src/mocks/features.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { AvailableFeature } from '~/types'
import { AvailableFeature, BillingV2FeatureType } from '~/types'

let features: AvailableFeature[] = []
export const useAvailableFeatures = (f: AvailableFeature[]): void => {
features = f
}
export const getAvailableFeatures = (): AvailableFeature[] => {
return features
export const getAvailableProductFeatures = (): BillingV2FeatureType[] => {
return features.map((feature) => {
return {
key: feature,
name: feature,
}
})
}
9 changes: 6 additions & 3 deletions frontend/src/mocks/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import {
} from 'lib/api.mock'
import { ResponseComposition, RestContext, RestRequest } from 'msw'

import { getAvailableFeatures } from '~/mocks/features'
import { SharingConfigurationType } from '~/types'

import { getAvailableProductFeatures } from './features'
import { billingJson } from './fixtures/_billing_v2'
import { Mocks, MockSignature, mocksToHandlers } from './utils'

Expand Down Expand Up @@ -77,7 +77,7 @@ export const defaultMocks: Mocks = {
'/api/projects/:team_id/warehouse_tables/': EMPTY_PAGINATED_RESPONSE,
'/api/organizations/@current/': (): MockSignature => [
200,
{ ...MOCK_DEFAULT_ORGANIZATION, available_features: getAvailableFeatures() },
{ ...MOCK_DEFAULT_ORGANIZATION, available_product_features: getAvailableProductFeatures() },
],
'/api/organizations/@current/roles/': EMPTY_PAGINATED_RESPONSE,
'/api/organizations/@current/members/': toPaginatedResponse([
Expand All @@ -97,7 +97,10 @@ export const defaultMocks: Mocks = {
200,
{
...MOCK_DEFAULT_USER,
organization: { ...MOCK_DEFAULT_ORGANIZATION, available_features: getAvailableFeatures() },
organization: {
...MOCK_DEFAULT_ORGANIZATION,
available_product_features: getAvailableProductFeatures(),
},
},
],
'/api/projects/@current/': MOCK_DEFAULT_TEAM,
Expand Down
5 changes: 2 additions & 3 deletions frontend/src/queries/nodes/InsightViz/EditorFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ export interface EditorFiltersProps {
}

export function EditorFilters({ query, showing, embedded }: EditorFiltersProps): JSX.Element | null {
const { user } = useValues(userLogic)
const availableFeatures = user?.organization?.available_features || []
const { hasAvailableFeature } = useValues(userLogic)

const { insight, insightProps } = useValues(insightLogic)
const {
Expand All @@ -74,7 +73,7 @@ export function EditorFilters({ query, showing, embedded }: EditorFiltersProps):
(isTrends && !NON_BREAKDOWN_DISPLAY_TYPES.includes(display || ChartDisplayType.ActionsLineGraph)) ||
isStepsFunnel ||
isTrendsFunnel
const hasPathsAdvanced = availableFeatures.includes(AvailableFeature.PATHS_ADVANCED)
const hasPathsAdvanced = hasAvailableFeature(AvailableFeature.PATHS_ADVANCED)
const hasAttribution = isStepsFunnel
const hasPathsHogQL = isPaths && pathsFilter?.includeEventTypes?.includes(PathType.HogQL)

Expand Down
5 changes: 2 additions & 3 deletions frontend/src/scenes/insights/views/Paths/PathStepPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@ export function PathStepPicker(): JSX.Element {
const { insightProps } = useValues(insightLogic)
const { pathsFilter } = useValues(pathsDataLogic(insightProps))
const { updateInsightFilter } = useActions(pathsDataLogic(insightProps))
const { hasAvailableFeature } = useValues(userLogic)

const { stepLimit } = pathsFilter || {}

const { user } = useValues(userLogic)

const MIN = 2,
MAX = user?.organization?.available_features.includes(AvailableFeature.PATHS_ADVANCED) ? 20 : 5
MAX = hasAvailableFeature(AvailableFeature.PATHS_ADVANCED) ? 20 : 5

const options: StepOption[] = Array.from(Array.from(Array.from(Array(MAX + 1).keys()).slice(MIN)), (v) => ({
label: `${v} Steps`,
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/organizationLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('organizationLogic', () => {
it('loads organization from API', async () => {
await expectLogic(logic).toDispatchActions(['loadCurrentOrganization', 'loadCurrentOrganizationSuccess'])
await expectLogic(logic).toMatchValues({
currentOrganization: { ...MOCK_DEFAULT_ORGANIZATION, available_features: [] },
currentOrganization: { ...MOCK_DEFAULT_ORGANIZATION },
})
})
})
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/scenes/organizationLogic.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { actions, afterMount, kea, listeners, path, reducers, selectors } from 'kea'
import { actions, afterMount, connect, kea, listeners, path, reducers, selectors } from 'kea'
import { loaders } from 'kea-loaders'
import api, { ApiConfig } from 'lib/api'
import { OrganizationMembershipLevel } from 'lib/constants'
Expand All @@ -22,6 +22,7 @@ export const organizationLogic = kea<organizationLogicType>([
deleteOrganizationSuccess: true,
deleteOrganizationFailure: true,
}),
connect([userLogic]),
reducers({
organizationBeingDeleted: [
null as OrganizationType | null,
Expand Down Expand Up @@ -65,8 +66,8 @@ export const organizationLogic = kea<organizationLogicType>([
})),
selectors({
hasTagging: [
(s) => [s.currentOrganization],
(currentOrganization) => currentOrganization?.available_features?.includes(AvailableFeature.TAGGING),
() => [userLogic.selectors.hasAvailableFeature],
(hasAvailableFeature) => hasAvailableFeature(AvailableFeature.TAGGING),
],
isCurrentOrganizationUnavailable: [
(s) => [s.currentOrganization, s.currentOrganizationLoading],
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/paths/PathNodeCardButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export function PathNodeCardButton({
filter,
setFilter,
}: PathNodeCardButton): JSX.Element {
const { user } = useValues(userLogic)
const hasAdvancedPaths = user?.organization?.available_features?.includes(AvailableFeature.PATHS_ADVANCED)
const { hasAvailableFeature } = useValues(userLogic)
const hasAdvancedPaths = hasAvailableFeature(AvailableFeature.PATHS_ADVANCED)

const nodeName = pageUrl(node)
const isPath = nodeName.includes('/')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ exports[`verifiedDomainsLogic values has proper defaults 1`] = `
"addModalShown": false,
"configureSAMLModalId": null,
"currentOrganization": {
"available_features": [
"sso_enforcement",
"saml",
"available_product_features": [
{
"key": "sso_enforcement",
"name": "sso_enforcement",
},
{
"key": "saml",
"name": "saml",
},
],
"available_product_features": [],
"created_at": "2020-09-24T15:05:01.254111Z",
"customer_id": null,
"enforce_2fa": false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expectLogic } from 'kea-test-utils'
import { userLogic } from 'scenes/userLogic'

import { useAvailableFeatures } from '~/mocks/features'
import { useMocks } from '~/mocks/jest'
Expand All @@ -9,6 +10,7 @@ import { isSecureURL, verifiedDomainsLogic } from './verifiedDomainsLogic'

describe('verifiedDomainsLogic', () => {
let logic: ReturnType<typeof verifiedDomainsLogic.build>
let userlogic: ReturnType<typeof userLogic.build>

beforeEach(() => {
useAvailableFeatures([AvailableFeature.SSO_ENFORCEMENT, AvailableFeature.SAML])
Expand Down Expand Up @@ -54,6 +56,8 @@ describe('verifiedDomainsLogic', () => {
})
initKeaTests()
logic = verifiedDomainsLogic()
userlogic = userLogic()
userlogic.mount()
logic.mount()
})

Expand Down Expand Up @@ -81,6 +85,7 @@ describe('verifiedDomainsLogic', () => {

describe('values', () => {
it('has proper defaults', async () => {
await expectLogic(userlogic).toFinishAllListeners()
await expectLogic(logic).toFinishAllListeners()
expect(logic.values).toMatchSnapshot()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import api from 'lib/api'
import { SECURE_URL_REGEX } from 'lib/constants'
import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
import { organizationLogic } from 'scenes/organizationLogic'
import { userLogic } from 'scenes/userLogic'

import { AvailableFeature, OrganizationDomainType } from '~/types'

Expand All @@ -31,7 +32,7 @@ export const isSecureURL = (url: string): boolean => {

export const verifiedDomainsLogic = kea<verifiedDomainsLogicType>([
path(['scenes', 'organization', 'verifiedDomainsLogic']),
connect({ values: [organizationLogic, ['currentOrganization']] }),
connect({ values: [organizationLogic, ['currentOrganization']], logic: [userLogic] }),
actions({
replaceDomain: (domain: OrganizationDomainType) => ({ domain }),
setAddModalShown: (shown: boolean) => ({ shown }),
Expand Down Expand Up @@ -134,14 +135,12 @@ export const verifiedDomainsLogic = kea<verifiedDomainsLogicType>([
(verifyingId && verifiedDomains.find(({ id }) => id === verifyingId)) || null,
],
isSSOEnforcementAvailable: [
(s) => [s.currentOrganization],
(currentOrganization): boolean =>
currentOrganization?.available_features.includes(AvailableFeature.SSO_ENFORCEMENT) ?? false,
() => [userLogic.selectors.hasAvailableFeature],
(hasAvailableFeature): boolean => hasAvailableFeature(AvailableFeature.SSO_ENFORCEMENT),
],
isSAMLAvailable: [
(s) => [s.currentOrganization],
(currentOrganization): boolean =>
currentOrganization?.available_features.includes(AvailableFeature.SAML) ?? false,
() => [userLogic.selectors.hasAvailableFeature],
(hasAvailableFeature): boolean => hasAvailableFeature(AvailableFeature.SAML),
],
}),
afterMount(({ actions }) => actions.loadVerifiedDomains()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { AvailableFeature, InsightType } from '~/types'
export function PathCleaningFiltersConfig(): JSX.Element | null {
const { updateCurrentTeam } = useActions(teamLogic)
const { currentTeam } = useValues(teamLogic)
const { user } = useValues(userLogic)
const hasAdvancedPaths = user?.organization?.available_features?.includes(AvailableFeature.PATHS_ADVANCED)
const { hasAvailableFeature } = useValues(userLogic)
const hasAdvancedPaths = hasAvailableFeature(AvailableFeature.PATHS_ADVANCED)

if (!currentTeam) {
return null
Expand Down
5 changes: 2 additions & 3 deletions frontend/src/scenes/userLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export const userLogic = kea<userLogicType>([
name: user.organization.name,
slug: user.organization.slug,
created_at: user.organization.created_at,
available_features: user.organization.available_features,
available_product_features: user.organization.available_product_features,
...user.organization.metadata,
})

Expand Down Expand Up @@ -214,8 +214,7 @@ export const userLogic = kea<userLogicType>([
: true
: false
}
// if we don't have the new available_product_features obj, fallback to old available_features
return !!user?.organization?.available_features.includes(feature)
return false
}
},
],
Expand Down
1 change: 0 additions & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ export interface OrganizationType extends OrganizationBasicType {
updated_at: string
plugins_access_level: PluginsAccessLevel
teams: TeamBasicType[]
available_features: AvailableFeatureUnion[]
available_product_features: BillingV2FeatureType[]
is_member_join_email_enabled: boolean
customer_id: string | null
Expand Down
2 changes: 0 additions & 2 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,6 @@ posthog/hogql_queries/insights/trends/aggregation_operations.py:0: error: Item "
posthog/hogql_queries/insights/trends/aggregation_operations.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "select" [union-attr]
posthog/hogql_queries/insights/trends/aggregation_operations.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "group_by" [union-attr]
posthog/hogql_queries/insights/trends/aggregation_operations.py:0: error: Item "None" of "list[Expr] | Any | None" has no attribute "append" [union-attr]
ee/billing/billing_manager.py:0: error: TypedDict "CustomerInfo" has no key "available_product_features" [typeddict-item]
ee/billing/billing_manager.py:0: note: Did you mean "available_features"?
posthog/hogql/resolver.py:0: error: Argument 1 of "visit" is incompatible with supertype "Visitor"; supertype defines the argument type as "AST" [override]
posthog/hogql/resolver.py:0: note: This violates the Liskov substitution principle
posthog/hogql/resolver.py:0: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Expand Down
3 changes: 2 additions & 1 deletion plugin-server/functional_tests/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,9 @@ export const createOrganization = async (organizationProperties = {}) => {
personalization: '{}', // DEPRECATED
setup_section_2_completed: true, // DEPRECATED
for_internal_metrics: false,
available_features: [],
domain_whitelist: [],
available_features: [],
available_product_features: [],
is_member_join_email_enabled: false,
slug: Math.round(Math.random() * 20000),
...organizationProperties,
Expand Down
Loading

0 comments on commit 008698a

Please sign in to comment.