-
Notifications
You must be signed in to change notification settings - Fork 58
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
(EAI-206): Custom fetch options + add authorized user to staging conversation custom data #308
Conversation
}; | ||
|
||
export class ConversationService { | ||
private serverUrl: string; | ||
private fetchOptions?: ConversationFetchOptions; |
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 define this.fetchOptions
in the class constructor so it should always be defined right?
Maybe an opportunity for cleanup in other spots in the class where we currently assume fetchOptions
may be undefined and use nullish coalescing (i.e. ??
)
@@ -222,12 +254,11 @@ export class ConversationService { | |||
}; | |||
|
|||
await fetchEventSource(this.getUrl(path, { stream: "true" }), { | |||
...this.fetchOptions, | |||
// Need to convert Headers to plain object for fetchEventSource | |||
headers: Object.fromEntries(this.fetchOptions?.headers ?? new Headers()), |
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.
Shouldn't this.fetchOptions
and this.fetchOptions.headers
always be defined since we assign it in the constructor?
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.
Couple of small changes in the frontend code but then I think this should be good to go! Tests seems to indicate the the overrides work well. Let's double check in staging after the merge to make sure and then we can cut new releases.
Jira: https://jira.mongodb.org/browse/EAI-206
Changes
In UI library:
In server library:
"Access-Control-Allow-Origin"
header from CORs/app rather than from the data streamer.On server implementation:
requireRequestOrigin()
andrequireValidIpAddress()
middleware and logic to add these to the conversation custom data b/c the new functionality to get the auth user overrides the defaults which used these.Todos:
Notes