rm getProfile from protocol, use ProfileDetails

AlexTugarev · AlexTugarev · commit 5faa6714188c · 2023-12-06T08:11:32.000Z
diff --git a/components/dashboard/src/user-settings/Account.tsx b/components/dashboard/src/user-settings/Account.tsx
@@ -17,11 +17,20 @@ import isEmail from "validator/lib/isEmail";
 import { useToast } from "../components/toasts/Toasts";
 import { InputWithCopy } from "../components/InputWithCopy";
 import { InputField } from "../components/forms/InputField";
-import { getPrimaryEmail, getProfile, isOrganizationOwned } from "@gitpod/public-api-common/lib/user-utils";
+import { getPrimaryEmail, isOrganizationOwned } from "@gitpod/public-api-common/lib/user-utils";
 import { User } from "@gitpod/public-api/lib/gitpod/v1/user_pb";
-import { User as UserProtocol } from "@gitpod/gitpod-protocol";
+import { User as UserProtocol, ProfileDetails } from "@gitpod/gitpod-protocol";
 import { useUpdateCurrentUserMutation } from "../data/current-user/update-mutation";
 
+type UserProfile = Pick<ProfileDetails, "emailAddress"> & Required<Pick<UserProtocol, "name" | "avatarUrl">>;
+function getProfile(user: User): UserProfile {
+    return {
+        name: user.name,
+        avatarUrl: user.avatarUrl,
+        emailAddress: getPrimaryEmail(user),
+    };
+}
+
 export default function Account() {
     const { user, setUser } = useContext(UserContext);
     const [modal, setModal] = useState(false);
@@ -43,17 +52,17 @@ export default function Account() {
             return;
         }
         if (canUpdateEmail) {
-            if (profileState.email.trim() === "") {
+            if (!profileState.emailAddress?.trim()) {
                 setErrorMessage("Email must not be empty.");
                 return;
             }
             // check valid email
-            if (!isEmail(profileState.email.trim())) {
+            if (!isEmail(profileState.emailAddress?.trim() || "")) {
                 setErrorMessage("Please enter a valid email.");
                 return;
             }
         } else {
-            profileState.email = getPrimaryEmail(user) || "";
+            profileState.emailAddress = getPrimaryEmail(user) || "";
         }
 
         const updatedUser = await updateUser.mutateAsync({
@@ -78,7 +87,7 @@ export default function Account() {
                 title="Delete Account"
                 areYouSureText="You are about to permanently delete your account."
                 buttonText="Delete Account"
-                buttonDisabled={typedEmail !== original.email}
+                buttonDisabled={typedEmail !== (original.emailAddress || "")}
                 visible={modal}
                 onClose={close}
                 onConfirm={deleteAccount}
@@ -133,8 +142,8 @@ export default function Account() {
 }
 
 function ProfileInformation(props: {
-    profileState: UserProtocol.Profile;
-    setProfileState: (newState: UserProtocol.Profile) => void;
+    profileState: UserProfile;
+    setProfileState: (newState: UserProfile) => void;
     errorMessage: string;
     emailIsReadonly?: boolean;
     user?: User;
@@ -161,10 +170,10 @@ function ProfileInformation(props: {
                     />
                     <TextInputField
                         label="Email"
-                        value={props.profileState.email}
+                        value={props.profileState.emailAddress || ""}
                         disabled={props.emailIsReadonly}
                         onChange={(val) => {
-                            props.setProfileState({ ...props.profileState, email: val });
+                            props.setProfileState({ ...props.profileState, emailAddress: val });
                         }}
                     />
                     {props.user && (
@@ -178,7 +187,7 @@ function ProfileInformation(props: {
                         <Subheading>Avatar</Subheading>
                         <img
                             className="rounded-full w-24 h-24"
-                            src={props.profileState.avatarURL}
+                            src={props.profileState.avatarUrl}
                             alt={props.profileState.name}
                         />
                     </div>
diff --git a/components/gitpod-protocol/src/protocol.ts b/components/gitpod-protocol/src/protocol.ts
@@ -104,39 +104,6 @@ export namespace User {
         }
         user.additionalData.ideSettings = newIDESettings;
     }
-
-    // TODO: refactor where this is referenced so it's more clearly tied to just analytics-tracking
-    // Let other places rely on the ProfileDetails type since that's what we store
-    // This is the profile data we send to our Segment analytics tracking pipeline
-    export interface Profile {
-        name: string;
-        email: string;
-        company?: string;
-        avatarURL?: string;
-        jobRole?: string;
-        jobRoleOther?: string;
-        explorationReasons?: string[];
-        signupGoals?: string[];
-        signupGoalsOther?: string;
-        onboardedTimestamp?: string;
-        companySize?: string;
-    }
-    export namespace Profile {
-        export function hasChanges(before: Profile, after: Profile) {
-            return (
-                before.name !== after.name ||
-                before.email !== after.email ||
-                before.company !== after.company ||
-                before.avatarURL !== after.avatarURL ||
-                before.jobRole !== after.jobRole ||
-                before.jobRoleOther !== after.jobRoleOther ||
-                // not checking explorationReasons or signupGoals atm as it's an array - need to check deep equality
-                before.signupGoalsOther !== after.signupGoalsOther ||
-                before.onboardedTimestamp !== after.onboardedTimestamp ||
-                before.companySize !== after.companySize
-            );
-        }
-    }
 }
 
 export interface WorkspaceTimeoutSetting {
diff --git a/components/public-api/typescript-common/src/user-utils.ts b/components/public-api/typescript-common/src/user-utils.ts
@@ -72,21 +72,3 @@ export function getName(user: User | UserProtocol): string | undefined {
 export function isOrganizationOwned(user: User | UserProtocol) {
     return !!user.organizationId;
 }
-
-// FIXME(at) get rid of this Nth indirection to read attributes of User entity
-export function getProfile(user: User | UserProtocol): UserProtocol.Profile {
-    const profile = UserProtocol.is(user) ? user.additionalData?.profile : user.profile;
-    return {
-        name: getName(user) || "",
-        email: getPrimaryEmail(user) || "",
-        company: profile?.companyName,
-        avatarURL: user?.avatarUrl,
-        jobRole: profile?.jobRole,
-        jobRoleOther: profile?.jobRoleOther,
-        explorationReasons: profile?.explorationReasons,
-        signupGoals: profile?.signupGoals,
-        signupGoalsOther: profile?.signupGoalsOther,
-        companySize: profile?.companySize,
-        onboardedTimestamp: profile?.onboardedTimestamp,
-    };
-}
diff --git a/components/server/src/user/user-service.ts b/components/server/src/user/user-service.ts
@@ -24,7 +24,7 @@ import { CreateUserParams } from "./user-authentication";
 import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
 import { TransactionalContext } from "@gitpod/gitpod-db/lib/typeorm/transactional-db-impl";
 import { RelationshipUpdater } from "../authorization/relationship-updater";
-import { getProfile } from "@gitpod/public-api-common/lib/user-utils";
+import { getName, getPrimaryEmail } from "@gitpod/public-api-common/lib/user-utils";
 
 @injectable()
 export class UserService {
@@ -106,7 +106,7 @@ export class UserService {
         await this.authorizer.checkPermissionOnUser(userId, "write_info", user.id);
 
         //hang on to user profile before it's overwritten for analytics below
-        const oldProfile = getProfile(user);
+        const oldProfile = Profile.getProfile(user);
 
         const allowedFields: (keyof User)[] = ["fullName", "additionalData"];
         for (const p of allowedFields) {
@@ -118,8 +118,8 @@ export class UserService {
         await this.userDb.updateUserPartial(user);
 
         //track event and user profile if profile of partialUser changed
-        const newProfile = getProfile(user);
-        if (this.hasChanges(oldProfile, newProfile)) {
+        const newProfile = Profile.getProfile(user);
+        if (Profile.hasChanges(oldProfile, newProfile)) {
             this.analytics.track({
                 userId: user.id,
                 event: "profile_changed",
@@ -133,21 +133,6 @@ export class UserService {
         return user;
     }
 
-    private hasChanges(before: User.Profile, after: User.Profile) {
-        return (
-            before.name !== after.name ||
-            before.email !== after.email ||
-            before.company !== after.company ||
-            before.avatarURL !== after.avatarURL ||
-            before.jobRole !== after.jobRole ||
-            before.jobRoleOther !== after.jobRoleOther ||
-            // not checking explorationReasons or signupGoals atm as it's an array - need to check deep equality
-            before.signupGoalsOther !== after.signupGoalsOther ||
-            before.onboardedTimestamp !== after.onboardedTimestamp ||
-            before.companySize !== after.companySize
-        );
-    }
-
     async updateWorkspaceTimeoutSetting(
         userId: string,
         targetUserId: string,
@@ -328,3 +313,53 @@ export class UserService {
         log.info("User verified", { userId: user.id });
     }
 }
+
+// TODO: refactor where this is referenced so it's more clearly tied to just analytics-tracking
+// Let other places rely on the ProfileDetails type since that's what we store
+// This is the profile data we send to our Segment analytics tracking pipeline
+interface Profile {
+    name: string;
+    email: string;
+    company?: string;
+    avatarURL?: string;
+    jobRole?: string;
+    jobRoleOther?: string;
+    explorationReasons?: string[];
+    signupGoals?: string[];
+    signupGoalsOther?: string;
+    onboardedTimestamp?: string;
+    companySize?: string;
+}
+namespace Profile {
+    export function hasChanges(before: Profile, after: Profile) {
+        return (
+            before.name !== after.name ||
+            before.email !== after.email ||
+            before.company !== after.company ||
+            before.avatarURL !== after.avatarURL ||
+            before.jobRole !== after.jobRole ||
+            before.jobRoleOther !== after.jobRoleOther ||
+            // not checking explorationReasons or signupGoals atm as it's an array - need to check deep equality
+            before.signupGoalsOther !== after.signupGoalsOther ||
+            before.onboardedTimestamp !== after.onboardedTimestamp ||
+            before.companySize !== after.companySize
+        );
+    }
+
+    export function getProfile(user: User): Profile {
+        const profile = user.additionalData?.profile;
+        return {
+            name: getName(user) || "",
+            email: getPrimaryEmail(user) || "",
+            company: profile?.companyName,
+            avatarURL: user?.avatarUrl,
+            jobRole: profile?.jobRole,
+            jobRoleOther: profile?.jobRoleOther,
+            explorationReasons: profile?.explorationReasons,
+            signupGoals: profile?.signupGoals,
+            signupGoalsOther: profile?.signupGoalsOther,
+            companySize: profile?.companySize,
+            onboardedTimestamp: profile?.onboardedTimestamp,
+        };
+    }
+}
