Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 17 additions & 10 deletions features/account-update.feature
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
Feature: Update account information
Background:
Given we are followed by "Alice"

Scenario: Update account information
When an authenticated "put" request is made to "/.ghost/activitypub/account" with data:
"""
{
"name": "Updated Name",
"bio": "Updated bio",
"username": "updatedUsername",
"avatarUrl": "https://example.com/avatar.jpg",
"bannerImageUrl": "https://example.com/banner.jpg"
}
"""
Given an authenticated "put" request is made to "/.ghost/activitypub/account" with the data:
| name | Updated Name |
| bio | Updated bio |
| username | updatedUsername |
| avatarUrl | https://example.com/avatar.jpg |
| bannerImageUrl | https://example.com/banner.jpg |
And the request is accepted with a 200
When an authenticated "get" request is made to "/.ghost/activitypub/account/me"
Then the request is accepted with a 200
And the response contains the account details:
| name | Updated Name |
| bio | Updated bio |
| avatarUrl | https://example.com/avatar.jpg |
| bannerImageUrl | https://example.com/banner.jpg |
| handle | @[email protected] |
And a "Update(Us)" activity is sent to "Alice"
20 changes: 16 additions & 4 deletions features/step_definitions/stepdefs.js
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ When(
);

When(
/^an authenticated (\"(post|put)\"\s)?request is made to "(.*)" with data:$/,
/^an authenticated (\"(post|put)\"\s)?request is made to "(.*)" with the data:$/,
async function (method, path, data) {
this.response = await fetchActivityPub(
`http://fake-ghost-activitypub.test${path}`,
Expand All @@ -693,7 +693,7 @@ When(
Accept: 'application/ld+json',
'Content-Type': 'application/json',
},
body: data,
body: JSON.stringify(data.rowsHash()),
},
);
},
Expand Down Expand Up @@ -1511,7 +1511,6 @@ Then('the request is accepted', async function () {
});

Then('the request is accepted with a {int}', function (statusCode) {
assert(this.response.ok);
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this as its not helpful, the assertion below is more useful

assert.equal(
this.response.status,
statusCode,
Expand Down Expand Up @@ -1650,7 +1649,8 @@ Then(
}
const actor = this.actors[actorName];

const object = this.objects[objectNameOrType];
const object =
this.objects[objectNameOrType] || this.actors[objectNameOrType];

const inboxUrl = new URL(actor.inbox);

Expand Down Expand Up @@ -1991,3 +1991,15 @@ Then('the response contains {string} account details', async function (name) {
assert.equal(typeof responseJson.followedByMe, 'boolean');
assert.equal(typeof responseJson.followsMe, 'boolean');
});

Then('the response contains the account details:', async function (data) {
const responseJson = await this.response.clone().json();

for (const [key, value] of Object.entries(data.rowsHash())) {
assert.equal(
responseJson[key],
value,
`Expected ${key} to be "${value}" but got "${responseJson[key]}"`,
);
}
});
13 changes: 12 additions & 1 deletion src/account/account.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type AccountSite = {
export interface ProfileUpdateParams {
name?: string | null;
bio?: string | null;
username?: string;
avatarUrl?: URL | null;
bannerImageUrl?: URL | null;
}
Expand All @@ -37,13 +38,14 @@ export class Account extends BaseEntity {

private _name: string | null;
private _bio: string | null;
private _username: string;
private _avatarUrl: URL | null;
private _bannerImageUrl: URL | null;

constructor(
public readonly id: number | null,
uuid: string | null,
public readonly username: string,
username: string,
name: string | null,
bio: string | null,
avatarUrl: URL | null,
Expand All @@ -57,6 +59,7 @@ export class Account extends BaseEntity {

this._name = name;
this._bio = bio;
this._username = username;
this._avatarUrl = avatarUrl;
this._bannerImageUrl = bannerImageUrl;

Expand Down Expand Up @@ -90,6 +93,10 @@ export class Account extends BaseEntity {
return this._bio;
}

get username(): string {
return this._username;
}

get avatarUrl(): URL | null {
return this._avatarUrl;
}
Expand All @@ -107,6 +114,10 @@ export class Account extends BaseEntity {
this._bio = params.bio;
}

if (params.username !== undefined) {
this._username = params.username;
}

if (params.avatarUrl !== undefined) {
this._avatarUrl = params.avatarUrl;
}
Expand Down
35 changes: 35 additions & 0 deletions src/account/account.entity.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('Account', () => {

expect(account.name).toBe('Updated Name');
expect(account.bio).toBe('Original Bio');
expect(account.username).toBe('testuser');
expect(account.avatarUrl?.href).toBe(
'https://example.com/original-avatar.png',
);
Expand Down Expand Up @@ -115,6 +116,35 @@ describe('Account', () => {

expect(account.name).toBe('Original Name');
expect(account.bio).toBe('Updated Bio');
expect(account.username).toBe('testuser');
expect(account.avatarUrl?.href).toBe(
'https://example.com/original-avatar.png',
);
expect(account.bannerImageUrl?.href).toBe(
'https://example.com/original-banner.png',
);
});

it('can update username', () => {
const account = new Account(
1,
'test-uuid',
'testuser',
'Original Name',
'Original Bio',
new URL('https://example.com/original-avatar.png'),
new URL('https://example.com/original-banner.png'),
null,
new URL('https://example.com/ap_id'),
new URL('https://example.com/url'),
new URL('https://example.com/followers'),
);

account.updateProfile({ username: 'updatedtestuser' });

expect(account.name).toBe('Original Name');
expect(account.bio).toBe('Original Bio');
expect(account.username).toBe('updatedtestuser');
expect(account.avatarUrl?.href).toBe(
'https://example.com/original-avatar.png',
);
Expand Down Expand Up @@ -144,6 +174,7 @@ describe('Account', () => {

expect(account.name).toBe('Original Name');
expect(account.bio).toBe('Original Bio');
expect(account.username).toBe('testuser');
expect(account.avatarUrl?.href).toBe(
'https://example.com/updated-avatar.png',
);
Expand Down Expand Up @@ -175,6 +206,7 @@ describe('Account', () => {

expect(account.name).toBe('Original Name');
expect(account.bio).toBe('Original Bio');
expect(account.username).toBe('testuser');
expect(account.avatarUrl?.href).toBe(
'https://example.com/original-avatar.png',
);
Expand All @@ -201,6 +233,7 @@ describe('Account', () => {
account.updateProfile({
name: 'Updated Name',
bio: 'Updated Bio',
username: 'updatedtestuser',
avatarUrl: new URL('https://example.com/updated-avatar.png'),
bannerImageUrl: new URL(
'https://example.com/updated-banner.png',
Expand All @@ -209,6 +242,7 @@ describe('Account', () => {

expect(account.name).toBe('Updated Name');
expect(account.bio).toBe('Updated Bio');
expect(account.username).toBe('updatedtestuser');
expect(account.avatarUrl?.href).toBe(
'https://example.com/updated-avatar.png',
);
Expand Down Expand Up @@ -239,6 +273,7 @@ describe('Account', () => {

expect(account.name).toBe('Original Name');
expect(account.bio).toBeNull();
expect(account.username).toBe('testuser');
expect(account.avatarUrl).toBeNull();
expect(account.bannerImageUrl?.href).toBe(
'https://example.com/original-banner.png',
Expand Down
1 change: 1 addition & 0 deletions src/account/account.repository.knex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export class KnexAccountRepository {
.update({
name: account.name,
bio: account.bio,
username: account.username,
avatar_url: account.avatarUrl?.href,
banner_image_url: account.bannerImageUrl?.href,
})
Expand Down
35 changes: 35 additions & 0 deletions src/account/account.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,4 +509,39 @@ export class AccountService {

return newAccount;
}

async updateAccountProfile(
account: Account,
data: {
name: string;
bio: string;
username: string;
avatarUrl: string;
bannerImageUrl: string;
},
) {
const profileData = {
name: data.name,
bio: data.bio,
username: data.username,
avatarUrl: new URL(data.avatarUrl),
bannerImageUrl: new URL(data.bannerImageUrl),
};

if (
account.name === profileData.name &&
account.bio === profileData.bio &&
account.username === profileData.username &&
account.avatarUrl?.toString() ===
profileData.avatarUrl.toString() &&
account.bannerImageUrl?.toString() ===
profileData.bannerImageUrl.toString()
) {
return;
}

account.updateProfile(profileData);

await this.accountRepository.save(account);
}
}
91 changes: 91 additions & 0 deletions src/account/account.service.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';

import type { FedifyContextFactory } from 'activitypub/fedify-context.factory';
import type { AsyncEvents } from 'core/events';
import type { Knex } from 'knex';
import type { Account } from './account.entity';
import type { KnexAccountRepository } from './account.repository.knex';
import { AccountService } from './account.service';

describe('AccountService', () => {
let knex: Knex;
let asyncEvents: AsyncEvents;
let knexAccountRepository: KnexAccountRepository;
let fedifyContextFactory: FedifyContextFactory;
let generateKeyPair: () => Promise<CryptoKeyPair>;
let accountService: AccountService;

beforeEach(() => {
knex = {} as Knex;
asyncEvents = {} as AsyncEvents;
knexAccountRepository = {
save: vi.fn(),
} as unknown as KnexAccountRepository;
fedifyContextFactory = {} as FedifyContextFactory;
generateKeyPair = vi.fn();

accountService = new AccountService(
knex,
asyncEvents,
knexAccountRepository,
fedifyContextFactory,
generateKeyPair,
);
});

describe('updateAccountProfile', () => {
it('should update the account profile with the provided data', async () => {
const account = {
updateProfile: vi.fn(),
} as unknown as Account;
const data = {
name: 'Alice',
bio: 'Eiusmod in cillum elit sit cupidatat reprehenderit ad quis qui consequat officia elit.',
username: 'alice',
avatarUrl: 'https://example.com/avatar/alice.png',
bannerImageUrl: 'https://example.com/banner/alice.png',
};

await accountService.updateAccountProfile(account, data);

expect(account.updateProfile).toHaveBeenCalledWith({
name: data.name,
bio: data.bio,
username: data.username,
avatarUrl: new URL(data.avatarUrl),
bannerImageUrl: new URL(data.bannerImageUrl),
});

expect(knexAccountRepository.save).toHaveBeenCalledWith(account);
});

it('should do nothing if the provided data is the same as the existing account profile', async () => {
const data = {
name: 'Alice',
bio: 'Eiusmod in cillum elit sit cupidatat reprehenderit ad quis qui consequat officia elit.',
username: 'alice',
avatarUrl: 'https://example.com/avatar/alice.png',
bannerImageUrl: 'https://example.com/banner/alice.png',
};

const account = {
name: data.name,
bio: data.bio,
username: data.username,
avatarUrl: new URL(data.avatarUrl),
bannerImageUrl: new URL(data.bannerImageUrl),
updateProfile: vi.fn(),
} as unknown as Account;

await accountService.updateAccountProfile(account, {
name: data.name,
bio: data.bio,
username: data.username,
avatarUrl: data.avatarUrl,
bannerImageUrl: data.bannerImageUrl,
});

expect(knexAccountRepository.save).not.toHaveBeenCalled();
});
});
});
1 change: 0 additions & 1 deletion src/dispatchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ export const keypairDispatcher = (
ctx: Context<ContextData>,
handle: string,
) {
if (handle !== ACTOR_DEFAULT_HANDLE) return [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we do this for the Actor dispatcher too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, updated in dc5de38

const site = await siteService.getSiteByHost(ctx.host);
if (site === null) return [];

Expand Down
8 changes: 7 additions & 1 deletion src/http/api/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,13 @@ export function createUpdateAccountHandler(accountService: AccountService) {
return new Response(JSON.stringify({}), { status: 400 });
}

//Todo: Update the account with the new data
await accountService.updateAccountProfile(account, {
name: data.name,
bio: data.bio,
username: data.username,
avatarUrl: data.avatarUrl,
bannerImageUrl: data.bannerImageUrl,
});

return new Response(JSON.stringify({}), { status: 200 });
};
Expand Down