Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: exp for nontechnical users in prod. analytics #26993

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions frontend/src/lib/constants.tsx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting on this file so we can thread the discussion...

I'm generally hesitant about this change because it effectively hides the rest of the SDKs we offer from the page.

The null hypothesis is that people won't notice the "recommended" filter, won't click the "X" to clear it, and won't see all the other SDKs we offer.

Why don't we just put the recommended ones at the top in a separate section, with some way to show that they are recommended?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing - I think with this we are maybe prioritizing the wrong set of users?

High ICP users - engineers - will want to see all the SDK options we have so they can find the one they need. Maybe the experience there needs to change for those users if their conversion through this step isn't great, but I don't see why we'd change the onboarding for our most popular product to cater to low ICP users.

If anything, we should only apply this change to people who we know are low-ICP (using role data is easy easy easy and required on signup for this exact reason). We can even skip this SDKs screen entirely for these people and do a custom invite thing where it says "you're all set up, now invite your technical team to implement" and in the email we send them we give them context about what needs to happen.

Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export const FEATURE_FLAGS = {
FF_DASHBOARD_TEMPLATES: 'ff-dashboard-templates', // owner: @EDsCODE
ARTIFICIAL_HOG: 'artificial-hog', // owner: @Twixes
CS_DASHBOARDS: 'cs-dashboards', // owner: @pauldambra
PRODUCT_ANALYTICS_MODIFIED_SDK_LIST: 'product-analytics-modified-sdk-list', // owner: @surbhi
PRODUCT_SPECIFIC_ONBOARDING: 'product-specific-onboarding', // owner: @raquelmsmith
REDIRECT_SIGNUPS_TO_INSTANCE: 'redirect-signups-to-instance', // owner: @raquelmsmith
APPS_AND_EXPORTS_UI: 'apps-and-exports-ui', // owner: @benjackwhite
Expand Down
16 changes: 11 additions & 5 deletions frontend/src/scenes/onboarding/sdks/SDKs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export function SDKs({
const { goToNextStep, completeOnboarding } = useActions(onboardingLogic)
const [showListeningFor, setShowListeningFor] = React.useState(false)
const [hasCheckedInstallation, setHasCheckedInstallation] = React.useState(false)
const { isUserInNonTechnicalTest } = useValues(sdksLogic)

const { width } = useWindowSize()

Expand Down Expand Up @@ -66,8 +67,17 @@ export function SDKs({
}
}, 2000)

const HelpCard = (): JSX.Element => (
<LemonCard className="mt-6" hoverEffect={false}>
<h3 className="font-bold">Need help with this step?</h3>
<p>Invite a team member to help you get set up.</p>
<InviteMembersButton type="secondary" />
</LemonCard>
)

return (
<OnboardingStep title="Install" stepKey={stepKey} continueOverride={<></>}>
{isUserInNonTechnicalTest && <HelpCard />}
<div className="flex gap-x-8 mt-6">
<div
className={`flex-col gap-y-2 flex-wrap gap-x-4 ${showSideBySide && 'min-w-[12.5rem] w-50'} ${
Expand Down Expand Up @@ -106,11 +116,7 @@ export function SDKs({
</LemonButton>
</React.Fragment>
))}
<LemonCard className="mt-6" hoverEffect={false}>
<h3 className="font-bold">Need help with this step?</h3>
<p>Invite a team member to help you get set up.</p>
<InviteMembersButton type="secondary" />
</LemonCard>
{!isUserInNonTechnicalTest && <HelpCard />}
</div>
{selectedSDK && productKey && !!sdkInstructionMap[selectedSDK.key] && (
<div
Expand Down
47 changes: 37 additions & 10 deletions frontend/src/scenes/onboarding/sdks/sdksLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ import { actions, afterMount, connect, events, kea, listeners, path, reducers, s
import { loaders } from 'kea-loaders'
import { urlToAction } from 'kea-router'
import api from 'lib/api'
import { FEATURE_FLAGS } from 'lib/constants'
import { LemonSelectOptions } from 'lib/lemon-ui/LemonSelect/LemonSelect'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { liveEventsTableLogic } from 'scenes/activity/live/liveEventsTableLogic'
import { userLogic } from 'scenes/userLogic'

import { HogQLQuery, NodeKind } from '~/queries/schema'
import { hogql } from '~/queries/utils'
Expand All @@ -12,7 +15,6 @@ import { ProductKey, SDK, SDKInstructionsMap } from '~/types'
import { onboardingLogic } from '../onboardingLogic'
import { allSDKs } from './allSDKs'
import type { sdksLogicType } from './sdksLogicType'

/*
To add SDK instructions for your product:
1. If needed, add a new ProductKey enum value in ~/types.ts
Expand All @@ -39,10 +41,27 @@ Products that will often be installed in multiple places, eg. web and mobile
*/
export const multiInstallProducts = [ProductKey.PRODUCT_ANALYTICS, ProductKey.FEATURE_FLAGS]

const getProductAnalyticsOrderedSDKs = (sdks: SDK[]): SDK[] => {
return [
...sdks.filter((sdk) => sdk.key === 'html'),
...sdks.filter((sdk) => sdk.key === 'javascript-web'),
surbhi-posthog marked this conversation as resolved.
Show resolved Hide resolved
...sdks.filter((sdk) => !['html', 'javascript-web'].includes(sdk.key)),
]
}

export const sdksLogic = kea<sdksLogicType>([
path(['scenes', 'onboarding', 'sdks', 'sdksLogic']),
connect({
values: [onboardingLogic, ['productKey'], liveEventsTableLogic, ['eventHosts']],
values: [
onboardingLogic,
['productKey'],
liveEventsTableLogic,
['eventHosts'],
featureFlagLogic,
['featureFlags'],
userLogic,
['user', 'isUserTechnical'],
],
}),
actions({
setSourceFilter: (sourceFilter: string | null) => ({ sourceFilter }),
Expand Down Expand Up @@ -133,6 +152,16 @@ export const sdksLogic = kea<sdksLogicType>([
return combinedSnippetAndLiveEventsHosts
},
],
isUserInNonTechnicalTest: [
(s) => [s.productKey, s.featureFlags, s.isUserTechnical],
(productKey, featureFlags, isUserTechnical): boolean => {
return (
productKey === ProductKey.PRODUCT_ANALYTICS &&
featureFlags[FEATURE_FLAGS.PRODUCT_ANALYTICS_MODIFIED_SDK_LIST] === 'test' &&
isUserTechnical === false
)
},
],
}),
loaders(({ actions }) => ({
hasSnippetEvents: [
Expand Down Expand Up @@ -173,14 +202,17 @@ export const sdksLogic = kea<sdksLogicType>([
})),
listeners(({ actions, values }) => ({
filterSDKs: () => {
const filteredSDks: SDK[] = allSDKs
let filteredSDks: SDK[] = allSDKs
.filter((sdk) => {
if (!values.sourceFilter || !sdk) {
return true
}
return sdk.tags.includes(values.sourceFilter)
})
.filter((sdk) => Object.keys(values.availableSDKInstructionsMap).includes(sdk.key))
if (values.isUserInNonTechnicalTest) {
filteredSDks = getProductAnalyticsOrderedSDKs(filteredSDks)
}
actions.setSDKs(filteredSDks)
actions.setSourceOptions(getSourceOptions(values.availableSDKInstructionsMap))
},
Expand Down Expand Up @@ -225,12 +257,7 @@ export const sdksLogic = kea<sdksLogicType>([
afterMount(({ actions }) => {
actions.loadSnippetEvents()
}),
urlToAction(({ actions }) => ({
'/onboarding/:productKey': (_productKey, { sdk }) => {
const matchedSDK = allSDKs.find((s) => s.key === sdk)
if (matchedSDK) {
actions.setSelectedSDK(matchedSDK)
}
},
urlToAction(() => ({
'/onboarding/:productKey': () => {},
})),
])
8 changes: 8 additions & 0 deletions frontend/src/scenes/userLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,14 @@ export const userLogic = kea<userLogicType>([
return user?.theme_mode || 'light'
},
],

isUserTechnical: [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should flip this so it defaults to technical for all users.

It's important to note we won't always have this data because it's only via email/password not social SSO (Google, Github, etc.)

(s) => [s.user],
(user) => {
const technicalRoles = ['engineering', 'founder']
return user?.role_at_organization && technicalRoles.includes(user.role_at_organization)
},
],
}),
afterMount(({ actions }) => {
const preloadedUser = getAppContext()?.current_user
Expand Down
1 change: 1 addition & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ export interface UserType extends UserBaseType {
scene_personalisation?: SceneDashboardChoice[]
theme_mode?: UserTheme | null
hedgehog_config?: Partial<HedgehogConfig>
role_at_organization?: string
}

export type HedgehogColorOptions =
Expand Down
12 changes: 8 additions & 4 deletions posthog/api/signup.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def create(self, validated_data, **kwargs):
create_team=self.create_team,
is_staff=is_instance_first_user,
is_email_verified=self.is_email_auto_verified(),
role_at_organization=role_at_organization,
**validated_data,
)
except IntegrityError:
Expand Down Expand Up @@ -142,6 +143,7 @@ def enter_demo(self, validated_data) -> User:
email = validated_data["email"]
first_name = validated_data["first_name"]
organization_name = validated_data["organization_name"]
role_at_organization = validated_data["role_at_organization"]
# In the demo env, social signups gets staff privileges
# - grep SOCIAL_AUTH_GOOGLE_OAUTH2_WHITELISTED_DOMAINS for more info
is_staff = self.is_social_signup
Expand All @@ -152,7 +154,9 @@ def enter_demo(self, validated_data) -> User:
self._organization,
self._team,
self._user,
) = manager.ensure_account_and_save(email, first_name, organization_name, is_staff=is_staff)
) = manager.ensure_account_and_save(
email, first_name, organization_name, role_at_organization, is_staff=is_staff
)

login(
self.context["request"],
Expand Down Expand Up @@ -209,8 +213,6 @@ def create(self, validated_data, **kwargs):
user: Optional[User] = None
is_new_user: bool = False

role_at_organization = validated_data.pop("role_at_organization", "")

if self.context["request"].user.is_authenticated:
user = cast(User, self.context["request"].user)

Expand All @@ -236,11 +238,13 @@ def create(self, validated_data, **kwargs):
if not user:
is_new_user = True
try:
role = validated_data.pop("role_at_organization", "")
user = User.objects.create_user(
invite.target_email,
validated_data.pop("password"),
validated_data.pop("first_name"),
is_email_verified=False,
role_at_organization=role,
**validated_data,
)
except IntegrityError:
Expand All @@ -264,7 +268,7 @@ def create(self, validated_data, **kwargs):
backend_processor="OrganizationInviteSignupSerializer",
user_analytics_metadata=user.get_analytics_metadata(),
org_analytics_metadata=user.organization.get_analytics_metadata() if user.organization else None,
role_at_organization=role_at_organization,
role_at_organization=role,
referral_source="signed up from invite link",
)

Expand Down
7 changes: 7 additions & 0 deletions posthog/api/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class UserSerializer(serializers.ModelSerializer):
notification_settings = serializers.DictField(required=False)
scene_personalisation = ScenePersonalisationBasicSerializer(many=True, read_only=True)
anonymize_data = ClassicBehaviorBooleanFieldSerializer()
role_at_organization = serializers.ChoiceField(choices=User.ROLE_CHOICES, required=True)

class Meta:
model = User
Expand Down Expand Up @@ -128,6 +129,7 @@ class Meta:
"scene_personalisation",
"theme_mode",
"hedgehog_config",
"role_at_organization",
]

read_only_fields = [
Expand Down Expand Up @@ -250,6 +252,11 @@ def validate_is_staff(self, value: bool) -> bool:
raise exceptions.PermissionDenied("You are not a staff user, contact your instance admin.")
return value

def validate_role_at_organization(self, value):
if value and value not in dict(User.ROLE_CHOICES):
raise serializers.ValidationError("Invalid role selected")
return value

def update(self, instance: "User", validated_data: Any) -> Any:
# Update current_organization and current_team
current_organization = validated_data.pop("set_current_organization", None)
Expand Down
31 changes: 31 additions & 0 deletions posthog/migrations/0533_user_role_at_organization.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 4.2.15 on 2024-12-19 21:27

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("posthog", "0532_taxonomy_unique_on_project"),
]

operations = [
migrations.AddField(
model_name="user",
name="role_at_organization",
field=models.CharField(
blank=True,
choices=[
("engineering", "Engineering"),
("data", "Data"),
("product", "Product Management"),
("founder", "Founder"),
("leadership", "Leadership"),
("marketing", "Marketing"),
("sales", "Sales / Success"),
("other", "Other"),
],
max_length=64,
null=True,
),
),
]
2 changes: 1 addition & 1 deletion posthog/migrations/max_migration.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0532_taxonomy_unique_on_project
0533_user_role_at_organization
13 changes: 13 additions & 0 deletions posthog/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,19 @@ class User(AbstractUser, UUIDClassicModel):
# Remove unused attributes from `AbstractUser`
username = None

ROLE_CHOICES = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this up to the top with the others.

("engineering", "Engineering"),
("data", "Data"),
("product", "Product Management"),
("founder", "Founder"),
("leadership", "Leadership"),
("marketing", "Marketing"),
("sales", "Sales / Success"),
("other", "Other"),
)

role_at_organization = models.CharField(max_length=64, choices=ROLE_CHOICES, null=True, blank=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you break this PR out into multiple PRs? Database migrations are always important to be methodical about:

  1. Add new column w/ migration
  2. Add changes to signup API to support the new attribute
  3. Update the frontend to send the new attribute (and make any other changes)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we can move this row up to be with the other user details.


objects: UserManager = UserManager()

@property
Expand Down