Refactor ChatProvider into a Redux store
#826
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -7,12 +7,25 @@ import { __, _n, sprintf } from '@wordpress/i18n'; | ||
| import { Icon, external } from '@wordpress/icons'; | ||
| import { useI18n } from '@wordpress/react-i18n'; | ||
| import React, { useState, useEffect, useRef, memo, useCallback, useMemo } from 'react'; | ||
| import { useSelector, useDispatch } from 'react-redux'; | ||
| import { useThemeDetails } from 'src/hooks/use-theme-details'; | ||
| import { cx } from 'src/lib/cx'; | ||
| import { RootState, AppDispatch } from 'src/stores'; | ||
| import { | ||
| setMessages, | ||
| fetchAssistantThunk, | ||
| generateMessage, | ||
| selectChatInput, | ||
| selectIsLoading, | ||
| selectMessages, | ||
| setChatInput, | ||
| updateFromSiteThunk, | ||
| updateFromTheme, | ||
| selectChatApiId, | ||
| Message as MessageType, | ||
| } from 'src/stores/chat-slice'; | ||
| import { AI_GUIDELINES_URL, LIMIT_OF_PROMPTS_PER_USER } from '../constants'; | ||
| import { useAuth } from '../hooks/use-auth'; | ||
| import { useOffline } from '../hooks/use-offline'; | ||
| import { usePromptUsage } from '../hooks/use-prompt-usage'; | ||
| import { useWelcomeMessages } from '../hooks/use-welcome-messages'; | ||
| @@ -95,34 +108,22 @@ const OfflineModeView = () => { | ||
| ); | ||
| }; | ||
|
|
||
| interface AuthenticatedViewProps { | ||
| messages: MessageType[]; | ||
| instanceId: string; | ||
| isAssistantThinking: boolean; | ||
| siteId: string; | ||
| submitPrompt: ( messageToSend: string, isRetry?: boolean ) => void; | ||
| wrapperRef: React.RefObject< HTMLDivElement >; | ||
| } | ||
|
|
||
| const AuthenticatedView = memo( | ||
| ( { | ||
| messages, | ||
| instanceId, | ||
| isAssistantThinking, | ||
| siteId, | ||
| submitPrompt, | ||
| wrapperRef, | ||
| }: AuthenticatedViewProps ) => { | ||
| const lastMessageRef = useRef< HTMLDivElement >( null ); | ||
| @@ -189,34 +190,27 @@ const AuthenticatedView = memo( | ||
| const RenderMessage = useCallback( | ||
| ( { message }: { message: MessageType } ) => ( | ||
| <> | ||
| <ChatMessage id={ `message-chat-${ message.id }` } message={ message } siteId={ siteId }> | ||
| { message.content } | ||
| </ChatMessage> | ||
| { message.failedMessage && ( | ||
| <ErrorNotice submitPrompt={ submitPrompt } messageContent={ message.content } /> | ||
| ) } | ||
| </> | ||
| ), | ||
| [ submitPrompt, siteId ] | ||
| ); | ||
|
|
||
| const RenderLastMessage = useCallback( | ||
| ( { | ||
| showThinking, | ||
| siteId, | ||
| message, | ||
| children, | ||
| }: { | ||
| message: MessageType; | ||
| showThinking: boolean; | ||
| siteId: string; | ||
| children: React.ReactNode; | ||
| } ) => { | ||
| const thinkingAnimation = { | ||
| @@ -236,7 +230,6 @@ const AuthenticatedView = memo( | ||
| id={ `message-chat-${ message.id }` } | ||
| message={ message } | ||
| siteId={ siteId } | ||
| > | ||
| <AnimatePresence mode="wait"> | ||
| { showThinking ? ( | ||
| @@ -261,7 +254,6 @@ const AuthenticatedView = memo( | ||
| <MarkDownWithCode | ||
| message={ message } | ||
| siteId={ siteId } | ||
| content={ message.content } | ||
| /> | ||
| { children } | ||
| @@ -286,15 +278,14 @@ const AuthenticatedView = memo( | ||
| { showLastMessage && ( | ||
| <RenderLastMessage | ||
| siteId={ siteId } | ||
| message={ lastMessage } | ||
| showThinking={ showThinking } | ||
| > | ||
| <div className="flex justify-end"> | ||
| { !! lastMessage.messageApiId && ( | ||
| <ChatRating | ||
| instanceId={ instanceId } | ||
| messageApiId={ lastMessage.messageApiId } | ||
| feedbackReceived={ !! lastMessage.feedbackReceived } | ||
| /> | ||
| ) } | ||
| @@ -352,104 +343,59 @@ const UnauthenticatedView = ( { onAuthenticate }: { onAuthenticate: () => void } | ||
| export function ContentTabAssistant( { selectedSite }: ContentTabAssistantProps ) { | ||
| const inputRef = useRef< HTMLTextAreaElement >( null ); | ||
| const wrapperRef = useRef< HTMLDivElement >( null ); | ||
| const dispatch = useDispatch< AppDispatch >(); | ||
| const chatInput = useSelector( ( state: RootState ) => | ||
| selectChatInput( state, selectedSite.id ) | ||
| ); | ||
| const { isAuthenticated, authenticate, user, client } = useAuth(); | ||
| const instanceId = user?.id ? `${ user.id }_${ selectedSite.id }` : selectedSite.id; | ||
| const chatApiId = useSelector( ( state: RootState ) => selectChatApiId( state, instanceId ) ); | ||
| const messages = useSelector( ( state: RootState ) => selectMessages( state, instanceId ) ); | ||
| const isAssistantThinking = useSelector( ( state: RootState ) => | ||
| selectIsLoading( state, instanceId ) | ||
| ); | ||
| const { userCanSendMessage } = usePromptUsage(); | ||
| const { messages: welcomeMessages, examplePrompts } = useWelcomeMessages(); | ||
| const isOffline = useOffline(); | ||
| const { __ } = useI18n(); | ||
| const lastMessage = messages.length === 0 ? undefined : messages[ messages.length - 1 ]; | ||
| const hasFailedMessage = messages.some( ( msg ) => msg.failedMessage ); | ||
|
|
||
| const { selectedThemeDetails: themeDetails } = useThemeDetails(); | ||
|
|
||
| useEffect( () => { | ||
| dispatch( updateFromSiteThunk( { site: selectedSite } ) ); | ||
| }, [ dispatch, selectedSite ] ); | ||
|
|
||
| useEffect( () => { | ||
| if ( themeDetails ) { | ||
| dispatch( updateFromTheme( themeDetails ) ); | ||
| } | ||
| }, [ dispatch, themeDetails ] ); | ||
|
|
||
| const submitPrompt = useCallback( | ||
| ( chatMessage: string, isRetry?: boolean ) => { | ||
| if ( ! chatMessage || ! client ) { | ||
| return; | ||
| } | ||
|
|
||
| if ( ! isRetry ) { | ||
| dispatch( setChatInput( { siteId: selectedSite.id, input: '' } ) ); | ||
| } | ||
|
|
||
| const newMessageId = isRetry ? messages.length - 1 : messages.length; | ||
| const message = generateMessage( chatMessage, 'user', newMessageId, chatApiId ); | ||
|
|
||
| dispatch( | ||
| fetchAssistantThunk( { client, instanceId, isRetry, message, siteId: selectedSite.id } ) | ||
| ); | ||
| }, | ||
| [ client, dispatch, instanceId, selectedSite.id, messages, chatApiId ] | ||
| ); | ||
|
|
||
| const clearConversation = () => { | ||
| dispatch( setChatInput( { siteId: selectedSite.id, input: '' } ) ); | ||
| dispatch( setMessages( { instanceId, messages: [] } ) ); | ||
| }; | ||
|
|
||
| // We should render only one notice at a time in the bottom area | ||
| @@ -494,8 +440,7 @@ export function ContentTabAssistant( { selectedSite }: ContentTabAssistantProps | ||
| <AuthenticatedView | ||
| messages={ messages } | ||
| isAssistantThinking={ isAssistantThinking } | ||
| instanceId={ instanceId } | ||
| siteId={ selectedSite.id } | ||
| submitPrompt={ submitPrompt } | ||
| wrapperRef={ wrapperRef } | ||
| @@ -513,10 +458,18 @@ export function ContentTabAssistant( { selectedSite }: ContentTabAssistantProps | ||
| <AIInput | ||
| ref={ inputRef } | ||
| disabled={ disabled } | ||
| input={ chatInput } | ||
| setInput={ ( input ) => { | ||
| dispatch( setChatInput( { siteId: selectedSite.id, input } ) ); | ||
|
There was a problem hiding this comment. I was thinking and:
Maybe I am missing something, but so far it looks like overkill. UPDATE: I see, we store values for different chats... sad that we trigger store so many times during typing, but looks like it's optimal option. Unless, we consider saving it in localStorage, it's typical UX approach, to avoid losing data in case of some errors or reloads of window. But it's more as an improvement. So I think it's better to leave it as is. There was a problem hiding this comment. Storing this state in the Redux store helps maintain the bug fixes introduced in #788. Sounds like we're on the same page already, but to respond to your points:
The component state is reset when navigating between tabs, and will soon (see https://github.com/Automattic/dotcom-forge/issues/10219) be reset when navigating between sites, too.
The middleware predicates are very cheap. Worth keeping in mind. |
||
| } } | ||
| handleSend={ () => { | ||
| submitPrompt( inputRef.current?.value ?? '' ); | ||
| } } | ||
| handleKeyDown={ ( event ) => { | ||
| if ( event.key === 'Enter' ) { | ||
| submitPrompt( inputRef.current?.value ?? '' ); | ||
| } | ||
| } } | ||
| clearConversation={ clearConversation } | ||
| isAssistantThinking={ isAssistantThinking } | ||
| /> | ||
|
There was a problem hiding this comment. This isn't a completely exhaustive test suite, but it's pretty good, and the limited changes I had to make should hopefully be reassuring. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,384 @@ | ||||||||||||
| import { createSlice, createAsyncThunk, PayloadAction } from '@reduxjs/toolkit'; | ||||||||||||
| import * as Sentry from '@sentry/electron/renderer'; | ||||||||||||
| import WPCOM from 'wpcom'; | ||||||||||||
| import { CHAT_ID_STORE_KEY, CHAT_MESSAGES_STORE_KEY } from 'src/constants'; | ||||||||||||
| import { getIpcApi } from 'src/lib/get-ipc-api'; | ||||||||||||
| import { RootState } from 'src/stores'; | ||||||||||||
| import { DEFAULT_PHP_VERSION } from 'vendor/wp-now/src/constants'; | ||||||||||||
|
|
||||||||||||
| export type Message = { | ||||||||||||
| id?: number; | ||||||||||||
| messageApiId?: number; | ||||||||||||
| content: string; | ||||||||||||
| role: 'user' | 'assistant'; | ||||||||||||
| chatApiId?: string; | ||||||||||||
| blocks?: { | ||||||||||||
| cliOutput?: string; | ||||||||||||
| cliStatus?: 'success' | 'error'; | ||||||||||||
| cliTime?: string; | ||||||||||||
| codeBlockContent?: string; | ||||||||||||
| }[]; | ||||||||||||
| createdAt: number; | ||||||||||||
| failedMessage?: boolean; | ||||||||||||
| feedbackReceived?: boolean; | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| const parseWpCliOutput = ( stdout: string ): string[] => { | ||||||||||||
| try { | ||||||||||||
| const data = JSON.parse( stdout ); | ||||||||||||
| return data?.map( ( item: { name: string } ) => item.name ) || []; | ||||||||||||
| } catch ( error ) { | ||||||||||||
| Sentry.captureException( error, { extra: { stdout } } ); | ||||||||||||
| return []; | ||||||||||||
| } | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| async function fetchPluginList( siteId: string ): Promise< string[] > { | ||||||||||||
| const { stdout, stderr } = await getIpcApi().executeWPCLiInline( { | ||||||||||||
| siteId, | ||||||||||||
| args: 'plugin list --format=json --status=active', | ||||||||||||
| skipPluginsAndThemes: true, | ||||||||||||
| } ); | ||||||||||||
|
|
||||||||||||
| return stderr ? [] : parseWpCliOutput( stdout ); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| async function fetchThemeList( siteId: string ): Promise< string[] > { | ||||||||||||
| const { stdout, stderr } = await getIpcApi().executeWPCLiInline( { | ||||||||||||
| siteId, | ||||||||||||
| args: 'theme list --format=json', | ||||||||||||
| skipPluginsAndThemes: true, | ||||||||||||
| } ); | ||||||||||||
|
|
||||||||||||
| return stderr ? [] : parseWpCliOutput( stdout ); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type UpdateFromSiteParams = { | ||||||||||||
| site: SiteDetails; | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| export const updateFromSiteThunk = createAsyncThunk( | ||||||||||||
| 'chat/updateFromSite', | ||||||||||||
| async ( { site }: UpdateFromSiteParams ) => { | ||||||||||||
| const [ plugins, themes ] = await Promise.all( [ | ||||||||||||
| fetchPluginList( site.id ), | ||||||||||||
| fetchThemeList( site.id ), | ||||||||||||
| ] ); | ||||||||||||
|
|
||||||||||||
| return { | ||||||||||||
| plugins, | ||||||||||||
| themes, | ||||||||||||
| }; | ||||||||||||
| } | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| type FetchAssistantParams = { | ||||||||||||
| client: WPCOM; | ||||||||||||
| instanceId: string; | ||||||||||||
| isRetry?: boolean; | ||||||||||||
| message: Message; | ||||||||||||
| siteId: string; | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| type FetchAssistantResponseData = { | ||||||||||||
| choices: { message: { content: string; id: number } }[]; | ||||||||||||
| id: string; | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| export const fetchAssistantThunk = createAsyncThunk( | ||||||||||||
| 'chat/fetchAssistant', | ||||||||||||
| async ( { client, instanceId, siteId }: FetchAssistantParams, thunkAPI ) => { | ||||||||||||
| const state = thunkAPI.getState() as RootState; | ||||||||||||
| const context = { | ||||||||||||
| current_url: state.chat.currentURL, | ||||||||||||
| number_of_sites: state.chat.numberOfSites, | ||||||||||||
| wp_version: state.chat.wpVersion, | ||||||||||||
| php_version: state.chat.phpVersion, | ||||||||||||
| plugins: state.chat.pluginListDict[ siteId ] || [], | ||||||||||||
| themes: state.chat.themeListDict[ siteId ] || [], | ||||||||||||
| current_theme: state.chat.themeName, | ||||||||||||
| is_block_theme: state.chat.isBlockTheme, | ||||||||||||
| ide: state.chat.availableEditors, | ||||||||||||
| site_name: state.chat.siteName, | ||||||||||||
| os: state.chat.os, | ||||||||||||
| }; | ||||||||||||
| const messages = state.chat.messagesDict[ instanceId ]; | ||||||||||||
| const chatApiId = state.chat.chatApiIdDict[ instanceId ]; | ||||||||||||
|
|
||||||||||||
| const { data, headers } = await new Promise< { | ||||||||||||
| data: FetchAssistantResponseData; | ||||||||||||
| headers: Record< string, string >; | ||||||||||||
| } >( ( resolve, reject ) => { | ||||||||||||
| client.req.post< FetchAssistantResponseData >( | ||||||||||||
| { | ||||||||||||
| path: '/studio-app/ai-assistant/chat', | ||||||||||||
| apiNamespace: 'wpcom/v2', | ||||||||||||
| body: { | ||||||||||||
| messages, | ||||||||||||
| chat_id: chatApiId, | ||||||||||||
| context, | ||||||||||||
| }, | ||||||||||||
| }, | ||||||||||||
| ( error, data, headers ) => { | ||||||||||||
| if ( error ) { | ||||||||||||
| return reject( error ); | ||||||||||||
| } | ||||||||||||
| return resolve( { data, headers } ); | ||||||||||||
| } | ||||||||||||
| ); | ||||||||||||
| } ); | ||||||||||||
|
|
||||||||||||
| return { | ||||||||||||
| chatApiId: data?.id, | ||||||||||||
| maxQuota: headers[ 'x-quota-max' ] || '', | ||||||||||||
| message: data?.choices?.[ 0 ]?.message?.content, | ||||||||||||
| messageApiId: data?.choices?.[ 0 ]?.message?.id, | ||||||||||||
| remainingQuota: headers[ 'x-quota-remaining' ] || '', | ||||||||||||
| }; | ||||||||||||
| } | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| type SendFeedbackParams = { | ||||||||||||
| client: WPCOM; | ||||||||||||
| instanceId: string; | ||||||||||||
| messageApiId: number; | ||||||||||||
| ratingValue: number; | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| export const sendFeedbackThunk = createAsyncThunk( | ||||||||||||
| 'chat/sendFeedback', | ||||||||||||
| async ( { client, messageApiId, ratingValue, instanceId }: SendFeedbackParams, thunkAPI ) => { | ||||||||||||
| const state = thunkAPI.getState() as RootState; | ||||||||||||
| const chatApiId = state.chat.chatApiIdDict[ instanceId ]; | ||||||||||||
|
|
||||||||||||
| try { | ||||||||||||
| await client.req.post( { | ||||||||||||
| path: `/odie/chat/wpcom-studio-chat/${ chatApiId }/${ messageApiId }/feedback`, | ||||||||||||
| apiNamespace: 'wpcom/v2', | ||||||||||||
| body: { | ||||||||||||
| rating_value: ratingValue, | ||||||||||||
| }, | ||||||||||||
| } ); | ||||||||||||
| } catch ( error ) { | ||||||||||||
| Sentry.captureException( error ); | ||||||||||||
| console.error( error ); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| const storedMessages = localStorage.getItem( CHAT_MESSAGES_STORE_KEY ); | ||||||||||||
| const storedChatIds = localStorage.getItem( CHAT_ID_STORE_KEY ); | ||||||||||||
| const EMPTY_MESSAGES: readonly Message[] = Object.freeze( [] ); | ||||||||||||
|
|
||||||||||||
| export interface ChatState { | ||||||||||||
| currentURL: string; | ||||||||||||
| pluginListDict: Record< string, string[] >; | ||||||||||||
| themeListDict: Record< string, string[] >; | ||||||||||||
| numberOfSites: number; | ||||||||||||
| phpVersion: string; | ||||||||||||
| siteName: string; | ||||||||||||
| themeName: string; | ||||||||||||
| isBlockTheme: boolean; | ||||||||||||
| os: string; | ||||||||||||
| availableEditors: string[]; | ||||||||||||
| wpVersion: string; | ||||||||||||
| messagesDict: { [ key: string ]: Message[] }; | ||||||||||||
| chatApiIdDict: { [ key: string ]: string | undefined }; | ||||||||||||
| chatInputBySite: { [ key: string ]: string }; | ||||||||||||
| isLoadingDict: Record< string, boolean >; | ||||||||||||
| promptUsageDict: Record< string, { maxQuota: string; remainingQuota: string } >; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const initialState: ChatState = { | ||||||||||||
| currentURL: '', | ||||||||||||
| pluginListDict: {}, | ||||||||||||
| themeListDict: {}, | ||||||||||||
| numberOfSites: 0, | ||||||||||||
| themeName: '', | ||||||||||||
| wpVersion: '', | ||||||||||||
| phpVersion: DEFAULT_PHP_VERSION, | ||||||||||||
| isBlockTheme: false, | ||||||||||||
| os: window.appGlobals?.platform || '', | ||||||||||||
| availableEditors: [], | ||||||||||||
| siteName: '', | ||||||||||||
| messagesDict: storedMessages ? JSON.parse( storedMessages ) : {}, | ||||||||||||
| chatApiIdDict: storedChatIds ? JSON.parse( storedChatIds ) : {}, | ||||||||||||
| chatInputBySite: {}, | ||||||||||||
| isLoadingDict: {}, | ||||||||||||
| promptUsageDict: {}, | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| export function generateMessage( | ||||||||||||
| content: string, | ||||||||||||
| role: 'user' | 'assistant', | ||||||||||||
| newMessageId: number, | ||||||||||||
| chatApiId?: string, | ||||||||||||
| messageApiId?: number | ||||||||||||
| ): Message { | ||||||||||||
| return { | ||||||||||||
| content, | ||||||||||||
| role, | ||||||||||||
| id: newMessageId, | ||||||||||||
| chatApiId, | ||||||||||||
| createdAt: Date.now(), | ||||||||||||
| feedbackReceived: false, | ||||||||||||
| messageApiId, | ||||||||||||
| }; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const chatSlice = createSlice( { | ||||||||||||
| name: 'chat', | ||||||||||||
| initialState, | ||||||||||||
| reducers: { | ||||||||||||
| resetChatState: ( state ) => { | ||||||||||||
| // Reassigning `initialState` to `state` doesn't work. Probably because of Immer.js. | ||||||||||||
| Object.assign( state, initialState ); | ||||||||||||
|
There was a problem hiding this comment. Also, I am thinking - initial state stores initial values of There was a problem hiding this comment. Hm, more thoughts - looks like you went another approach and we can remove this action: There was a problem hiding this comment. Yeah, There was a problem hiding this comment. The last one thing is I wanted gained to comeback this this specific message #826 (comment) It's cool that we added comment, makes sense, but theoretically, somebody can use it in code in the future and I have assumption that it can be buggy. For the initialState we retrieve data from localStorage during starting Studio: Then we store it inside Maybe I wasn't enough clear, so I will provide the code, which I think will be selfexplanation of m concern: There was a problem hiding this comment. And in general, as for me, it's very bad practice to store something like this action in code, only for test purposes. I propose either to find some way to patch store in test to add this method or at least to add There was a problem hiding this comment. Point taken! 👍 Given that There was a problem hiding this comment.
I get your point. Let me think on this and see if I can come up with a better approach There was a problem hiding this comment. I found this: There was a problem hiding this comment. Maybe we can use it for our case There was a problem hiding this comment. I moved the reset state action to a test util file per your suggestion in this thread, @nightnei. Great suggestion 👍 Much cleaner this way.
|
||||||||||||
| }, | ||||||||||||
|
There was a problem hiding this comment. Do you mean Anyay, I found more right approach https://stackoverflow.com/questions/59424523/reset-state-to-initial-with-redux-toolkit
Suggested change
There was a problem hiding this comment. Returning |
||||||||||||
| updateFromTheme: ( | ||||||||||||
| state, | ||||||||||||
| action: PayloadAction< NonNullable< SiteDetails[ 'themeDetails' ] > > | ||||||||||||
| ) => { | ||||||||||||
| state.themeName = action.payload.name; | ||||||||||||
| state.isBlockTheme = action.payload.isBlockTheme; | ||||||||||||
| }, | ||||||||||||
| setMessages: ( | ||||||||||||
| state, | ||||||||||||
| action: PayloadAction< { instanceId: string; messages: Message[] } > | ||||||||||||
| ) => { | ||||||||||||
| const { instanceId, messages } = action.payload; | ||||||||||||
| state.messagesDict[ instanceId ] = messages; | ||||||||||||
| }, | ||||||||||||
| setChatInput: ( state, action: PayloadAction< { siteId: string; input: string } > ) => { | ||||||||||||
| const { siteId, input } = action.payload; | ||||||||||||
| state.chatInputBySite[ siteId ] = input; | ||||||||||||
| }, | ||||||||||||
| updateMessage: ( | ||||||||||||
| state, | ||||||||||||
| action: PayloadAction< { | ||||||||||||
| cliOutput?: string; | ||||||||||||
| cliStatus?: 'success' | 'error'; | ||||||||||||
| cliTime?: string; | ||||||||||||
| codeBlockContent: string; | ||||||||||||
| messageId: number; | ||||||||||||
| siteId: string; | ||||||||||||
| } > | ||||||||||||
| ) => { | ||||||||||||
| const { cliOutput, cliStatus, cliTime, codeBlockContent, messageId, siteId } = action.payload; | ||||||||||||
|
|
||||||||||||
| if ( ! state.messagesDict[ siteId ] ) { | ||||||||||||
| state.messagesDict[ siteId ] = []; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| state.messagesDict[ siteId ].forEach( ( message ) => { | ||||||||||||
| if ( message.id === messageId ) { | ||||||||||||
| message.blocks?.forEach( ( block ) => { | ||||||||||||
| if ( block.codeBlockContent === codeBlockContent ) { | ||||||||||||
| block.cliOutput = cliOutput; | ||||||||||||
| block.cliStatus = cliStatus; | ||||||||||||
| block.cliTime = cliTime; | ||||||||||||
| } | ||||||||||||
| } ); | ||||||||||||
| } | ||||||||||||
| } ); | ||||||||||||
| }, | ||||||||||||
| }, | ||||||||||||
| extraReducers: ( builder ) => { | ||||||||||||
| builder | ||||||||||||
| .addCase( updateFromSiteThunk.pending, ( state, action ) => { | ||||||||||||
| const { site } = action.meta.arg; | ||||||||||||
|
|
||||||||||||
| state.currentURL = `http://localhost:${ site.port }`; | ||||||||||||
| state.phpVersion = site.phpVersion ?? DEFAULT_PHP_VERSION; | ||||||||||||
| state.siteName = site.name; | ||||||||||||
| } ) | ||||||||||||
|
There was a problem hiding this comment. I was looking at code and looks like there is no sense to create separate case for There was a problem hiding this comment. Even if it takes a very short time to resolve There was a problem hiding this comment.
But we don't have rejected handler for this case. There was a problem hiding this comment.
Then we'd need to add one. Duplicating the logic (or abstracting it) is not very appealing.
Only if a user manages to send a message before |
||||||||||||
| .addCase( updateFromSiteThunk.fulfilled, ( state, action ) => { | ||||||||||||
| const { plugins, themes } = action.payload; | ||||||||||||
| const siteId = action.meta.arg.site.id; | ||||||||||||
|
|
||||||||||||
| state.pluginListDict[ siteId ] = plugins; | ||||||||||||
| state.themeListDict[ siteId ] = themes; | ||||||||||||
| } ) | ||||||||||||
| .addCase( fetchAssistantThunk.pending, ( state, action ) => { | ||||||||||||
| const { message, instanceId, isRetry } = action.meta.arg; | ||||||||||||
|
|
||||||||||||
| state.isLoadingDict[ instanceId ] = true; | ||||||||||||
|
|
||||||||||||
| if ( ! state.messagesDict[ instanceId ] ) { | ||||||||||||
| state.messagesDict[ instanceId ] = []; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if ( ! isRetry ) { | ||||||||||||
| state.messagesDict[ instanceId ].push( message ); | ||||||||||||
| } else { | ||||||||||||
| state.messagesDict[ instanceId ].forEach( ( msg ) => { | ||||||||||||
| if ( msg.id === message.id ) { | ||||||||||||
| msg.failedMessage = false; | ||||||||||||
| } | ||||||||||||
| } ); | ||||||||||||
| } | ||||||||||||
| } ) | ||||||||||||
| .addCase( fetchAssistantThunk.rejected, ( state, action ) => { | ||||||||||||
| const { message, instanceId } = action.meta.arg; | ||||||||||||
|
|
||||||||||||
| state.isLoadingDict[ instanceId ] = false; | ||||||||||||
|
|
||||||||||||
| state.messagesDict[ instanceId ].forEach( ( msg ) => { | ||||||||||||
| if ( msg.id === message.id ) { | ||||||||||||
| msg.failedMessage = true; | ||||||||||||
| } | ||||||||||||
| } ); | ||||||||||||
| } ) | ||||||||||||
| .addCase( fetchAssistantThunk.fulfilled, ( state, action ) => { | ||||||||||||
| const { instanceId } = action.meta.arg; | ||||||||||||
|
|
||||||||||||
| state.isLoadingDict[ instanceId ] = false; | ||||||||||||
|
|
||||||||||||
| const message = generateMessage( | ||||||||||||
| action.payload.message, | ||||||||||||
| 'assistant', | ||||||||||||
| state.messagesDict[ instanceId ].length, | ||||||||||||
| action.payload.chatApiId, | ||||||||||||
| action.payload.messageApiId | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| state.messagesDict[ instanceId ].push( message ); | ||||||||||||
|
|
||||||||||||
| if ( message.chatApiId ) { | ||||||||||||
| state.chatApiIdDict[ instanceId ] = message.chatApiId; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| state.promptUsageDict[ instanceId ] = { | ||||||||||||
| maxQuota: action.payload.maxQuota, | ||||||||||||
| remainingQuota: action.payload.remainingQuota, | ||||||||||||
| }; | ||||||||||||
| } ) | ||||||||||||
| .addCase( sendFeedbackThunk.pending, ( state, action ) => { | ||||||||||||
| const { instanceId, messageApiId } = action.meta.arg; | ||||||||||||
|
|
||||||||||||
| if ( ! state.messagesDict[ instanceId ] ) { | ||||||||||||
| state.messagesDict[ instanceId ] = []; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| state.messagesDict[ instanceId ].forEach( ( message ) => { | ||||||||||||
| if ( message.messageApiId === messageApiId ) { | ||||||||||||
| message.feedbackReceived = true; | ||||||||||||
| } | ||||||||||||
| } ); | ||||||||||||
| } ); | ||||||||||||
| }, | ||||||||||||
| selectors: { | ||||||||||||
| selectChatInput: ( state, siteId: string ) => state.chatInputBySite[ siteId ] ?? '', | ||||||||||||
| selectMessages: ( state, instanceId: string ) => | ||||||||||||
| state.messagesDict[ instanceId ] ?? EMPTY_MESSAGES, | ||||||||||||
| selectChatApiId: ( state, instanceId: string ) => state.chatApiIdDict[ instanceId ], | ||||||||||||
| selectIsLoading: ( state, instanceId: string ) => state.isLoadingDict[ instanceId ] ?? false, | ||||||||||||
| }, | ||||||||||||
| } ); | ||||||||||||
|
|
||||||||||||
| export const { updateFromTheme, setMessages, setChatInput, updateMessage, resetChatState } = | ||||||||||||
| chatSlice.actions; | ||||||||||||
|
|
||||||||||||
| export const { selectChatInput, selectMessages, selectChatApiId, selectIsLoading } = | ||||||||||||
| chatSlice.selectors; | ||||||||||||
|
There was a problem hiding this comment. WDYT about exporting There was a problem hiding this comment. Good idea – I like it 👍 Will make even more sense once we have more stores. |
||||||||||||
|
|
||||||||||||
| export default chatSlice.reducer; | ||||||||||||
|
There was a problem hiding this comment. I would avoid default exports, as mentioned it above |
||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||
| import { configureStore, createListenerMiddleware } from '@reduxjs/toolkit'; | ||||||
| import { CHAT_ID_STORE_KEY, CHAT_MESSAGES_STORE_KEY } from 'src/constants'; | ||||||
|
There was a problem hiding this comment. WDYT about adding prefix for such constants as
Suggested change
There was a problem hiding this comment. Good point, makes sense 👍 |
||||||
| import chatReducer from 'src/stores/chat-slice'; | ||||||
|
|
||||||
| export type RootState = { | ||||||
|
There was a problem hiding this comment. I am thinking, what about creating |
||||||
| chat: ReturnType< typeof chatReducer >; | ||||||
| }; | ||||||
|
|
||||||
| const listenerMiddleware = createListenerMiddleware< RootState >(); | ||||||
|
|
||||||
| listenerMiddleware.startListening( { | ||||||
|
There was a problem hiding this comment. Not critical, more just thinking out loud - should we unsubscribe during logout? There was a problem hiding this comment. I think this would be easier to accomplish if the auth data was also stored in Redux. Given that it's not necessary, I would defer this until later. |
||||||
| predicate( action, currentState, previousState ) { | ||||||
| return currentState.chat.messagesDict !== previousState.chat.messagesDict; | ||||||
| }, | ||||||
| effect( action, listenerApi ) { | ||||||
| const state = listenerApi.getState() as RootState; | ||||||
|
There was a problem hiding this comment. It's redundant, sicne you type middleware here
Suggested change
|
||||||
| localStorage.setItem( CHAT_MESSAGES_STORE_KEY, JSON.stringify( state.chat.messagesDict ) ); | ||||||
| }, | ||||||
| } ); | ||||||
|
There was a problem hiding this comment. I want to highlight this piece in particular because it illustrates how we could use Redux logic to more effectively update We are subscribing to state changes here, with the |
||||||
|
|
||||||
| listenerMiddleware.startListening( { | ||||||
|
There was a problem hiding this comment. Looking at a glance - it's dificult to understand what specific listener do. They look similar and when we have much more listeners - it will be difficult to look up specific one. Maybe let's add some small comment to every listener? WDYT? There was a problem hiding this comment. Makes sense 👍 |
||||||
| predicate( action, currentState, previousState ) { | ||||||
| return currentState.chat.chatApiIdDict !== previousState.chat.chatApiIdDict; | ||||||
| }, | ||||||
| effect( action, listenerApi ) { | ||||||
| const state = listenerApi.getState() as RootState; | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| localStorage.setItem( CHAT_ID_STORE_KEY, JSON.stringify( state.chat.chatApiIdDict ) ); | ||||||
| }, | ||||||
| } ); | ||||||
|
|
||||||
| const store = configureStore( { | ||||||
| reducer: { | ||||||
| chat: chatReducer, | ||||||
| }, | ||||||
| middleware: ( getDefaultMiddleware ) => | ||||||
| getDefaultMiddleware().prepend( listenerMiddleware.middleware ), | ||||||
| } ); | ||||||
|
|
||||||
| export default store; | ||||||
|
There was a problem hiding this comment. WDYT about avoiding default exports? There was a problem hiding this comment. Sure, makes sense 👍 |
||||||
|
|
||||||
| export type AppDispatch = typeof store.dispatch; | ||||||
|
There was a problem hiding this comment. We already started copy-pasting and importing it everywhere. WDYT about creating |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,260 @@ | ||
| import WPCOM from 'wpcom'; | ||
| import { CHAT_ID_STORE_KEY, CHAT_MESSAGES_STORE_KEY } from 'src/constants'; | ||
| import { getIpcApi } from 'src/lib/get-ipc-api'; | ||
| import store from 'src/stores'; | ||
| import { | ||
| fetchAssistantThunk, | ||
| generateMessage, | ||
| resetChatState, | ||
| sendFeedbackThunk, | ||
| setMessages, | ||
| updateFromSiteThunk, | ||
| } from 'src/stores/chat-slice'; | ||
|
|
||
| jest.mock( 'src/lib/get-ipc-api' ); | ||
|
|
||
| const mockClientReqPostUsingCallback = jest.fn().mockImplementation( ( params, callback ) => { | ||
| callback( | ||
| null, | ||
| { | ||
| id: 'chatcmpl-123', | ||
| choices: [ | ||
| { | ||
| message: { | ||
| id: 42, | ||
| content: 'Test assistant response', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| 'x-quota-max': '100', | ||
| 'x-quota-remaining': '99', | ||
| } | ||
| ); | ||
| } ); | ||
|
|
||
| const mockClientUsingCallback = { | ||
| req: { post: mockClientReqPostUsingCallback }, | ||
| } as unknown as WPCOM; | ||
|
|
||
| const mockClientReqPostUsingPromise = jest.fn().mockResolvedValue( { | ||
| data: 'success', | ||
| } ); | ||
|
|
||
| const mockClientUsingPromise = { | ||
| req: { post: mockClientReqPostUsingPromise }, | ||
| } as unknown as WPCOM; | ||
|
|
||
| describe( 'chat-slice', () => { | ||
| beforeEach( () => { | ||
| jest.clearAllMocks(); | ||
| localStorage.clear(); | ||
| store.dispatch( resetChatState() ); | ||
| } ); | ||
|
|
||
| describe( 'fetchAssistantThunk', () => { | ||
| it( 'should add assistant message to state when fulfilled', async () => { | ||
| const instanceId = 'test-site'; | ||
| const userMessage = generateMessage( 'Hello test 1', 'user', 0, 'chatcmpl-123', 42 ); | ||
|
|
||
| const result = await store.dispatch( | ||
| fetchAssistantThunk( { | ||
| client: mockClientUsingCallback, | ||
| instanceId, | ||
| message: userMessage, | ||
| siteId: instanceId, | ||
| } ) | ||
| ); | ||
|
|
||
| expect( result.type ).toBe( 'chat/fetchAssistant/fulfilled' ); | ||
| expect( result.payload ).toEqual( { | ||
| chatApiId: 'chatcmpl-123', | ||
| maxQuota: '100', | ||
| message: 'Test assistant response', | ||
| messageApiId: 42, | ||
| remainingQuota: '99', | ||
| } ); | ||
|
|
||
| const state = store.getState(); | ||
| const messages = state.chat.messagesDict[ instanceId ]; | ||
|
|
||
| expect( messages ).toHaveLength( 2 ); | ||
| expect( messages[ 0 ] ).toEqual( userMessage ); | ||
| expect( messages[ 1 ] ).toMatchObject( { | ||
| content: 'Test assistant response', | ||
| role: 'assistant', | ||
| chatApiId: 'chatcmpl-123', | ||
| messageApiId: 42, | ||
| } ); | ||
|
|
||
| expect( state.chat.promptUsageDict[ instanceId ] ).toEqual( { | ||
| maxQuota: '100', | ||
| remainingQuota: '99', | ||
| } ); | ||
| } ); | ||
|
|
||
| it( 'should mark message as failed when rejected', async () => { | ||
| const instanceId = 'test-site'; | ||
| const userMessage = generateMessage( 'Hello test 2', 'user', 0, 'chatcmpl-123', 42 ); | ||
|
|
||
| mockClientReqPostUsingCallback.mockImplementationOnce( ( params, callback ) => { | ||
| callback( new Error( 'API Error' ), null, {} ); | ||
| } ); | ||
|
|
||
| const result = await store.dispatch( | ||
| fetchAssistantThunk( { | ||
| client: mockClientUsingCallback, | ||
| instanceId, | ||
| message: userMessage, | ||
| siteId: instanceId, | ||
| } ) | ||
| ); | ||
|
|
||
| expect( result.type ).toBe( 'chat/fetchAssistant/rejected' ); | ||
|
|
||
| const state = store.getState(); | ||
| const messages = state.chat.messagesDict[ instanceId ]; | ||
|
|
||
| expect( messages ).toHaveLength( 1 ); | ||
| expect( messages[ 0 ] ).toMatchObject( { | ||
| ...userMessage, | ||
| failedMessage: true, | ||
| } ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'sendFeedbackThunk', () => { | ||
| it( 'should mark message as feedback received', async () => { | ||
| const instanceId = 'test-site'; | ||
|
|
||
| const userMessage = generateMessage( 'Hello test 3', 'user', 0, 'chatcmpl-123', 42 ); | ||
| const assistantMessage = generateMessage( 'Response', 'assistant', 1, 'chatcmpl-123', 43 ); | ||
| store.dispatch( setMessages( { instanceId, messages: [ userMessage, assistantMessage ] } ) ); | ||
|
|
||
| const result = await store.dispatch( | ||
| sendFeedbackThunk( { | ||
| client: mockClientUsingPromise, | ||
| instanceId, | ||
| messageApiId: 42, | ||
| ratingValue: 1, | ||
| } ) | ||
| ); | ||
|
|
||
| expect( result.type ).toBe( 'chat/sendFeedback/fulfilled' ); | ||
|
|
||
| const state = store.getState(); | ||
| console.log( 'state', state ); | ||
| const messages = state.chat.messagesDict[ instanceId ]; | ||
|
|
||
| expect( messages[ 0 ].feedbackReceived ).toBe( true ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'localStorage persistence', () => { | ||
| it( 'should persist messagesDict and chatApiIdDict changes to localStorage', async () => { | ||
| const instanceId = 'test-site'; | ||
| const userMessage = generateMessage( 'Hello test 4', 'user', 0, 'chatcmpl-123', 42 ); | ||
|
|
||
| await store.dispatch( | ||
| fetchAssistantThunk( { | ||
| client: mockClientUsingCallback, | ||
| instanceId, | ||
| message: userMessage, | ||
| siteId: instanceId, | ||
| } ) | ||
| ); | ||
|
|
||
| const storedMessages = JSON.parse( localStorage.getItem( CHAT_MESSAGES_STORE_KEY ) || '{}' ); | ||
| expect( storedMessages[ instanceId ] ).toHaveLength( 2 ); | ||
| expect( storedMessages[ instanceId ][ 0 ] ).toEqual( userMessage ); | ||
| expect( storedMessages[ instanceId ][ 1 ] ).toMatchObject( { | ||
| content: 'Test assistant response', | ||
| role: 'assistant', | ||
| } ); | ||
|
|
||
| const storedChatIds = JSON.parse( localStorage.getItem( CHAT_ID_STORE_KEY ) || '{}' ); | ||
| expect( storedChatIds[ instanceId ] ).toBe( 'chatcmpl-123' ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'updateFromSiteThunk', () => { | ||
| const mockSite: SiteDetails = { | ||
| id: 'test-site', | ||
| name: 'Test Site', | ||
| port: 8881, | ||
| phpVersion: '8.0', | ||
| path: '/test/path', | ||
| running: true, | ||
| url: 'http://localhost:8881', | ||
| }; | ||
|
|
||
| beforeEach( () => { | ||
| ( getIpcApi as jest.Mock ).mockReturnValue( { | ||
| executeWPCLiInline: jest.fn().mockResolvedValue( { | ||
| stdout: JSON.stringify( [ { name: 'woocommerce' }, { name: 'jetpack' } ] ), | ||
| stderr: '', | ||
| } ), | ||
| } ); | ||
| } ); | ||
|
|
||
| it( 'should update plugin and theme lists when WP CLI succeeds', async () => { | ||
| const result = await store.dispatch( updateFromSiteThunk( { site: mockSite } ) ); | ||
|
|
||
| expect( result.type ).toBe( 'chat/updateFromSite/fulfilled' ); | ||
| expect( result.payload ).toEqual( { | ||
| plugins: [ 'woocommerce', 'jetpack' ], | ||
| themes: [ 'woocommerce', 'jetpack' ], | ||
| } ); | ||
|
|
||
| const state = store.getState(); | ||
| expect( state.chat.pluginListDict[ mockSite.id ] ).toEqual( [ 'woocommerce', 'jetpack' ] ); | ||
| expect( state.chat.themeListDict[ mockSite.id ] ).toEqual( [ 'woocommerce', 'jetpack' ] ); | ||
| expect( state.chat.currentURL ).toBe( 'http://localhost:8881' ); | ||
| expect( state.chat.phpVersion ).toBe( '8.0' ); | ||
| expect( state.chat.siteName ).toBe( 'Test Site' ); | ||
| } ); | ||
|
|
||
| it( 'should handle WP CLI errors gracefully', async () => { | ||
| ( getIpcApi as jest.Mock ).mockReturnValue( { | ||
| executeWPCLiInline: jest.fn().mockResolvedValue( { | ||
| stdout: '', | ||
| stderr: 'Error: WP CLI failed', | ||
| } ), | ||
| } ); | ||
|
|
||
| const result = await store.dispatch( updateFromSiteThunk( { site: mockSite } ) ); | ||
|
|
||
| expect( result.type ).toBe( 'chat/updateFromSite/fulfilled' ); | ||
| expect( result.payload ).toEqual( { | ||
| plugins: [], | ||
| themes: [], | ||
| } ); | ||
|
|
||
| const state = store.getState(); | ||
| expect( state.chat.pluginListDict[ mockSite.id ] ).toEqual( [] ); | ||
| expect( state.chat.themeListDict[ mockSite.id ] ).toEqual( [] ); | ||
| } ); | ||
|
|
||
| it( 'should handle JSON parsing errors gracefully', async () => { | ||
| ( getIpcApi as jest.Mock ).mockReturnValue( { | ||
| executeWPCLiInline: jest.fn().mockResolvedValue( { | ||
| stdout: 'Invalid JSON', | ||
| stderr: '', | ||
| } ), | ||
| } ); | ||
|
|
||
| const result = await store.dispatch( updateFromSiteThunk( { site: mockSite } ) ); | ||
|
|
||
| expect( result.type ).toBe( 'chat/updateFromSite/fulfilled' ); | ||
| expect( result.payload ).toEqual( { | ||
| plugins: [], | ||
| themes: [], | ||
| } ); | ||
|
|
||
| const state = store.getState(); | ||
| expect( state.chat.pluginListDict[ mockSite.id ] ).toEqual( [] ); | ||
| expect( state.chat.themeListDict[ mockSite.id ] ).toEqual( [] ); | ||
| } ); | ||
| } ); | ||
| } ); |
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.
A new version 2.5.1 was released 3 days ago. We could update it.