-
Notifications
You must be signed in to change notification settings - Fork 7
RG-T117 Chatbot fixes #263
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,6 +103,13 @@ export default function ChatScreen() { | |
|
|
||
| const openChannel = useCallback( | ||
| (channelId: string) => { | ||
| // The assistant conversation always opens in its dedicated restricted screen | ||
| // (text only, no reactions/threads/deletes) instead of the generic conversation. | ||
| const channel = useChatStore.getState().channels.find((c) => c.ChatChannelId === channelId); | ||
| if (channel?.ChannelType === ChatChannelType.Chatbot) { | ||
| router.push('/chatbot' as Href); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Magic string vulnerability caused by hardcoding the route path '/chatbot' as a raw string literal. Define a centralized routes module (e.g., Kody rule violation: Centralize string constants Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return; | ||
| } | ||
| router.push(`/chat/${channelId}` as Href); | ||
| }, | ||
| [router] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,13 @@ | |
| import { useTranslation } from 'react-i18next'; | ||
| import { Platform } from 'react-native'; | ||
|
|
||
| import { copyToClipboard } from '@/components/chat/chat-utils'; | ||
| import { MessageActionsSheet } from '@/components/chat/message-actions-sheet'; | ||
| import { MessageBubble } from '@/components/chat/message-bubble'; | ||
| import { TypingDots } from '@/components/chat/typing-indicator'; | ||
| import { Actionsheet, ActionsheetBackdrop, ActionsheetContent, ActionsheetDragIndicator, ActionsheetDragIndicatorWrapper } from '@/components/ui/actionsheet'; | ||
| import { Box } from '@/components/ui/box'; | ||
| import { Button, ButtonText } from '@/components/ui/button'; | ||
| import { Center } from '@/components/ui/center'; | ||
| import { FlatList } from '@/components/ui/flat-list'; | ||
| import { FocusAwareStatusBar } from '@/components/ui/focus-aware-status-bar'; | ||
|
|
@@ -16,11 +20,14 @@ | |
| import { Pressable } from '@/components/ui/pressable'; | ||
| import { Spinner } from '@/components/ui/spinner'; | ||
| import { Text } from '@/components/ui/text'; | ||
| import { Textarea, TextareaInput } from '@/components/ui/textarea'; | ||
| import { VStack } from '@/components/ui/vstack'; | ||
| import { type ChatMessageResultData } from '@/models/v4/chat'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
| import { useChatStore } from '@/stores/chat/store'; | ||
| import { useChatSystemStatus } from '@/stores/feature-flags/store'; | ||
| import { securityStore } from '@/stores/security/store'; | ||
| import { useToastStore } from '@/stores/toast/store'; | ||
|
|
||
| export default function ChatbotScreen() { | ||
| const { t } = useTranslation(); | ||
|
|
@@ -30,7 +37,11 @@ | |
| const chatbotChannelId = useChatStore((s) => s.chatbotChannelId); | ||
| const chatbotTyping = useChatStore((s) => s.chatbotTyping); | ||
| const messages = useChatStore((s) => (chatbotChannelId ? s.messagesByChannel[chatbotChannelId] : undefined)); | ||
| const isModerator = !!securityStore((s) => s.rights)?.IsAdmin; | ||
| const [text, setText] = useState(''); | ||
| const [actionsMessage, setActionsMessage] = useState<ChatMessageResultData | null>(null); | ||
| const [editMessage, setEditMessage] = useState<ChatMessageResultData | null>(null); | ||
| const [editText, setEditText] = useState(''); | ||
|
|
||
| useFocusEffect( | ||
| useCallback(() => { | ||
|
|
@@ -61,7 +72,7 @@ | |
|
|
||
| const renderItem = useCallback( | ||
| ({ item }: { item: ChatMessageResultData }) => ( | ||
| <MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={() => undefined} onToggleReaction={() => undefined} /> | ||
| <MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={setActionsMessage} onToggleReaction={() => undefined} /> | ||
|
Check warning on line 75 in src/app/(app)/chatbot.tsx
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Hide reaction controls in assistant messages. If an assistant message has existing reactions, Pass an explicit assistant or As per coding guidelines, avoid anonymous functions in 🤖 Prompt for AI AgentsSource: Coding guidelines There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance regression caused by inline arrow functions in JSX props, which create new function instances on every render. Move these function definitions outside the render method to prevent unnecessary re-renders. Kody rule violation: Avoid using .bind() or arrow functions in JSX props Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| ), | ||
| [currentUserId] | ||
| ); | ||
|
|
@@ -138,6 +149,57 @@ | |
| </Pressable> | ||
| </HStack> | ||
| </KeyboardAvoidingView> | ||
|
|
||
| {/* Restricted actions for assistant messages: copy, edit own, pin (moderator), flag. */} | ||
| <MessageActionsSheet | ||
| message={actionsMessage} | ||
| isOpen={actionsMessage !== null} | ||
| onClose={() => setActionsMessage(null)} | ||
| isOwn={!!actionsMessage?.SenderUserId && actionsMessage.SenderUserId === currentUserId} | ||
| isModerator={isModerator} | ||
| assistant | ||
| onReact={() => undefined} | ||
| onReply={() => undefined} | ||
| onCopy={async (m) => { | ||
| const ok = await copyToClipboard(m.Body ?? ''); | ||
| useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable')); | ||
| }} | ||
| onEdit={(m) => { | ||
| setEditMessage(m); | ||
| setEditText(m.Body ?? ''); | ||
| }} | ||
| onDelete={() => undefined} | ||
| onFlag={(m, reason) => useChatStore.getState().flagMessage(m.ChatMessageId, reason)} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled promise rejection occurs because the Kody rule violation: Handle async operations with proper error handling Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| onTogglePin={(m, pinned) => chatbotChannelId && useChatStore.getState().togglePin(m.ChatMessageId, chatbotChannelId, pinned)} | ||
| onModeratorDelete={() => undefined} | ||
| /> | ||
|
|
||
| {/* Edit own message */} | ||
| <Actionsheet isOpen={editMessage !== null} onClose={() => setEditMessage(null)}> | ||
| <ActionsheetBackdrop /> | ||
| <ActionsheetContent> | ||
| <ActionsheetDragIndicatorWrapper> | ||
| <ActionsheetDragIndicator /> | ||
| </ActionsheetDragIndicatorWrapper> | ||
| <VStack className="w-full p-2" space="md"> | ||
| <Text className="text-base font-semibold text-typography-900">{t('chat.edit_message')}</Text> | ||
| <Textarea> | ||
| <TextareaInput value={editText} onChangeText={setEditText} multiline /> | ||
| </Textarea> | ||
| <Button | ||
| className="bg-primary-600" | ||
| onPress={() => { | ||
| if (editMessage && chatbotChannelId && editText.trim()) { | ||
| void useChatStore.getState().editMessage(editMessage.ChatMessageId, chatbotChannelId, editText.trim()); | ||
| } | ||
| setEditMessage(null); | ||
| }} | ||
| > | ||
| <ButtonText>{t('chat.save')}</ButtonText> | ||
| </Button> | ||
| </VStack> | ||
| </ActionsheetContent> | ||
| </Actionsheet> | ||
| </Box> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,18 +59,35 @@ export default function ChannelConversationScreen() { | |
| const [editText, setEditText] = useState(''); | ||
| const [imageUri, setImageUri] = useState<string | null>(null); | ||
| const [presenceIds, setPresenceIds] = useState<Set<string>>(new Set()); | ||
| const [resolveAttempted, setResolveAttempted] = useState(false); | ||
| const unsubscribeRef = useRef<(() => void) | null>(null); | ||
|
|
||
| const isDm = channel?.ChannelType === ChatChannelType.DirectMessage; | ||
| const isChatbot = channel?.ChannelType === ChatChannelType.Chatbot; | ||
| // Deep links (push notifications, cold starts) can arrive before the channel | ||
| // list loads; the channel type is unknown until then. Treat a completed fetch | ||
| // with no match as resolved so unknown channels keep the generic screen. | ||
| const isResolved = !!channel || resolveAttempted; | ||
| const showSender = !isDm; | ||
|
|
||
| // Chronological order (oldest-first); FlashList renders bottom-anchored via maintainVisibleContentPosition. | ||
| const ordered = useMemo(() => messages ?? [], [messages]); | ||
|
|
||
| // Mount: activate channel, join hub, load history and members. | ||
| // Resolve the channel identity for deep links before mounting the generic view. | ||
| useEffect(() => { | ||
| if (channel || resolveAttempted || !isChatEnabled) return; | ||
| void useChatStore | ||
| .getState() | ||
| .fetchChannels() | ||
| .finally(() => setResolveAttempted(true)); | ||
|
Comment on lines
+79
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled promise rejection occurs because the Kody rule violation: Handle async operations with proper error handling Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| }, [channel, resolveAttempted, isChatEnabled]); | ||
|
|
||
| // Mount: activate channel, join hub, load history and members. Assistant | ||
| // conversations are handled by the dedicated chatbot screen — never join or | ||
| // load them here, and wait for unresolved deep links to identify first. | ||
| useFocusEffect( | ||
| useCallback(() => { | ||
| if (!channelId || !isChatEnabled) return; | ||
| if (!channelId || !isChatEnabled || !isResolved || isChatbot) return; | ||
| const store = useChatStore.getState(); | ||
| store.setActiveChannel(channelId); | ||
| void store.joinChannel(channelId); | ||
|
|
@@ -79,7 +96,7 @@ export default function ChannelConversationScreen() { | |
| return () => { | ||
| useChatStore.getState().setActiveChannel(null); | ||
| }; | ||
| }, [channelId, isChatEnabled]) | ||
| }, [channelId, isChatEnabled, isResolved, isChatbot]) | ||
| ); | ||
|
|
||
| // Fetch presence for the channel members (for the header online dot). | ||
|
|
@@ -99,10 +116,10 @@ export default function ChannelConversationScreen() { | |
|
|
||
| // Mark read whenever the newest message changes while viewing. | ||
| useEffect(() => { | ||
| if (channelId && ordered.length > 0) { | ||
| if (channelId && isResolved && !isChatbot && ordered.length > 0) { | ||
| void useChatStore.getState().markChannelRead(channelId); | ||
| } | ||
| }, [channelId, ordered.length]); | ||
| }, [channelId, isResolved, isChatbot, ordered.length]); | ||
|
|
||
| const otherOnline = useMemo(() => { | ||
| if (!isDm) return false; | ||
|
|
@@ -257,6 +274,23 @@ export default function ChannelConversationScreen() { | |
| return <Redirect href="/(app)" />; | ||
| } | ||
|
|
||
| // Deep link to a channel that isn't loaded yet: wait for the channel list so | ||
| // assistant conversations never mount the full-featured view. | ||
| if (!isResolved) { | ||
| return ( | ||
| <Box className="size-full flex-1 items-center justify-center bg-background-0"> | ||
| <Stack.Screen options={{ title, headerShown: true, headerBackTitle: '' }} /> | ||
| <Spinner /> | ||
| </Box> | ||
| ); | ||
| } | ||
|
|
||
| // Assistant conversations always use the dedicated restricted screen (text only, | ||
| // no reactions/threads/deletes) — catch deep links and stale routes here. | ||
| if (isChatbot) { | ||
| return <Redirect href={'/chatbot' as Href} />; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| return ( | ||
| <Box className="size-full flex-1 bg-background-0"> | ||
| <Stack.Screen | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import * as Clipboard from 'expo-clipboard'; | ||
| import { type TFunction } from 'i18next'; | ||
|
|
||
| import { getAvatarUrl } from '@/lib/utils'; | ||
|
|
@@ -105,9 +106,9 @@ export function hasLink(body?: string | null): boolean { | |
| } | ||
|
|
||
| /** | ||
| * Copies text to the clipboard. Works on web/Electron via the async Clipboard | ||
| * API; native returns false (no clipboard native module is installed) so callers | ||
| * can surface an appropriate message. | ||
| * Copies text to the clipboard. Uses the async Clipboard API on web/Electron | ||
| * and expo-clipboard on native; returns false only when both are unavailable | ||
| * or the write fails, so callers can surface an appropriate message. | ||
| */ | ||
| export async function copyToClipboard(text: string): Promise<boolean> { | ||
| try { | ||
|
|
@@ -117,9 +118,13 @@ export async function copyToClipboard(text: string): Promise<boolean> { | |
| return true; | ||
| } | ||
| } catch { | ||
| // ignore and fall through | ||
| // ignore and fall through to the native module | ||
| } | ||
| try { | ||
| return await Clipboard.setStringAsync(text); | ||
| } catch { | ||
| return false; | ||
| } | ||
|
Comment on lines
+123
to
127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Void return mismatch causes a successful native copy to evaluate as falsy, triggering the 'copy_unavailable' toast in try {
await Clipboard.setStringAsync(text);
return true;
} catch {
return false;
}Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return false; | ||
| } | ||
|
|
||
| const IMAGE_MIME_BY_EXTENSION: Record<string, 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.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the required API endpoint wrapper.
These helpers call the Axios client directly. Route them through
createApiEndpointorcreateCachedApiEndpointto follow the required API-module boundary.As per coding guidelines, always use
createApiEndpointorcreateCachedApiEndpointfor API endpoints with typed response generics.Also applies to: 17-22
🤖 Prompt for AI Agents
Source: Coding guidelines