Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/backend/core/api/viewsets/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ def get(self, request, task_id):
result_data["error"] = task_result.result["error"]
elif isinstance(task_result.result, Exception):
logger.exception(
"Task %s failed with unhandled exception", task_id,
"Task %s failed with unhandled exception",
task_id,
exc_info=task_result.result,
)
result_data["status"] = "FAILURE"
Expand Down
9 changes: 5 additions & 4 deletions src/backend/core/api/viewsets/thread_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,17 @@ def create(self, request, *args, **kwargs):
def perform_destroy(self, instance):
"""Prevent deletion of the last editor access on a thread."""
if instance.role == enums.ThreadAccessRoleChoices.EDITOR:
remaining_editors = (
# Evaluate the queryset to actually acquire FOR UPDATE locks;
# .count() generates SELECT COUNT(*) which drops FOR UPDATE.
editor_ids = list(
models.ThreadAccess.objects.select_for_update()
.filter(
thread=instance.thread,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
.exclude(id=instance.id)
.exists()
.values_list("id", flat=True)
)
if not remaining_editors:
if len(editor_ids) <= 1:
raise ValidationError(
"Cannot delete the last editor access of a thread."
)
Expand Down
1 change: 0 additions & 1 deletion src/backend/core/tests/api/test_import_file_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ def test_api_task_detail_other_user_should_be_forbidden(self):
assert response.status_code == status.HTTP_200_OK
assert response.data["result"]["imported"] == 42


def test_api_task_detail_worker_crash_exception_should_be_serialized(self):
"""Test that a task with an exception result (e.g. WorkerLostError)
returns a properly serialized error instead of a 500."""
Expand Down
5 changes: 5 additions & 0 deletions src/frontend/src/features/forms/components/combobox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type ComboBoxProps = {
onChange?: (value: string[]) => void,
renderChipLabel?: (item: Option) => string,
valueValidator?: (value: string) => boolean,
autoFocus?: boolean,
} & Omit<SelectProps, 'value' | 'defaultValue' | 'onChange'>;

export const ComboBox = (props: ComboBoxProps) => {
Expand Down Expand Up @@ -150,6 +151,10 @@ export const ComboBox = (props: ComboBoxProps) => {
props.onChange?.(selectedItems.map(item => item.value || item.label));
}, [selectedItems]);

useEffect(() => {
if (props.autoFocus) inputRef.current?.focus();
}, [props.autoFocus]);

return (
<Field className={clsx("c__combobox", {
"c__combobox--disabled": props.disabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,11 +535,6 @@ export const MessageForm = ({
}
}

useEffect(() => {
if (draftMessage) form.setFocus("subject");
else form.setFocus("to")
}, []);

useEffect(() => {
startAutoSave();
return () => stopAutoSave();
Expand Down Expand Up @@ -599,6 +594,7 @@ export const MessageForm = ({
<RhfContactComboBox
name="to"
label={t("To:")}
autoFocus={mode === "forward"}
// icon={<span className="material-icons">group</span>}
text={form.formState.errors.to && !Array.isArray(form.formState.errors.to) ? form.formState.errors.to.message : t("Enter the email addresses of the recipients separated by commas")}
textItems={Array.isArray(form.formState.errors.to) ? form.formState.errors.to?.map((error, index) => t(error!.message as string, { email: form.getValues('to')?.[index] })) : []}
Expand Down Expand Up @@ -668,7 +664,7 @@ export const MessageForm = ({
draft={draft}
submitDraft={form.handleSubmit(saveDraft)}
ensureDraft={ensureDraft}
blockNoteOptions={{ autofocus: canWriteMessages ? "end" : undefined }}
blockNoteOptions={{ autofocus: canWriteMessages && mode !== "forward" ? "end" : undefined }}
uploadInlineImage={attachmentHook.uploadInlineImage}
uploadFiles={attachmentHook.uploadFiles}
removeInlineImage={attachmentHook.removeInlineImage}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
border-top: 1px solid var(--c--contextuals--border--default);
padding: var(--c--globals--spacings--sm) var(--c--globals--spacings--base);
z-index: 1;
transition: transform 0.2s ease-out, opacity 0.15s ease-out;
}

.thread-event-input--hidden {
transform: translateY(calc(100% + 1px));
opacity: 0;
pointer-events: none;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

.thread-event-input__edit-banner {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useRef, useState } from "react";
import { RefObject, useCallback, useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { Icon, IconSize, IconType, UserRow } from "@gouvfr-lasuite/ui-kit";
import { useThreadsEventsCreate, useThreadsEventsPartialUpdate, useThreadsUsersList, UserWithoutAbilities, ThreadEventTypeEnum, ThreadEvent } from "@/features/api/gen";
Expand All @@ -8,30 +8,42 @@ import { Button } from "@gouvfr-lasuite/cunningham-react";
import { useMailboxContext } from "@/features/providers/mailbox";
import { useAuth } from "@/features/auth";
import { SuggestionInput } from "@/features/ui/components/suggestion-input";
import { useThreadViewContext } from "../../provider";
import { useIMInputVisibility } from "./use-im-input-visibility";
import clsx from "clsx";

type ThreadEventInputProps = {
threadId: string;
editingEvent?: ThreadEvent | null;
onCancelEdit?: () => void;
onEventCreated?: () => void;
containerRef: RefObject<HTMLDivElement | null>;
};

/**
* Small fixed input bar at the bottom of the thread view for adding internal comments.
* Also handles editing existing events when `editingEvent` is provided.
*/
export const ThreadEventInput = ({ threadId, editingEvent, onCancelEdit, onEventCreated }: ThreadEventInputProps) => {
export const ThreadEventInput = ({ threadId, editingEvent, onCancelEdit, onEventCreated, containerRef }: ThreadEventInputProps) => {
const { t } = useTranslation();
const { invalidateThreadEvents } = useMailboxContext();
const { user: currentUser } = useAuth();
const { isMessageFormFocused } = useThreadViewContext();

const isEditing = !!editingEvent;

const { isVisible, onFocusChange } = useIMInputVisibility({
containerRef,
threadId,
isEditing,
isMessageFormFocused,
});
const textareaRef = useRef<HTMLTextAreaElement>(null);
const [content, setContent] = useState("");
const [mentions, setMentions] = useState<Array<{ id: string; name: string }>>([]);
const [showMentionPopover, setShowMentionPopover] = useState(false);
const [mentionFilter, setMentionFilter] = useState("");

const isEditing = !!editingEvent;

const createEvent = useThreadsEventsCreate();
const updateEvent = useThreadsEventsPartialUpdate();
const isPending = isEditing ? updateEvent.isPending : createEvent.isPending;
Expand Down Expand Up @@ -226,8 +238,23 @@ export const ThreadEventInput = ({ threadId, editingEvent, onCancelEdit, onEvent
}
}, [editingEvent, resetInput]);

const handleFocus = useCallback(() => {
onFocusChange(true);
}, [onFocusChange]);

const handleBlur = useCallback((e: React.FocusEvent<HTMLDivElement>) => {
if (!e.currentTarget.contains(e.relatedTarget as Node)) {
onFocusChange(false);
}
}, [onFocusChange]);

return (
<div className="thread-event-input">
<div
className={clsx("thread-event-input", { "thread-event-input--hidden": !isVisible })}
onFocus={handleFocus}
onBlur={handleBlur}
inert={!isVisible}
>
{isEditing && (
<div className="thread-event-input__edit-banner">
<Icon name="edit" type={IconType.OUTLINED} size={IconSize.SMALL} aria-hidden="true" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { RefObject, useEffect, useRef, useState } from "react";

/**
* Distance (px) from the bottom of the scroll container within which
* the ThreadEventInput stays visible. Beyond this threshold, the input
* slides out to avoid cluttering the reading area.
*/
const NEAR_BOTTOM_THRESHOLD = 150;

/**
* Minimum downward scroll delta between two animation frames to
* be considered a "fast scroll". When the user scrolls down quickly,
* the ThreadEventInput is revealed even if the user hasn't reached the
* bottom — signaling intent to interact.
*/
const FAST_SCROLL_DOWN_THRESHOLD = 35; // px
const FAST_SCROLL_TIMEOUT = 2000; // ms

/**
* Minimum upward scroll distance (px) from the point where fast-scroll
* was triggered before dismissing the input. Prevents tiny scroll
* adjustments from hiding the input too eagerly.
*/
const SCROLL_UP_DISMISS_THRESHOLD = 100; // px

type UseIMInputVisibilityOptions = {
containerRef: RefObject<HTMLDivElement | null>;
threadId: string;
isEditing: boolean;
isMessageFormFocused: boolean;
};

/**
* Manages the show/hide logic for the ThreadEventInput based on scroll
* position and user interaction:
* - visible when near the bottom of the scroll container
* - visible during fast downward scrolling (signals intent to interact)
* - visible when the input itself is focused or in edit mode
* - hidden when the message reply form has focus
*/
export const useIMInputVisibility = ({
containerRef,
threadId,
isEditing,
isMessageFormFocused,
}: UseIMInputVisibilityOptions) => {
const [isNearBottom, setIsNearBottom] = useState(false);
const [isFocused, setIsFocused] = useState(false);
const [isFastScrollingDown, setIsFastScrollingDown] = useState(false);
const rafRef = useRef<number | null>(null);
const lastScrollTopRef = useRef<number | null>(null);
const fastScrollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const fastScrollOriginRef = useRef<number | null>(null);

useEffect(() => {
const el = containerRef.current;
if (!el) return;

const handleScroll = () => {
if (rafRef.current !== null) return;
rafRef.current = requestAnimationFrame(() => {
rafRef.current = null;
const scrollTop = el.scrollTop;
const distanceFromBottom =
el.scrollHeight - scrollTop - el.clientHeight;
setIsNearBottom(distanceFromBottom <= NEAR_BOTTOM_THRESHOLD);

// Detect fast downward scroll
const prevScrollTop = lastScrollTopRef.current;
lastScrollTopRef.current = scrollTop;
if (prevScrollTop !== null) {
const delta = scrollTop - prevScrollTop;
if (delta >= FAST_SCROLL_DOWN_THRESHOLD) {
setIsFastScrollingDown(true);
fastScrollOriginRef.current = scrollTop;
if (fastScrollTimeoutRef.current !== null) {
clearTimeout(fastScrollTimeoutRef.current);
}
fastScrollTimeoutRef.current = setTimeout(() => {
fastScrollTimeoutRef.current = null;
fastScrollOriginRef.current = null;
setIsFastScrollingDown(false);
}, FAST_SCROLL_TIMEOUT);
} else if (
delta < 0 &&
fastScrollOriginRef.current !== null &&
fastScrollOriginRef.current - scrollTop >= SCROLL_UP_DISMISS_THRESHOLD
) {
if (fastScrollTimeoutRef.current !== null) {
clearTimeout(fastScrollTimeoutRef.current);
fastScrollTimeoutRef.current = null;
}
fastScrollOriginRef.current = null;
setIsFastScrollingDown(false);
}
}
});
};

el.addEventListener("scroll", handleScroll, { passive: true });
return () => {
el.removeEventListener("scroll", handleScroll);
if (rafRef.current !== null) {
cancelAnimationFrame(rafRef.current);
rafRef.current = null;
}
if (fastScrollTimeoutRef.current !== null) {
clearTimeout(fastScrollTimeoutRef.current);
fastScrollTimeoutRef.current = null;
}
};
}, [threadId]);

// Reset all scroll-related state on thread change
useEffect(
() => () => {
setIsNearBottom(false);
setIsFocused(false);
setIsFastScrollingDown(false);
lastScrollTopRef.current = null;
fastScrollOriginRef.current = null;
},
[threadId],
);

const isVisible =
isEditing ||
isFocused ||
(!isMessageFormFocused && (isNearBottom || isFastScrollingDown));

return {
isVisible,
onFocusChange: setIsFocused,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,16 @@ export const ThreadMessage = forwardRef<HTMLSpanElement, ThreadMessageProps>(
/>

{isMessageReady && showReplyForm && (
<section className="thread-message__reply-form" ref={replyFormRef}>
<section
className="thread-message__reply-form"
ref={replyFormRef}
onFocus={() => threadViewContext.setIsMessageFormFocused(true)}
onBlur={(e) => {
if (!e.currentTarget.contains(e.relatedTarget as Node)) {
threadViewContext.setIsMessageFormFocused(false);
}
}}
>
<MessageReplyForm
mode={replyFormMode}
handleClose={handleCloseReplyForm}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ const ThreadViewComponent = ({ threadItems, mailboxId, thread, showTrashedMessag
editingEvent={editingEvent}
onCancelEdit={() => setEditingEvent(null)}
onEventCreated={scrollToBottom}
containerRef={rootRef}
/>
)}
</div>
Expand Down Expand Up @@ -410,7 +411,7 @@ export const ThreadView = () => {
}

return (
<ThreadViewProvider messageIds={messageIds}>
<ThreadViewProvider threadId={selectedThread!.id} messageIds={messageIds}>
<ThreadViewComponent
mailboxId={selectedMailbox!.id}
thread={selectedThread!}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createContext, PropsWithChildren, useContext, useEffect, useMemo, useState } from "react";

type ThreadViewProviderProps = PropsWithChildren<{
threadId: string;
messageIds: readonly string[];
}>

Expand All @@ -11,6 +12,8 @@ type ThreadViewContextType = {
reset: (messageId?: string) => void;
hasBeenInitialized: boolean;
setHasBeenInitialized: (hasBeenInitialized: boolean) => void;
isMessageFormFocused: boolean;
setIsMessageFormFocused: (focused: boolean) => void;
}

const ThreadViewContext = createContext<ThreadViewContextType | undefined>(undefined);
Expand All @@ -19,9 +22,10 @@ const ThreadViewContext = createContext<ThreadViewContextType | undefined>(undef
* Provider to manage the thread view context.
* It allows to track the readiness state of the thread view (Does all messages content are loaded?).
*/
const ThreadViewProvider = ({ messageIds, children }: ThreadViewProviderProps) => {
const ThreadViewProvider = ({ threadId, messageIds, children }: ThreadViewProviderProps) => {
const [messagesReadiness, setMessagesReadiness] = useState(new Map(messageIds.map((id) => [id, false])));
const [hasBeenInitialized, setHasBeenInitialized] = useState(false);
const [isMessageFormFocused, setIsMessageFormFocused] = useState(false);
Comment thread
jbpenrath marked this conversation as resolved.

const isReady = useMemo(() => {
return Array.from(messagesReadiness.values()).every((isReady) => isReady === true);
Expand Down Expand Up @@ -62,11 +66,18 @@ const ThreadViewProvider = ({ messageIds, children }: ThreadViewProviderProps) =
reset,
hasBeenInitialized,
setHasBeenInitialized,
messagesReadiness,
}), [isReady, setMessageReadiness, isMessageReady, reset, messagesReadiness, hasBeenInitialized, setHasBeenInitialized]);
isMessageFormFocused,
setIsMessageFormFocused,
}), [isReady, setMessageReadiness, isMessageReady, reset, hasBeenInitialized, setHasBeenInitialized, isMessageFormFocused]);



// Reset focus state when the active thread changes to prevent a stale
// `true` from hiding ThreadEventInput on the next thread.
useEffect(() => {
setIsMessageFormFocused(false);
}, [threadId]);

// If the list of message IDs changes, update the readiness state context
useEffect(() => {
const currentMessageIds = Array.from(messagesReadiness.keys());
Expand Down
Loading