social Inbox: webhooks #7
Conversation
api/hooks.ts
Outdated
| url: Type.String({ | ||
| format: 'uri' | ||
| }), | ||
| method: Type.Union([Type.Literal('GET'), Type.Literal('POST'), Type.Literal('PUT'), Type.Literal('DELETE')]), |
There was a problem hiding this comment.
Can you take this type and put it into it's own const at the top called WebHookSchema? Maybe place the type definition into hookstore or hooksystem.
api/hooks.ts
Outdated
| }, async (request, reply) => { | ||
| // const domain = request.params.domain; | ||
| const hook = request.body | ||
| await store.setModerationQueued(hook) |
There was a problem hiding this comment.
The hook store should be set on the actor store.
| await store.setModerationQueued(hook) | |
| await store.forActor(actor).setModerationQueued(hook) |
api/hooks.ts
Outdated
| }), | ||
| response: { | ||
| 200: Type.Object({ | ||
| message: Type.String(), |
There was a problem hiding this comment.
Could you omit the message property and just have the hook in the response? Ideally we should keep the raw objects in get responses without extra wrappers.
api/hooksystem.ts
Outdated
| store: HookStore | ||
| fetch: FetchLike | ||
|
|
||
| constructor (store: HookStore, fetch: FetchLike = globalThis.fetch) { |
There was a problem hiding this comment.
| constructor (store: HookStore, fetch: FetchLike = globalThis.fetch) { | |
| constructor (store: Store, fetch: FetchLike = globalThis.fetch) { |
Hook stores are actor specific so we should take the full store and use store.forActor(actor).hooks
api/hooks.ts
Outdated
| import { Type } from '@sinclair/typebox' | ||
| import { HookStore } from '../store/HookStore' | ||
|
|
||
| export const hookRoutes = (cfg: APIConfig, store: HookStore) => async (server: FastifyTypebox): Promise<void> => { |
There was a problem hiding this comment.
Instead of passing in the hook store, pass in the full Store
Done.
|
store/HookStore.ts
Outdated
| this.db = db | ||
| } | ||
|
|
||
| async setHook (type: string, hook: Hook): Promise<void> { |
There was a problem hiding this comment.
| async setHook (type: string, hook: Hook): Promise<void> { | |
| async set (type: string, hook: Hook): Promise<void> { |
what do you think of removing the word hook from the methods since it will likely be accessed as hooks.set() anyways
Done. |
api/apsystem.ts
Outdated
|
|
||
| constructor (publicURL: string, store: Store, modCheck: ModerationChecker, fetch: FetchLike = globalThis.fetch) { | ||
| this.publicURL = publicURL | ||
| this.store = store | ||
| this.modCheck = modCheck | ||
| this.fetch = fetch | ||
| this.hookSystem = new HookSystem(store, fetch) |
There was a problem hiding this comment.
The hooksystem should be passed into the constructor in case we wanted to mock it.
Ah, missed this one. |
|
Great, thank you! |
hyphacoop/distributed-press-organizing#93