From f76e316aa654473ff88ba5c537afae76c2209455 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 28 Oct 2024 16:10:08 -0700 Subject: [PATCH 1/4] chore(ui): add feedback_replace to ui, update listeners w/ feedback_ref --- .../wfReactInterface/traceServerClient.ts | 56 +++++++++++++++---- .../traceServerClientTypes.ts | 10 ++++ .../traceServerDirectClient.ts | 9 +++ 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index af997016a4a..227aa90f9f4 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -5,6 +5,8 @@ import { FeedbackCreateRes, FeedbackPurgeReq, FeedbackPurgeRes, + FeedbackReplaceReq, + FeedbackReplaceRes, TraceCallsDeleteReq, TraceCallUpdateReq, TraceRefsReadBatchReq, @@ -23,7 +25,13 @@ export class TraceServerClient extends DirectTraceServerClient { }> = []; private onDeleteListeners: Array<() => void>; private onRenameListeners: Array<() => void>; - private onFeedbackListeners: Record void>>; + // weave_ref -> feedback_ref -> callback + // For feedback without a feedback_ref, (the default case) + // the key is the empty string (this.FEEDBACK_REF_DEFAULT). + private onFeedbackListeners: Record< + string, + Record void>> + >; constructor(baseUrl: string) { super(baseUrl); @@ -34,6 +42,8 @@ export class TraceServerClient extends DirectTraceServerClient { this.onFeedbackListeners = {}; } + private FEEDBACK_REF_DEFAULT = ''; + /** * Registers a callback to be called when a delete operation occurs. * This method is purely for local notification within the client @@ -60,20 +70,25 @@ export class TraceServerClient extends DirectTraceServerClient { } public registerOnFeedbackListener( weaveRef: string, - callback: () => void + callback: () => void, + feedbackRef?: string ): () => void { - if (!(weaveRef in this.onFeedbackListeners)) { - this.onFeedbackListeners[weaveRef] = []; + const feedbackRefResolved = feedbackRef ?? this.FEEDBACK_REF_DEFAULT; + if (!(feedbackRefResolved in this.onFeedbackListeners)) { + this.onFeedbackListeners[feedbackRefResolved] = {}; + } + if (!(weaveRef in this.onFeedbackListeners[feedbackRefResolved])) { + this.onFeedbackListeners[feedbackRefResolved][weaveRef] = []; } - this.onFeedbackListeners[weaveRef].push(callback); + this.onFeedbackListeners[feedbackRefResolved][weaveRef].push(callback); return () => { - const newListeners = this.onFeedbackListeners[weaveRef].filter( - listener => listener !== callback - ); + const newListeners = this.onFeedbackListeners[feedbackRefResolved][ + weaveRef + ].filter(listener => listener !== callback); if (newListeners.length) { - this.onFeedbackListeners[weaveRef] = newListeners; + this.onFeedbackListeners[feedbackRefResolved][weaveRef] = newListeners; } else { - delete this.onFeedbackListeners[weaveRef]; + delete this.onFeedbackListeners[feedbackRefResolved][weaveRef]; } }; } @@ -94,7 +109,10 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackCreate(req: FeedbackCreateReq): Promise { const res = super.feedbackCreate(req).then(createRes => { - const listeners = this.onFeedbackListeners[req.weave_ref] ?? []; + const feedbackRefResolved = + req.payload?.feedback_ref ?? this.FEEDBACK_REF_DEFAULT; + const listeners = + this.onFeedbackListeners[feedbackRefResolved][req.weave_ref] ?? []; listeners.forEach(listener => listener()); return createRes; }); @@ -106,13 +124,27 @@ export class TraceServerClient extends DirectTraceServerClient { // information about the refs that were modified. // For now, just call all registered feedback listeners. for (const listeners of Object.values(this.onFeedbackListeners)) { - listeners.forEach(listener => listener()); + for (const listenersForWeaveRef of Object.values(listeners)) { + listenersForWeaveRef.forEach(listener => listener()); + } } return purgeRes; }); return res; } + public feedbackReplace(req: FeedbackReplaceReq): Promise { + const res = super.feedbackReplace(req).then(replaceRes => { + const feedbackRefResolved = + req.payload?.feedback_ref ?? this.FEEDBACK_REF_DEFAULT; + const listeners = + this.onFeedbackListeners[feedbackRefResolved][req.weave_ref] ?? []; + listeners.forEach(listener => listener()); + return replaceRes; + }); + return res; + } + public readBatch(req: TraceRefsReadBatchReq): Promise { return this.requestReadBatch(req); } diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts index 88113a37a74..2181f2db758 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts @@ -190,6 +190,16 @@ export type FeedbackPurgeError = { detail: string; }; export type FeedbackPurgeRes = FeedbackPurgeSuccess | FeedbackPurgeError; + +export type FeedbackReplaceReq = FeedbackCreateReq & { + feedback_id: string; +}; +export type FeedbackReplaceSuccess = {}; +export type FeedbackReplaceError = { + detail: string; +}; +export type FeedbackReplaceRes = FeedbackCreateRes; + interface TraceObjectsFilter { base_object_classes?: string[]; object_ids?: string[]; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts index caaf63b7f56..e1ee4fb8a8f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts @@ -23,6 +23,8 @@ import { FeedbackPurgeRes, FeedbackQueryReq, FeedbackQueryRes, + FeedbackReplaceReq, + FeedbackReplaceRes, TraceCallReadReq, TraceCallReadRes, TraceCallSchema, @@ -268,6 +270,13 @@ export class DirectTraceServerClient { ); } + public feedbackReplace(req: FeedbackReplaceReq): Promise { + return this.makeRequest( + '/feedback/replace', + req + ); + } + public fileContent( req: TraceFileContentReadReq ): Promise { From cbcdd33aa8463da93f63c502358abcf6f710d947 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 7 Nov 2024 10:34:51 -0800 Subject: [PATCH 2/4] typ --- .../Browse3/pages/wfReactInterface/traceServerClient.ts | 6 +++--- .../pages/wfReactInterface/traceServerClientTypes.ts | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index 227aa90f9f4..fc50a73ffd2 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -12,7 +12,7 @@ import { TraceRefsReadBatchReq, TraceRefsReadBatchRes, } from './traceServerClientTypes'; -import {DirectTraceServerClient} from './traceServerDirectClient'; +import { DirectTraceServerClient } from './traceServerDirectClient'; const DEFAULT_BATCH_INTERVAL = 150; const MAX_REFS_PER_BATCH = 1000; @@ -110,7 +110,7 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackCreate(req: FeedbackCreateReq): Promise { const res = super.feedbackCreate(req).then(createRes => { const feedbackRefResolved = - req.payload?.feedback_ref ?? this.FEEDBACK_REF_DEFAULT; + req.annotation_ref ?? this.FEEDBACK_REF_DEFAULT; const listeners = this.onFeedbackListeners[feedbackRefResolved][req.weave_ref] ?? []; listeners.forEach(listener => listener()); @@ -136,7 +136,7 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackReplace(req: FeedbackReplaceReq): Promise { const res = super.feedbackReplace(req).then(replaceRes => { const feedbackRefResolved = - req.payload?.feedback_ref ?? this.FEEDBACK_REF_DEFAULT; + req.annotation_ref ?? this.FEEDBACK_REF_DEFAULT; const listeners = this.onFeedbackListeners[feedbackRefResolved][req.weave_ref] ?? []; listeners.forEach(listener => listener()); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts index 98bff37a822..41d067f081e 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts @@ -1,4 +1,4 @@ -import {Query} from './traceServerClientInterface/query'; +import { Query } from './traceServerClientInterface/query'; type ExtraKeysAllowed = { [key: string]: any; }; @@ -144,6 +144,10 @@ export type FeedbackCreateReq = { weave_ref: string; feedback_type: string; payload: Record; + annotation_ref?: string; + runnable_ref?: string; + call_ref?: string; + trigger_ref?: string; }; export type FeedbackCreateSuccess = { From 91ea1fb3e463d544f0372152f2dfc7fa820e2ac4 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 7 Nov 2024 13:31:51 -0800 Subject: [PATCH 3/4] wip --- .../wfReactInterface/traceServerClient.ts | 42 +++++++++---------- .../traceServerClientTypes.ts | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index fc50a73ffd2..13a1f4dcf9a 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -12,7 +12,7 @@ import { TraceRefsReadBatchReq, TraceRefsReadBatchRes, } from './traceServerClientTypes'; -import { DirectTraceServerClient } from './traceServerDirectClient'; +import {DirectTraceServerClient} from './traceServerDirectClient'; const DEFAULT_BATCH_INTERVAL = 150; const MAX_REFS_PER_BATCH = 1000; @@ -25,9 +25,9 @@ export class TraceServerClient extends DirectTraceServerClient { }> = []; private onDeleteListeners: Array<() => void>; private onRenameListeners: Array<() => void>; - // weave_ref -> feedback_ref -> callback - // For feedback without a feedback_ref, (the default case) - // the key is the empty string (this.FEEDBACK_REF_DEFAULT). + // weave_ref -> feedback_type -> callback + // For feedback without a feedback_type, (the default case) + // the key is the empty string (this.FEEDBACK_TYPE_DEFAULT). private onFeedbackListeners: Record< string, Record void>> @@ -42,7 +42,7 @@ export class TraceServerClient extends DirectTraceServerClient { this.onFeedbackListeners = {}; } - private FEEDBACK_REF_DEFAULT = ''; + private FEEDBACK_TYPE_DEFAULT = ''; /** * Registers a callback to be called when a delete operation occurs. @@ -71,24 +71,24 @@ export class TraceServerClient extends DirectTraceServerClient { public registerOnFeedbackListener( weaveRef: string, callback: () => void, - feedbackRef?: string + feedbackType?: string ): () => void { - const feedbackRefResolved = feedbackRef ?? this.FEEDBACK_REF_DEFAULT; - if (!(feedbackRefResolved in this.onFeedbackListeners)) { - this.onFeedbackListeners[feedbackRefResolved] = {}; + const feedbackTypeResolved = feedbackType ?? this.FEEDBACK_TYPE_DEFAULT; + if (!(feedbackTypeResolved in this.onFeedbackListeners)) { + this.onFeedbackListeners[feedbackTypeResolved] = {}; } - if (!(weaveRef in this.onFeedbackListeners[feedbackRefResolved])) { - this.onFeedbackListeners[feedbackRefResolved][weaveRef] = []; + if (!(weaveRef in this.onFeedbackListeners[feedbackTypeResolved])) { + this.onFeedbackListeners[feedbackTypeResolved][weaveRef] = []; } - this.onFeedbackListeners[feedbackRefResolved][weaveRef].push(callback); + this.onFeedbackListeners[feedbackTypeResolved][weaveRef].push(callback); return () => { - const newListeners = this.onFeedbackListeners[feedbackRefResolved][ + const newListeners = this.onFeedbackListeners[feedbackTypeResolved][ weaveRef ].filter(listener => listener !== callback); if (newListeners.length) { - this.onFeedbackListeners[feedbackRefResolved][weaveRef] = newListeners; + this.onFeedbackListeners[feedbackTypeResolved][weaveRef] = newListeners; } else { - delete this.onFeedbackListeners[feedbackRefResolved][weaveRef]; + delete this.onFeedbackListeners[feedbackTypeResolved][weaveRef]; } }; } @@ -109,10 +109,10 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackCreate(req: FeedbackCreateReq): Promise { const res = super.feedbackCreate(req).then(createRes => { - const feedbackRefResolved = - req.annotation_ref ?? this.FEEDBACK_REF_DEFAULT; + const feedbackTypeResolved = + req.feedback_type ?? this.FEEDBACK_TYPE_DEFAULT; const listeners = - this.onFeedbackListeners[feedbackRefResolved][req.weave_ref] ?? []; + this.onFeedbackListeners[feedbackTypeResolved][req.weave_ref] ?? []; listeners.forEach(listener => listener()); return createRes; }); @@ -135,10 +135,10 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackReplace(req: FeedbackReplaceReq): Promise { const res = super.feedbackReplace(req).then(replaceRes => { - const feedbackRefResolved = - req.annotation_ref ?? this.FEEDBACK_REF_DEFAULT; + const feedbackTypeResolved = + req.feedback_type ?? this.FEEDBACK_TYPE_DEFAULT; const listeners = - this.onFeedbackListeners[feedbackRefResolved][req.weave_ref] ?? []; + this.onFeedbackListeners[feedbackTypeResolved][req.weave_ref] ?? []; listeners.forEach(listener => listener()); return replaceRes; }); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts index 41d067f081e..8147857a5fb 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts @@ -1,4 +1,4 @@ -import { Query } from './traceServerClientInterface/query'; +import {Query} from './traceServerClientInterface/query'; type ExtraKeysAllowed = { [key: string]: any; }; From 005a1654e1f4a180142d7664fc377e6ba904eb90 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 7 Nov 2024 18:15:25 -0800 Subject: [PATCH 4/4] fix --- .../wfReactInterface/traceServerClient.ts | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index 13a1f4dcf9a..febafdc7f32 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -26,8 +26,6 @@ export class TraceServerClient extends DirectTraceServerClient { private onDeleteListeners: Array<() => void>; private onRenameListeners: Array<() => void>; // weave_ref -> feedback_type -> callback - // For feedback without a feedback_type, (the default case) - // the key is the empty string (this.FEEDBACK_TYPE_DEFAULT). private onFeedbackListeners: Record< string, Record void>> @@ -74,21 +72,21 @@ export class TraceServerClient extends DirectTraceServerClient { feedbackType?: string ): () => void { const feedbackTypeResolved = feedbackType ?? this.FEEDBACK_TYPE_DEFAULT; - if (!(feedbackTypeResolved in this.onFeedbackListeners)) { - this.onFeedbackListeners[feedbackTypeResolved] = {}; + if (!(weaveRef in this.onFeedbackListeners)) { + this.onFeedbackListeners[weaveRef] = {}; } - if (!(weaveRef in this.onFeedbackListeners[feedbackTypeResolved])) { - this.onFeedbackListeners[feedbackTypeResolved][weaveRef] = []; + if (!(feedbackTypeResolved in this.onFeedbackListeners[weaveRef])) { + this.onFeedbackListeners[weaveRef][feedbackTypeResolved] = []; } - this.onFeedbackListeners[feedbackTypeResolved][weaveRef].push(callback); + this.onFeedbackListeners[weaveRef][feedbackTypeResolved].push(callback); return () => { - const newListeners = this.onFeedbackListeners[feedbackTypeResolved][ - weaveRef + const newListeners = this.onFeedbackListeners[weaveRef][ + feedbackTypeResolved ].filter(listener => listener !== callback); if (newListeners.length) { - this.onFeedbackListeners[feedbackTypeResolved][weaveRef] = newListeners; + this.onFeedbackListeners[weaveRef][feedbackTypeResolved] = newListeners; } else { - delete this.onFeedbackListeners[feedbackTypeResolved][weaveRef]; + delete this.onFeedbackListeners[weaveRef][feedbackTypeResolved]; } }; } @@ -109,10 +107,13 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackCreate(req: FeedbackCreateReq): Promise { const res = super.feedbackCreate(req).then(createRes => { - const feedbackTypeResolved = - req.feedback_type ?? this.FEEDBACK_TYPE_DEFAULT; const listeners = - this.onFeedbackListeners[feedbackTypeResolved][req.weave_ref] ?? []; + this.onFeedbackListeners[req.weave_ref]?.[req.feedback_type] ?? []; + listeners.push( + ...(this.onFeedbackListeners?.[req.weave_ref]?.[ + this.FEEDBACK_TYPE_DEFAULT + ] ?? []) + ); listeners.forEach(listener => listener()); return createRes; }); @@ -135,10 +136,14 @@ export class TraceServerClient extends DirectTraceServerClient { public feedbackReplace(req: FeedbackReplaceReq): Promise { const res = super.feedbackReplace(req).then(replaceRes => { - const feedbackTypeResolved = - req.feedback_type ?? this.FEEDBACK_TYPE_DEFAULT; const listeners = - this.onFeedbackListeners[feedbackTypeResolved][req.weave_ref] ?? []; + this.onFeedbackListeners[req.weave_ref]?.[req.feedback_type] ?? []; + // also fire the default feedback type listeners + listeners.push( + ...(this.onFeedbackListeners?.[req.weave_ref]?.[ + this.FEEDBACK_TYPE_DEFAULT + ] ?? []) + ); listeners.forEach(listener => listener()); return replaceRes; });