This is nullable now

Should this be null though? When would an account never have a followers URL?

Great question, I don't know - but it's nullable in the DB, so I think we should fix that all together.

This code is weird and shouldn't really live here, or at least not be exposed like this - why does an Account have a getApIdForPost method lol

What about this just being type AccountDraft = Omit<Account, 'id'> so we don't have to list all of the account properties here?

Yeah so - my thinking here is that these two types at the top of the file serve as documentation, and the duplication is preferred because it's easier to read right away. What do you think about that?

Ah ok, yeh i'm fine with that, i guess it's unlikely they are going to come out of sync without us knowing

I just wanted to avoid the repetition of

username: params.username === undefined ? this.username : params.username,

But I can put it back if it's preferred

Nah, this is a cool utility, maybe we have generic utils module thing somewhere?

I did try to pull it out - but unfortunately it does't work well when you pull it out, because you need to pass in the this value and it becomes a bit of a mess like get(this, params, 'key') PLUS the types become super fucky

Ah cool, let leave it in

🛠️ Refactor suggestion

⚠️ Potential issue

updateProfile can assign undefined to non-optional fields

get('username') (and the other calls) returns string | undefined, but username in Data<Account> is required (string).
With strictNullChecks enabled this does not type-check and at runtime may propagate an undefined username.

A safer, fully-typed alternative:

-        type P = ProfileUpdateParams;
-        const get = <K extends keyof P>(prop: K): P[K] =>
-            params[prop] === undefined ? this[prop] : params[prop];
-
-        return AccountEntity.create({
-            ...this,
-            username: get('username'),
-            name: get('name'),
-            bio: get('bio'),
-            avatarUrl: get('avatarUrl'),
-            bannerImageUrl: get('bannerImageUrl'),
-        });
+        return AccountEntity.create({
+            ...this,
+            username: params.username ?? this.username,
+            name: params.name ?? this.name,
+            bio: params.bio ?? this.bio,
+            avatarUrl: params.avatarUrl ?? this.avatarUrl,
+            bannerImageUrl: params.bannerImageUrl ?? this.bannerImageUrl,
+        });

This keeps the implementation concise, avoids the helper closure, and preserves correct types.

I don't think this is true, we explicitly do not return undefined if it is that on the params object, and instead return the prop from this which does match the type required in Data<Account>

@allouis You're absolutely right. Looking closer at your implementation, the runtime behavior is perfectly safe:

const get = <K extends keyof P>(prop: K): P[K] =>
    params[prop] === undefined ? this[prop] : params[prop];

This correctly falls back to this[prop] when params[prop] is undefined, so it will never return undefined for required fields.

The only subtlety is that TypeScript's type system declares the return type as P[K] which includes undefined for optional properties, so static type checking might complain even though the runtime behavior is correct.

Thanks for the clarification and pushing back on my suggestion!

Learnt from: allouis
PR: TryGhost/ActivityPub#566
File: src/account/account.entity.ts:110-123
Timestamp: 2025-04-24T10:41:15.039Z
Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.

Yeah I can probably put this somewhere else, but where?