SCAL-338185: Pass analystId via APP_INIT embedParams.spotterAnalystConfig - #669
Conversation
commit: |
…nfig The Spotter app reads the analyst config as spotterAnalystConfig.analystId off the APP_INIT payload, but the SDK was flattening it into an `analystId` query param on the iframe src, so the app never picked it up. - Add buildSpotterAnalystAppInitData(), mirroring the share-conversation and starter-prompts builders, and chain it into SpotterEmbed.getAppInitData(). The config is omitted entirely when analystId is absent, so an empty spotterAnalystConfig sends nothing rather than an empty object. - Drop the AnalystId query param and its now-unused Param enum member.
22db827 to
a2d10fe
Compare
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
|
@gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors how spotterAnalystConfig (specifically analystId) is transmitted. Instead of appending it to the iframe URL as a query parameter, it is now sent via the APP_INIT event payload under embedParams. This change includes removing AnalystId from the Param enum, introducing the buildSpotterAnalystAppInitData utility function, and updating the corresponding unit tests to verify the new initialization flow. I have no additional feedback to provide as the implementation is clean and well-tested.
There was a problem hiding this comment.
Code Review
This pull request refactors the Spotter embed to pass analystId (via spotterAnalystConfig) through the APP_INIT payload (embedParams.spotterAnalystConfig) instead of as a URL query parameter. The changes include updating SpotterEmbed initialization, implementing the buildSpotterAnalystAppInitData utility, removing the deprecated AnalystId query parameter, and updating the unit tests accordingly. There are no review comments, and I have no feedback to provide.
3 participants
What
spotterAnalystConfig.analystIdwas being flattened into ananalystIdquery param on the iframesrc. The Spotter app reads the analyst config asspotterAnalystConfig.analystIdoff theAPP_INITpayload, so the value never reached it. This moves it toembedParams, alongside the other Spotter object-shaped configs.Changes
buildSpotterAnalystAppInitData()inspotter-utils.ts, mirroringbuildSpotterShareConversationAppInitData/buildStarterPromptsAppInitData, and chained it intoSpotterEmbed.getAppInitData(). It preserves existingembedParamskeys and returns the payload untouched when there is noanalystId.spotterAnalystConfigonSpotterAppInitData.embedParams.AnalystIdquery param write and the now-unusedParam.AnalystIdenum member (Paramis not exported fromindex.ts, so this is internal surface only).SpotterAnalystConfiginterface that an earlier merge with main left behind.Behaviour notes
spotterAnalystConfig: {}sends nothing at all rather than an empty object, matching the previous URL-param behaviour and sparing the app from distinguishing "no analyst pinned" from "empty config".spotterSidebarConfigalready makes. It is also no longer exposed in the iframe URL, browser history, or proxy logs.SpotterEmbedViewConfig.spotterAnalystConfigis unchanged for hosts.Testing
conversation.spec.tswith one assertinganalystIdis absent from the iframe src, plus fourAPP_INITtests (present; coexisting withspotterSidebarConfig; absent when unconfigured; absent when{}).spotter-utils.spec.ts.conversation.spec.ts+spotter-utils.spec.ts: 108 passed.tsc --noEmitand eslint clean.