FoundKeyGang/FoundKey
FoundKeyGang/FoundKey
7
19
Fork 22
Code Issues 37 Pull requests 4 Releases Activity

handle Update activities for Notes #395

Manually merged
Johann150 merged 7 commits from note-editing into main 2023-07-02 08:07:24 +00:00
Conversation 1 Commits 7 Files changed 17 +695 -419
Johann150 commented 2023-06-09 16:12:55 +00:00
Owner
Copy link

This adds handling for Update activities whose object is a Note (aka "editing").

The things that can be edited are:

  • content (ap:content)
    consequently also the contained emoji and hashtags
  • content warning (ap:summary)
  • human readable URL (ap:url)
  • name (ap:name)
  • attached files and their properties (e.g. image descriptions)

Anything else that is sent in the activity is disregarded as updating it is not supported, in particular changing the recipients/addressing information in this way is not supported.

This adds handling for `Update` activities whose `object` is a `Note` (aka "editing"). The things that can be edited are: - content (`ap:content`) consequently also the contained emoji and hashtags - content warning (`ap:summary`) - human readable URL (`ap:url`) - name (`ap:name`) - attached files and their properties (e.g. image descriptions) Anything else that is sent in the activity is disregarded as updating it is not supported, in particular changing the recipients/addressing information in this way is not supported.
Johann150 added the
feature
 label 2023-06-09 16:12:55 +00:00
Johann150 added a new dependency 2023-06-09 16:13:18 +00:00
#294 Handle edits
helene requested changes 2023-06-09 19:35:05 +00:00
helene left a comment
Contributor
Copy link

Much cleaner than whatever hacked-up copypastes I could've came up with; I can't attest to it working or not (I don't run an instance and can't tell from the code itself as I'm not familiar enough with the codebase) but I don't have much to say here other than it looks good to me and the refactoring (splitting side-effects etc in a different file for Notes) is pretty important and helps a lot for the cleanliness and clarity of this feature.

Much cleaner than whatever hacked-up copypastes I could've came up with; I can't attest to it working or not (I don't run an instance and can't tell from the code itself as I'm not familiar enough with the codebase) but I don't have much to say here other than it looks good to me and the refactoring (splitting side-effects etc in a different file for Notes) is pretty important and helps a lot for the cleanliness and clarity of this feature.
packages/backend/src/models/repositories/note.ts Outdated
@ -169,6 +169,7 @@ export const NoteRepository = db.getRepository(Note).extend({
const packed: Packed<'Note'> = await awaitAll({
id: note.id,
createdAt: note.createdAt.toISOString(),
updatedAt: note.udpatedAt?.toISOString() ?? null,
helene commented 2023-06-09 19:15:19 +00:00
Contributor
Copy link

s/udpatedAt/updatedAt

`s/udpatedAt/updatedAt`
Johann150 marked this conversation as resolved
packages/backend/src/remote/activitypub/kernel/update/note.ts Outdated
@ -0,0 +16,4 @@
// and since this is not a direct creation, handle it silently
createNote(resolver, actor, note, true);
const unlock = await getApLock(uri);
helene commented 2023-06-09 19:27:14 +00:00
Contributor
Copy link

I assume this is used to get exclusive ownership of an AP object?

I assume this is used to get exclusive ownership of an AP object?
Johann150 commented 2023-06-11 12:53:57 +00:00
Author
Owner
Copy link

Yes this is also used in the respective AP kernel function for Create/Note.

Yes this is also used in the respective AP kernel function for `Create/Note`.
helene commented 2023-06-11 16:13:39 +00:00
Contributor
Copy link

I'd assume it's also used for Deletes; it does not seem to be used in the alternative path for updateNote though, which could be a problem (I assume that is why there was a unlock() there but no matching definition)

I'd assume it's also used for `Delete`s; it does not seem to be used in the alternative path for `updateNote` though, which could be a problem (I assume that is why there was a `unlock()` there but no matching definition)
Johann150 marked this conversation as resolved
packages/backend/src/remote/activitypub/kernel/update/note.ts Outdated
@ -0,0 +22,4 @@
const existsNow = await Notes.findOneByOrFail({ uri });
// set the updatedAt timestamp since the note was changed
await Notes.update(existsNow.id, { updatedAt: new Date() });
return 'ok: unkown note created and marked as updated';
helene commented 2023-06-09 19:26:15 +00:00
Contributor
Copy link

s/unkown/unknown

`s/unkown/unknown`
Johann150 marked this conversation as resolved
packages/backend/src/remote/activitypub/kernel/update/note.ts Outdated
@ -0,0 +43,4 @@
} catch (e) {
return `skip: update note rejected: ${e.message}`;
} finally {
unlock();
helene commented 2023-06-09 19:30:10 +00:00
Contributor
Copy link

unlock is undefined here, I suggest removing the finally block

`unlock` is undefined here, I suggest removing the `finally` block
Johann150 marked this conversation as resolved
Johann150 force-pushed note-editing from f0f497c09c to 9665c04ce3 2023-06-11 17:51:06 +00:00 Compare
norm commented 2023-06-19 18:13:46 +00:00
Owner
Copy link

Running this branch on misskey.biribiri.dev right now, doesn't seem like updates are even processed for some reason. The logs don't seem to reveal anything related to update activities.

Running this branch on misskey.biribiri.dev right now, doesn't seem like updates are even processed for some reason. The logs don't seem to reveal anything related to update activities.
Johann150 force-pushed note-editing from 9665c04ce3 to 723eed294c 2023-06-27 20:05:10 +00:00 Compare
Johann150 force-pushed note-editing from 900f980cc8 to 8ec579c894 2023-06-27 20:26:48 +00:00 Compare
Johann150 force-pushed note-editing from 8ec579c894 to 1f542265ee 2023-06-29 19:49:38 +00:00 Compare
Johann150 force-pushed note-editing from 1f542265ee to eb91b1e03f 2023-07-01 20:02:44 +00:00 Compare
Johann150 force-pushed note-editing from eb91b1e03f to 796adc8599 2023-07-02 08:07:08 +00:00 Compare
Johann150 manually merged commit 796adc8599 into main 2023-07-02 08:07:24 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
helene
Labels
Clear labels   
feature

task that proposes new feature(s)

  
fix

task to fix behavioral problems in the FoundKey codebase

  
upkeep

task that should make FoundKey smaller and more maintainable

No labels
feature
fix
upkeep
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#294 Handle edits}
FoundKeyGang/FoundKey
Reference: FoundKeyGang/FoundKey#395
No description provided.
Delete branch "note-editing"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?