-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Assistants] Fix remove
behaviour
#869
Conversation
createdByMe: el.createdById.toString() === (locals.user?._id ?? locals.sessionId).toString(), | ||
})), | ||
assistants: assistants | ||
.filter((el) => userAssistantsSet.has(el._id.toString())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this filter
is the key line
@@ -76,8 +76,11 @@ export const load: LayoutServerLoad = async ({ locals, depends }) => { | |||
.limit(300) | |||
.toArray(); | |||
|
|||
const userAssistants = settings?.assistants?.map((assistantId) => assistantId.toString()) ?? []; | |||
const userAssistantsSet = new Set(userAssistants); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have EqualitySet
that we could bring to this codebase:
/**
* Subclass of Set but works with non-primitives
* using a bijective object to id mapping.
* (passed to the constructor)
*
* By default the mapping is calling the String function on the
* object.
*/
export class EqualitySet<K = ObjectId> extends Set<unknown> {
private oToId: (o: K) => string;
constructor(oToId?: (o: K) => string);
constructor(iterable: Iterable<K>, oToId?: (o: K) => string);
constructor(iterableOrFn?: Iterable<K> | ((o: K) => string), oToId?: (o: K) => string) {
super();
const iterable = oToId
? (iterableOrFn as Iterable<K>)
: iterableOrFn
? Symbol.iterator in iterableOrFn
? (iterableOrFn as Iterable<K>)
: undefined
: undefined;
this.oToId = (iterable ? oToId : (iterableOrFn as (o: K) => string)) || String;
/// do not pass iterable to super()
/// because this.oToId is not yet defined.
if (iterable) {
for (const k of iterable) {
this.add(k);
}
}
}
override delete(key: K): boolean {
return super.delete(this.oToId(key));
}
override has(key: K): boolean {
return super.has(this.oToId(key));
}
override add(key: K): this {
return super.add(this.oToId(key));
}
*toIterable<Ret = K>(idToObject: (str: string) => Ret): Iterable<Ret> {
for (const val of this) {
yield idToObject(val as string);
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the process of chat-ui becoming moon-landing continues :)
Follow up to huggingface#859, specifically to [this comment](huggingface#859 (comment)) Bug: when a user removes an assistant, the assistant was NOT being removed from the settings list. [This PR fixes this problem](huggingface#869 (comment))
Follow up to #859, specifically to this comment
Bug: when a user removes an assistant, the assistant was NOT being removed from the settings list.
This PR fixes this problem