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
12 changes: 8 additions & 4 deletions features/handle-mention.feature
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
Feature: Incoming mentions

Scenario: We receive a Create(Note) with a mention
Scenario: We receive a public mention from someone
Given an Actor "Person(Alice)"
And a "Create(Note)" Activity "Note" by "Alice" with content "Hello @index@site.com" that mentions "Us"
When "Alice" sends "Note" to the Inbox
Then the request is accepted
When "Alice" sends us a public mention
And the mention is in our notifications

Scenario: We receive a private mention from someone
Given an Actor "Person(Alice)"
When "Alice" sends us a private mention
And the mention is not in our notifications
45 changes: 0 additions & 45 deletions features/step_definitions/activitypub_steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,46 +67,6 @@ async function activityCreatedByWithContent(
}
}

async function activityCreatedByWithMention(
activityDef,
name,
actorName,
content,
mentionedActorName,
) {
const { activity: activityType, object: objectName } =
parseActivityString(activityDef);
if (!activityType) {
throw new Error(`could not match ${activityDef} to an activity`);
}

const actor = this.actors[actorName];
const mentionedActor = this.actors[mentionedActorName];
const tags = [
{
type: 'Mention',
name: `@${mentionedActor.username}@${mentionedActor.domain}`,
href: mentionedActor.apId,
},
];
const object =
this.actors[objectName] ??
this.activities[objectName] ??
this.objects[objectName] ??
(await createObject(objectName, actor, content, tags));

const activity = await createActivity(activityType, object, actor);

const parsed = parseActivityString(name);
if (parsed.activity === null || parsed.object === null) {
this.activities[name] = activity;
this.objects[name] = object;
} else {
this.activities[parsed.activity] = activity;
this.objects[parsed.object] = object;
}
}

Given('a {string} Activity {string} by {string}', activityCreatedBy);

Given(
Expand All @@ -119,11 +79,6 @@ Given(
activityCreatedBy,
);

Given(
'a {string} Activity {string} by {string} with content {string} that mentions {string}',
activityCreatedByWithMention,
);

Given('an Actor {string}', async function (actorDef) {
const { type, name } = parseActorString(actorDef);

Expand Down
125 changes: 125 additions & 0 deletions features/step_definitions/mention_steps.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { Then, When } from '@cucumber/cucumber';

import assert from 'node:assert';

import { createActivity, createObject } from '../support/fixtures.js';
import { waitForItemInNotifications } from '../support/notifications.js';
import { fetchActivityPub } from '../support/request.js';

When('{string} sends us a public mention', async function (actorName) {
const actor = this.actors[actorName];
if (!actor) {
throw new Error(
`Actor ${actorName} not found - did you forget a step?`,
);
}

// Use the existing local actor (Us) from the test context
const localActor = this.actors.Us;
if (!localActor) {
throw new Error('Local actor (Us) not found in test context');
}

const mention = `@${localActor.preferredUsername}@self.test`;
const tags = [
{
type: 'Mention',
name: mention,
href: localActor.id,
},
];

// Create a public Note with the mention (uses defaults: to='as:Public')
const object = await createObject('Note', actor, `Hello ${mention}`, tags);
const activity = await createActivity('Create', object, actor);

await fetchActivityPub('https://self.test/.ghost/activitypub/inbox/index', {
method: 'POST',
headers: {
'Content-Type': 'application/ld+json',
},
body: JSON.stringify(activity),
});

this.mentionId = object.id;
this.activities['Create(Note)'] = activity;
this.objects.Note = object;
});

When('{string} sends us a private mention', async function (actorName) {
const actor = this.actors[actorName];
if (!actor) {
throw new Error(
`Actor ${actorName} not found - did you forget a step?`,
);
}

// Use the existing local actor (Us) from the test context
const localActor = this.actors.Us;
if (!localActor) {
throw new Error('Local actor (Us) not found in test context');
}

const mention = `@${localActor.preferredUsername}@self.test`;
const tags = [
{
type: 'Mention',
name: mention,
href: localActor.id,
},
];

// Create a Note with the mention
const object = await createObject('Note', actor, `Hello ${mention}`, tags);

// Make it private by addressing it only to the mentioned user
object.to = localActor.id;
object.cc = [];

const activity = await createActivity('Create', object, actor);
activity.to = localActor.id;
activity.cc = [];

await fetchActivityPub('https://self.test/.ghost/activitypub/inbox/index', {
method: 'POST',
headers: {
'Content-Type': 'application/ld+json',
},
body: JSON.stringify(activity),
});

this.mentionId = object.id;
this.activities['Create(Note)'] = activity;
this.objects.Note = object;
});

Then('the mention is in our notifications', async function () {
if (!this.mentionId) {
throw new Error(
'You need to call a step which creates a mention before this',
);
}

const found = await waitForItemInNotifications(this.mentionId);
assert(found, `Expected mention ${this.mentionId} to be in notifications`);
});

Then('the mention is not in our notifications', async function () {
if (!this.mentionId) {
throw new Error(
'You need to call a step which creates a mention before this',
);
}

try {
await waitForItemInNotifications(this.mentionId);
assert.fail(
`Expected mention ${this.mentionId} to not be in notifications`,
);
} catch (error) {
assert.equal(
error.message,
`Max retries reached when waiting on item in notifications`,
);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this - this step is always going to take ~7.5s to run (waiting for the max timeout)

I get why we need to do this, but it doesn't feel great. Would we be better just keeping the happy path at the the e2e level, and then testing the non-happy paths at the unit (or integration) level?

Also, semantically, not receiving a private mention is not really a feature, more an implementation detail (because we don't support private mention) so we probably don't need to document it via cucumber?

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's fair! I wanted to have a e2e test here to reproduce the actual bug that was reported - a private mention that is treated as public content - but agree the negative e2e test is slow.

Let me see if I move this test to unit / integratio

2 changes: 1 addition & 1 deletion features/support/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function waitForItemInNotifications(

if (options.retryCount === MAX_RETRIES) {
throw new Error(
`Max retries reached (${MAX_RETRIES}) when waiting on item in notifications`,
`Max retries reached when waiting on item in notifications`,
);
}

Expand Down
12 changes: 11 additions & 1 deletion src/activity-handlers/create.handler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Context, Create } from '@fedify/fedify';
import { type Context, type Create, PUBLIC_COLLECTION } from '@fedify/fedify';

import type { ContextData } from '@/app';
import { exhaustiveCheck, getError, isError } from '@/core/result';
Expand All @@ -21,6 +21,16 @@ export class CreateHandler {
return;
}

const recipients = [...create.toIds, ...create.ccIds].map(
(id) => id.href,
);
const isPublic = recipients.includes(PUBLIC_COLLECTION.href);

if (!isPublic) {
ctx.data.logger.info('Create activity is not public - exit');
return;
}

// This handles storing the posts in the posts table
const postResult = await this.postService.getByApId(create.objectId);

Expand Down