Fix: Always include selected options at the top of the list

.github/workflows/postgres-end-to-end-tests.yml

@@ -77,6 +77,13 @@ jobs:
           pnpm test-ui-postgres &&
           killall node
 
+      - uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: playwright-report
+          path: src/packages/end-to-end/playwright-report/
+          retention-days: 30
+
     env:
       CI: true
       DATABASE_HOST: localhost

src/packages/admin-ui-components/src/combo-box/component.tsx

@@ -68,12 +68,12 @@ export const ComboBox = ({
 	dataFetcher,
 	searchDebounceMs = 300,
 }: SelectProps) => {
-	const valueArray = arrayify(value);
+	const valueArray = arrayify(value) as SelectOption[];
 	const inputRef = useAutoFocus<HTMLInputElement>(autoFocus);
 	const selectBoxRef = useRef<HTMLDivElement>(null);
 
 	// Lazy loading state
-	const [dynamicOptions, setDynamicOptions] = useState<SelectOption[]>([]);
+	const [dynamicOptions, setDynamicOptions] = useState<SelectOption[]>(valueArray);
 	const [isLoadingMore, setIsLoadingMore] = useState(false);
 	const [searchTerm, setSearchTerm] = useState('');
 	const [searchTimeout, setSearchTimeout] = useState<NodeJS.Timeout | null>(null);
@@ -86,17 +86,20 @@ export const ComboBox = ({
 	// Use ref to track if we're already loading data to prevent duplicate fetches
 	const fetchedPagesRef = useRef(new Set<number>());
 
+	// Store the selected ids in a set for easy lookup - this is our source of truth for selection
+	const selectedIds = useMemo(() => new Set(valueArray.map((item) => item.value)), [valueArray]);
+
+	const sortOptionsBySelectedFirst = useCallback((opt1: SelectOption, opt2: SelectOption) => {
+		return (selectedIds.has(opt2.value) ? 1 : 0) - (selectedIds.has(opt1.value) ? 1 : 0)
+	}, [selectedIds]);
+
 	// Calculate items once - use dynamic options if dataFetcher is provided, otherwise use static options
 	const options = useMemo(() => {
 		return dataFetcher ? dynamicOptions : staticOptions || [];
 	}, [dataFetcher, dynamicOptions, staticOptions]);
 
-	// Store the selected ids in a set for easy lookup - this is our source of truth for selection
-	const selectedIds = useMemo(() => new Set(valueArray.map((item) => item.value)), [valueArray]);
-
-	const sortOptionsBySelectedFirst = (opt1: SelectOption, opt2: SelectOption) => {
-		return (selectedIds.has(opt2.value) ? 1 : 0) - (selectedIds.has(opt1.value) ? 1 : 0)
-	}
+	// Duplicates can occur for a variety of reasons – e.g., when using this for a string field filter
+	const optionIds = useMemo(() => new Set(options.map((item) => item.value)), [options]);
 
 	// Handle individual item deselection
 	const handleItemDeselect = useCallback(
@@ -121,14 +124,15 @@ export const ComboBox = ({
 	} = useCombobox({
 		items: options,
 		id: fieldId,
+		selectedItem: null,
 		itemToString: (item) => item?.label ?? '',
 		isItemDisabled: () => disabled,
 		onInputValueChange: ({ inputValue }) => {
 			onInputChange?.(inputValue);
 
 			if (dataFetcher && inputValue !== undefined) {
 				fetchedPagesRef.current.clear();
-				setDynamicOptions([]);
+				setDynamicOptions(valueArray);
 				setCurrentPage(1);
 				setHasReachedEnd(false);
 				setSearchTerm(inputValue);
@@ -187,14 +191,14 @@ export const ComboBox = ({
 				if (lastSearchTermRef.current === search) {
 					// Search term hasn't changed, merge the options in.
 					if (result && result.length > 0) {
-						setDynamicOptions((prev) => [...prev, ...result]);
+						setDynamicOptions((prev) => [...prev, ...result.filter(o => !optionIds.has(o.value))]);
 						setCurrentPage(page);
 					} else {
 						setHasReachedEnd(true);
 					}
 				} else {
 					// If the search term has changed, we need to reset the options
-					setDynamicOptions(result);
+					setDynamicOptions([...valueArray, ...result.filter(o => !selectedIds.has(o.value))]);
 					setCurrentPage(1);
 					setHasReachedEnd(false);
 					fetchedPagesRef.current.clear();
@@ -207,7 +211,7 @@ export const ComboBox = ({
 				setIsLoadingMore(false);
 			}
 		},
-		[dataFetcher, isOpen]
+		[dataFetcher, isOpen, selectedIds, optionIds]
 	);
 
 	// Scroll the menu to the top when it's opened.

src/packages/end-to-end/src/__tests__/ui/postgres/combobox.test.ts

@@ -0,0 +1,54 @@
+import { test, expect } from '@playwright/test';
+import { config } from '../../../config';
+
+test('Detail Panel - should allow deselecting of an entity in a one to many relationship field', async ({
+	page,
+}) => {
+	await page.goto(config.adminUiUrl);
+
+	await page.getByTestId('Invoice-entity-link').click();
+	await page.getByRole('cell', { name: '1', exact: true }).click();
+
+	// We should begin in this state
+	await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText(
+		'invoiceLines2 Selected×'
+	);
+
+	await page.getByTestId('detail-panel-field-invoiceLines').click();
+	await page.getByRole('option', { name: '2', exact: true }).click();
+
+	await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText('invoiceLines1×');
+});
+
+test('Filter - should be able to interact with and select options using the keyboard', async ({
+	page,
+}) => {
+	await page.goto(config.adminUiUrl);
+	await page.getByTestId('Album-entity-link').click();
+
+	// First arrow press opens the list
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	// Go down to the third option
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	await page.getByTestId('artist-filter-input').press('Enter');
+	await expect(page.getByTestId('artist-filter-box')).toContainText(/^A/);
+});
+
+test('Filter - should be able to deselect options using the keyboard', async ({ page }) => {
+	await page.goto(config.adminUiUrl);
+	await page.getByTestId('Album-entity-link').click();
+
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	await page.getByRole('option', { name: 'AC/DC' }).click();
+	await page.getByTestId('artist-filter-input').press('Escape');
+
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	await expect(page.getByRole('option').first()).toContainText('AC/DC');
+
+	// Deselect
+	await page.getByTestId('artist-filter-input').press('ArrowDown');
+	await page.getByTestId('artist-filter-input').press('Enter');
+	await expect(page.getByTestId('artist-filter-box')).not.toContainText('AC/DC');
+});

src/packages/end-to-end/src/__tests__/ui/postgres/enum-editor.test.ts

@@ -20,47 +20,3 @@ test('Detail Panel - should allow editing of an entity with an enum dropdown', a
 		'paymentStatusUNPAID×'
 	);
 });
-
-test('Detail Panel - should allow deselecting of an entity in a one to many relationship field', async ({
-	page,
-}) => {
-	await page.goto(config.adminUiUrl);
-
-	await page.getByTestId('Invoice-entity-link').click();
-	await page.getByRole('cell', { name: '1', exact: true }).click();
-
-	// We should begin in this state
-	await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText(
-		'invoiceLines2 Selected×'
-	);
-
-	await page.getByTestId('detail-panel-field-invoiceLines').click();
-	await page.getByRole('option', { name: '2', exact: true }).click();
-
-	await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText('invoiceLines1×');
-});
-
-test('Filter - should be able to interact with and select options using the keyboard', async ({
-	page,
-}) => {
-	await page.goto(config.adminUiUrl);
-	await page.getByTestId('Album-entity-link').click();
-
-	// First arrow press opens the list
-	await page.getByTestId('artist-filter-input').press('ArrowDown');
-	// Go down to the third option
-	await page.getByTestId('artist-filter-input').press('ArrowDown');
-	await page.getByTestId('artist-filter-input').press('ArrowDown');
-	await page.getByTestId('artist-filter-input').press('ArrowDown');
-	await page.getByTestId('artist-filter-input').press('Enter');
-	await expect(page.getByTestId('artist-filter-box')).toContainText('Aaron Goldberg');
-
-	// Re-open the list, and then the selected option should now be the first in the list
-	await page.getByTestId('artist-filter-input').press('ArrowDown');
-	await expect(page.getByRole('option').first()).toContainText('Aaron Goldberg');
-
-	// Deselect
-	await page.getByTestId('artist-filter-input').press('ArrowDown');
-	await page.getByTestId('artist-filter-input').press('Enter');
-	await expect(page.getByTestId('artist-filter-box')).not.toContainText('Aaron Goldberg');
-});