From 8ac4e6ffd1e0f41c6cbd9fe15472224bfc6038f3 Mon Sep 17 00:00:00 2001 From: Teresa Hoang <125500434+teresaqhoang@users.noreply.github.com> Date: Mon, 10 Apr 2023 09:36:49 -0700 Subject: [PATCH] Simplifying state + removing unused components (#371) ### Motivation and Context Removing unused parts of app, including chatSlice from redux and filter icon in ChatList ### Description Consolidated ChatState into ConversationsState, will manage state from conversations array. Commenting out Filter icon under "Conversations" list until we're ready to implement Changed login error to be more helpful to error cases we've observed --- samples/apps/copilot-chat-app/README.md | 2 +- .../apps/copilot-chat-app/WebApp/README.md | 4 +- .../apps/copilot-chat-app/WebApp/env.example | 2 +- .../apps/copilot-chat-app/WebApp/src/App.tsx | 19 ++-- .../copilot-chat-app/WebApp/src/Constants.ts | 2 +- .../WebApp/src/components/Login.tsx | 15 ++-- .../src/components/chat/ChatHistoryItem.tsx | 12 +-- .../WebApp/src/components/chat/ChatRoom.tsx | 10 +-- .../WebApp/src/components/chat/ChatStatus.tsx | 5 +- .../components/chat/chat-list/ChatList.tsx | 90 ++++++++++--------- .../WebApp/src/libs/useChat.ts | 34 +++---- .../WebApp/src/redux/app/store.ts | 4 +- .../WebApp/src/redux/features/app/AppState.ts | 2 - .../WebApp/src/redux/features/app/appSlice.ts | 8 +- .../src/redux/features/chat/chatSlice.ts | 49 ---------- .../{chat => conversations}/ChatState.ts | 0 .../conversations/ConversationsState.ts | 26 +++--- .../conversations/conversationsSlice.ts | 20 +++-- 18 files changed, 128 insertions(+), 176 deletions(-) delete mode 100644 samples/apps/copilot-chat-app/WebApp/src/redux/features/chat/chatSlice.ts rename samples/apps/copilot-chat-app/WebApp/src/redux/features/{chat => conversations}/ChatState.ts (100%) diff --git a/samples/apps/copilot-chat-app/README.md b/samples/apps/copilot-chat-app/README.md index 06983324b722..1928666e2e64 100644 --- a/samples/apps/copilot-chat-app/README.md +++ b/samples/apps/copilot-chat-app/README.md @@ -94,7 +94,7 @@ and these components are functional: 2. Copy `.env.example` into a new file with the name “`.env`” and make the following configuration changes to match your instance: 3. Use the Application (client) ID from the Azure Portal steps above and - paste the GUID into the `.env` file next to `REACT_APP_CHAT_CLIENT_ID= ` + paste the GUID into the `.env` file next to `REACT_APP_AAD_CLIENT_ID= ` 4. Execute the command `yarn install` 5. Execute the command `yarn start` diff --git a/samples/apps/copilot-chat-app/WebApp/README.md b/samples/apps/copilot-chat-app/WebApp/README.md index 780871752bf4..fd37d251e3ec 100644 --- a/samples/apps/copilot-chat-app/WebApp/README.md +++ b/samples/apps/copilot-chat-app/WebApp/README.md @@ -26,12 +26,12 @@ The Copilot Chat sameple showcases how to build an enriched intelligent app, wit 1. Ensure the SKWebApi is running at `https://localhost:40443/`. See [SKWebApi README](../SKWebApi/README.md) for instructions. 2. Create an `.env` file to this folder root with the following variables and fill in with your information, where - `REACT_APP_CHAT_CLIENT_ID=` is the GUID copied from the **Application (client) ID** from your app registration in the Azure Portal and + `REACT_APP_AAD_CLIENT_ID=` is the GUID copied from the **Application (client) ID** from your app registration in the Azure Portal and `REACT_APP_BACKEND_URI=` is the URI where your backend is running. ``` REACT_APP_BACKEND_URI=https://localhost:40443/ - REACT_APP_CHAT_CLIENT_ID= + REACT_APP_AAD_CLIENT_ID= ``` 3. **Run** the following command `yarn install` (if you have never run the app before) and/or `yarn start` from the command line. diff --git a/samples/apps/copilot-chat-app/WebApp/env.example b/samples/apps/copilot-chat-app/WebApp/env.example index 437ecce2f32e..78c20e110024 100644 --- a/samples/apps/copilot-chat-app/WebApp/env.example +++ b/samples/apps/copilot-chat-app/WebApp/env.example @@ -1,2 +1,2 @@ REACT_APP_BACKEND_URI=http://localhost:40443/ -REACT_APP_CHAT_CLIENT_ID= \ No newline at end of file +REACT_APP_AAD_CLIENT_ID= \ No newline at end of file diff --git a/samples/apps/copilot-chat-app/WebApp/src/App.tsx b/samples/apps/copilot-chat-app/WebApp/src/App.tsx index 2dadf7248bdf..142b5a95f5e0 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/App.tsx +++ b/samples/apps/copilot-chat-app/WebApp/src/App.tsx @@ -33,13 +33,13 @@ const useClasses = makeStyles({ justifyContent: 'space-between', }, persona: { - marginRight: '20px' - } + marginRight: '20px', + }, }); enum AppState { ProbeForBackend, - Chat + Chat, } const App: FC = () => { @@ -50,9 +50,10 @@ const App: FC = () => { const account = msalInstance.getActiveAccount(); useEffect(() => { - // TODO: Load Conversations from BE + // TODO: Load conversations from BE const keys = Object.keys(conversations); dispatch(setSelectedConversation(keys[0])); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); return ( @@ -61,15 +62,15 @@ const App: FC = () => { - {appState === AppState.ProbeForBackend && + {appState === AppState.ProbeForBackend && ( setAppState(AppState.Chat)} /> - } - {appState === AppState.Chat && + )} + {appState === AppState.Chat && (
-
+
Copilot Chat {
- } + )}
); diff --git a/samples/apps/copilot-chat-app/WebApp/src/Constants.ts b/samples/apps/copilot-chat-app/WebApp/src/Constants.ts index 8aa2f4b45334..c0d64a95742b 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/Constants.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/Constants.ts @@ -6,7 +6,7 @@ export const Constants = { msal: { method: 'redirect', // 'redirect' | 'popup' auth: { - clientId: process.env.REACT_APP_CHAT_CLIENT_ID as string, + clientId: process.env.REACT_APP_AAD_CLIENT_ID as string, authority: `https://login.microsoftonline.com/common`, }, cache: { diff --git a/samples/apps/copilot-chat-app/WebApp/src/components/Login.tsx b/samples/apps/copilot-chat-app/WebApp/src/components/Login.tsx index 9a63cc9ba4e0..fa65072e0b2a 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/components/Login.tsx +++ b/samples/apps/copilot-chat-app/WebApp/src/components/Login.tsx @@ -20,8 +20,12 @@ export const Login: React.FC = () => { const handleError = (error: any) => { console.error(error); - setErrorMessage((error as Error).message); - } + setErrorMessage( + `Login failed. Check that you have a valid REACT_APP_AAD_CLIENT_ID set in your .env file. See ${ + (error as Error).name + } in console for more details.`, + ); + }; const handleSignIn = async (): Promise => { try { @@ -38,11 +42,8 @@ export const Login: React.FC = () => { useEffect(() => { handleSignIn(); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - return ( -
- {errorMessage && {errorMessage}} -
- ); + return
{errorMessage && {errorMessage}}
; }; diff --git a/samples/apps/copilot-chat-app/WebApp/src/components/chat/ChatHistoryItem.tsx b/samples/apps/copilot-chat-app/WebApp/src/components/chat/ChatHistoryItem.tsx index c999ba9cdafa..2fac2c4f9c17 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/components/chat/ChatHistoryItem.tsx +++ b/samples/apps/copilot-chat-app/WebApp/src/components/chat/ChatHistoryItem.tsx @@ -98,9 +98,11 @@ export const ChatHistoryItem: React.FC = (props) => { } const isMe = message.sender === account?.homeAccountId; - const member = chat.getAudienceMemberForId(message.sender); - const avatar = isMe ? - member?.photo ? { image: { src: member.photo } } : undefined + const member = chat.getAudienceMemberForId(message.sender, selectedId, conversations[selectedId].audience); + const avatar = isMe + ? member?.photo + ? { image: { src: member.photo } } + : undefined : { image: { src: conversations[selectedId].botProfilePicture } }; const fullName = member?.fullName ?? message.sender; @@ -108,9 +110,7 @@ export const ChatHistoryItem: React.FC = (props) => { <>
{!isMe && } -
+
{!isMe && }
- ; -} + })} + +
+ + ); +}; diff --git a/samples/apps/copilot-chat-app/WebApp/src/libs/useChat.ts b/samples/apps/copilot-chat-app/WebApp/src/libs/useChat.ts index caf95dade283..89ca8ce20833 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/libs/useChat.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/libs/useChat.ts @@ -1,20 +1,23 @@ -import { useAccount } from "@azure/msal-react"; -import { Constants } from "../Constants"; -import { useAppDispatch, useAppSelector } from "../redux/app/hooks"; -import { RootState } from "../redux/app/store"; -import { ChatState, initialBotMessage } from "../redux/features/chat/ChatState"; -import { addConversation, setSelectedConversation, updateConversation } from "../redux/features/conversations/conversationsSlice"; -import { ChatUser } from "./models/ChatUser"; -import { useSemanticKernel } from "./semantic-kernel/useSemanticKernel"; +import { useAccount } from '@azure/msal-react'; +import { Constants } from '../Constants'; +import { useAppDispatch, useAppSelector } from '../redux/app/hooks'; +import { RootState } from '../redux/app/store'; +import { ChatState, initialBotMessage } from '../redux/features/conversations/ChatState'; +import { + addConversation, + setSelectedConversation, + updateConversation, +} from '../redux/features/conversations/conversationsSlice'; +import { ChatUser } from './models/ChatUser'; +import { useSemanticKernel } from './semantic-kernel/useSemanticKernel'; export const useChat = () => { - const { audience } = useAppSelector((state: RootState) => state.chat); const dispatch = useAppDispatch(); const account = useAccount(); const sk = useSemanticKernel(process.env.REACT_APP_BACKEND_URI as string); const { conversations } = useAppSelector((state: RootState) => state.conversations); - const botProfilePictures : string[] = [ + const botProfilePictures: string[] = [ '/assets/bot-icon-1.png', '/assets/bot-icon-2.png', '/assets/bot-icon-3.png', @@ -22,9 +25,8 @@ export const useChat = () => { '/assets/bot-icon-5.png', ]; - const getAudienceMemberForId = (id: string) => - { - if (id === 'bot') return Constants.bot.profile; + const getAudienceMemberForId = (id: string, chatId: string, audience: ChatUser[]) => { + if (id === `${chatId}-bot` || id.toLocaleLowerCase() === 'bot') return Constants.bot.profile; return audience.find((member) => member.id === id); }; @@ -47,7 +49,7 @@ export const useChat = () => { audience: [user], botTypingTimestamp: 0, botProfilePicture: botProfilePictures.at(botProfilePictureIndex) ?? '/assets/bot-icon-1.png', - } + }; dispatch(addConversation(newChat)); dispatch(setSelectedConversation(name)); return name; @@ -71,6 +73,6 @@ export const useChat = () => { return { getAudienceMemberForId, createChat, - getResponse + getResponse, }; -} \ No newline at end of file +}; diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/app/store.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/app/store.ts index e0218841c69b..e6c657905180 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/redux/app/store.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/redux/app/store.ts @@ -2,14 +2,12 @@ import { configureStore } from '@reduxjs/toolkit'; import appReducer from '../features/app/appSlice'; -import chatReducer from '../features/chat/chatSlice'; import conversationsReducer from '../features/conversations/conversationsSlice'; export const store = configureStore({ reducer: { app: appReducer, - chat: chatReducer, - conversations: conversationsReducer + conversations: conversationsReducer, }, }); diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/AppState.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/AppState.ts index c5a7c9a0c7aa..866215525542 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/AppState.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/AppState.ts @@ -6,6 +6,4 @@ export interface AppState { message: string; type: AlertType; }; - unclaimed?: boolean; - documentId?: string; } diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/appSlice.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/appSlice.ts index b597bb7ed2cf..9bdbf6be7693 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/appSlice.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/redux/features/app/appSlice.ts @@ -4,8 +4,7 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { AlertType } from '../../../libs/models/AlertType'; import { AppState } from './AppState'; -const initialState: AppState = { -}; +const initialState: AppState = {}; export const appSlice = createSlice({ name: 'app', @@ -14,12 +13,9 @@ export const appSlice = createSlice({ setAlert: (state: AppState, action: PayloadAction<{ message: string; type: AlertType }>) => { state.alert = action.payload; }, - setUnclaimed: (state: AppState, action: PayloadAction) => { - state.unclaimed = action.payload; - }, }, }); -export const { setAlert, setUnclaimed } = appSlice.actions; +export const { setAlert } = appSlice.actions; export default appSlice.reducer; diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/features/chat/chatSlice.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/features/chat/chatSlice.ts deleted file mode 100644 index fc3cfaf7d505..000000000000 --- a/samples/apps/copilot-chat-app/WebApp/src/redux/features/chat/chatSlice.ts +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { ChatMessage } from '../../../libs/models/ChatMessage'; -import { ChatUser } from '../../../libs/models/ChatUser'; -import { ChatState, initialState } from './ChatState'; - -export const chatSlice = createSlice({ - name: 'chat', - initialState, - reducers: { - setAudience: (state: ChatState, action: PayloadAction) => { - state.audience = action.payload; - }, - setChatUserTyping: (state: ChatState, action: PayloadAction<{ id: string; timestamp: number }>) => { - const user = state.audience.find((u) => u.id === action.payload.id); - if (user) { - user.lastTypingTimestamp = action.payload.timestamp; - } - }, - setBotTyping: (state: ChatState, action: PayloadAction) => { - state.botTypingTimestamp = action.payload ? Date.now() : 0; - }, - setChatMessages: (state: ChatState, action: PayloadAction) => { - state.messages = action.payload; - }, - addMessage: (state: ChatState, action: PayloadAction) => { - state.messages = [...state.messages, action.payload]; - }, - setChat: (state: ChatState, action: PayloadAction) => { - const newState = action.payload; - state.messages = newState.messages; - state.audience = newState.audience; - state.botTypingTimestamp = newState.botTypingTimestamp; - state.id = newState.id; - }, - }, -}); - -export const { - setAudience, - setChatUserTyping, - setBotTyping, - setChatMessages, - addMessage, - setChat -} = chatSlice.actions; - -export default chatSlice.reducer; diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/features/chat/ChatState.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/ChatState.ts similarity index 100% rename from samples/apps/copilot-chat-app/WebApp/src/redux/features/chat/ChatState.ts rename to samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/ChatState.ts diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/ConversationsState.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/ConversationsState.ts index dbb44bc9c1d2..a047dc3bb2d7 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/ConversationsState.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/ConversationsState.ts @@ -1,30 +1,30 @@ // Copyright (c) Microsoft. All rights reserved. -import { ChatMessage } from "../../../libs/models/ChatMessage"; -import { ChatState, initialChatName, initialState as initialChatState } from "../chat/ChatState"; +import { ChatMessage } from '../../../libs/models/ChatMessage'; +import { ChatState, initialChatName, initialState as initialChatState } from './ChatState'; -export type Conversation = { - [key:string]: ChatState -} +export type Conversations = { + [key: string]: ChatState; +}; export interface ConversationsState { - conversations: Conversation; + conversations: Conversations; selectedId: string; } export const initialState: ConversationsState = { conversations: { - [initialChatName]: initialChatState + [initialChatName]: initialChatState, }, selectedId: initialChatName, }; export type UpdateConversationPayload = { - id: string, - messages: ChatMessage[] -} + id: string; + messages: ChatMessage[]; +}; export interface ConversationTitleChange { - id: string, - newId: string -} \ No newline at end of file + id: string; + newId: string; +} diff --git a/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/conversationsSlice.ts b/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/conversationsSlice.ts index 58e479fa1a64..b6cac36b1456 100644 --- a/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/conversationsSlice.ts +++ b/samples/apps/copilot-chat-app/WebApp/src/redux/features/conversations/conversationsSlice.ts @@ -2,23 +2,23 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { ChatMessage } from '../../../libs/models/ChatMessage'; -import { ChatState } from '../chat/ChatState'; -import { Conversation, ConversationsState, ConversationTitleChange, initialState } from './ConversationsState'; +import { ChatState } from './ChatState'; +import { Conversations, ConversationsState, ConversationTitleChange, initialState } from './ConversationsState'; export const conversationsSlice = createSlice({ name: 'hub', initialState, reducers: { - setConversations: (state: ConversationsState, action: PayloadAction) => { + setConversations: (state: ConversationsState, action: PayloadAction) => { state.conversations = action.payload; }, editConversationTitle: (state: ConversationsState, action: PayloadAction) => { const newId = action.payload.newId; state.conversations[action.payload.id].id = newId; - const updatedChat = { ...state.conversations[action.payload.id]}; + const updatedChat = { ...state.conversations[action.payload.id] }; delete state.conversations[action.payload.id]; // frontload conversation so it's at the top of chatlist - state.conversations = { [newId]: updatedChat , ...state.conversations} + state.conversations = { [newId]: updatedChat, ...state.conversations }; state.selectedId = action.payload.newId; }, setSelectedConversation: (state: ConversationsState, action: PayloadAction) => { @@ -28,18 +28,22 @@ export const conversationsSlice = createSlice({ const newId = action.payload.id ?? ''; state.conversations = { [newId]: action.payload, ...state.conversations }; }, - updateConversation: (state: ConversationsState, action: PayloadAction<{ message: ChatMessage, chatId?: string}>) => { + updateConversation: ( + state: ConversationsState, + action: PayloadAction<{ message: ChatMessage; chatId?: string }>, + ) => { const { message, chatId } = action.payload; const id = chatId ?? state.selectedId; const conversation = state.conversations[id]; conversation.messages.push(message); delete state.conversations[id]; // frontload conversation so it's at the top of chatlist - state.conversations = { [id]: conversation , ...state.conversations} + state.conversations = { [id]: conversation, ...state.conversations }; }, }, }); -export const { setConversations, editConversationTitle, setSelectedConversation, addConversation, updateConversation } = conversationsSlice.actions; +export const { setConversations, editConversationTitle, setSelectedConversation, addConversation, updateConversation } = + conversationsSlice.actions; export default conversationsSlice.reducer;