[bugfix/frontend] fix typo and other oddness in patchRemoteEmojis (#2281)

* fix emoji test model

* found the bug!

* remove unused 'current' import

* comment useChecklistReducer

* wah

* lint

* fix cleaner tests

internal/cleaner/emoji_test.go

@@ -9,8 +9,21 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
+func copyMap(in map[string]*gtsmodel.Emoji) map[string]*gtsmodel.Emoji {
+	out := make(map[string]*gtsmodel.Emoji, len(in))
+
+	for k, v1 := range in {
+		v2 := new(gtsmodel.Emoji)
+		*v2 = *v1
+		out[k] = v2
+	}
+
+	return out
+}
+
 func (suite *CleanerTestSuite) TestEmojiUncacheRemote() {
 	suite.testEmojiUncacheRemote(
 		context.Background(),
@@ -54,16 +67,28 @@ func (suite *CleanerTestSuite) TestEmojiPruneUnusedDryRun() {
 }
 
 func (suite *CleanerTestSuite) TestEmojiFixCacheStates() {
+	// Copy testrig emojis + mark
+	// rainbow emoji as uncached
+	// so there's something to fix.
+	emojis := copyMap(suite.emojis)
+	emojis["rainbow"].Cached = util.Ptr(false)
+
 	suite.testEmojiFixCacheStates(
 		context.Background(),
-		mapvals(suite.emojis),
+		mapvals(emojis),
 	)
 }
 
 func (suite *CleanerTestSuite) TestEmojiFixCacheStatesDryRun() {
+	// Copy testrig emojis + mark
+	// rainbow emoji as uncached
+	// so there's something to fix.
+	emojis := copyMap(suite.emojis)
+	emojis["rainbow"].Cached = util.Ptr(false)
+
 	suite.testEmojiFixCacheStates(
 		gtscontext.SetDryRun(context.Background()),
-		mapvals(suite.emojis),
+		mapvals(emojis),
 	)
 }
 

testrig/testmodels.go

@@ -1129,7 +1129,7 @@ func NewTestEmojis() map[string]*gtsmodel.Emoji {
 			ImageRemoteURL:         "",
 			ImageStaticRemoteURL:   "",
 			ImageURL:               "http://localhost:8080/fileserver/01AY6P665V14JJR0AFVRT7311Y/emoji/original/01F8MH9H8E4VG3KDYJR9EGPXCQ.png",
-			ImagePath:              "/01AY6P665V14JJR0AFVRT7311Y/emoji/original/01F8MH9H8E4VG3KDYJR9EGPXCQ.png",
+			ImagePath:              "01AY6P665V14JJR0AFVRT7311Y/emoji/original/01F8MH9H8E4VG3KDYJR9EGPXCQ.png",
 			ImageStaticURL:         "http://localhost:8080/fileserver/01AY6P665V14JJR0AFVRT7311Y/emoji/static/01F8MH9H8E4VG3KDYJR9EGPXCQ.png",
 			ImageStaticPath:        "01AY6P665V14JJR0AFVRT7311Y/emoji/static/01F8MH9H8E4VG3KDYJR9EGPXCQ.png",
 			ImageContentType:       "image/png",
@@ -1141,7 +1141,7 @@ func NewTestEmojis() map[string]*gtsmodel.Emoji {
 			URI:                    "http://localhost:8080/emoji/01F8MH9H8E4VG3KDYJR9EGPXCQ",
 			VisibleInPicker:        util.Ptr(true),
 			CategoryID:             "01GGQ8V4993XK67B2JB396YFB7",
-			Cached:                 util.Ptr(false),
+			Cached:                 util.Ptr(true),
 		},
 		"yell": {
 			ID:                     "01GD5KP5CQEE1R3X43Y1EHS2CW",

web/source/settings/admin/emoji/remote/parse-from-toot.js

@@ -127,11 +127,12 @@ function CopyEmojiForm({ localEmojiCodes, type, emojiList }) {
 		{
 			changedOnly: false,
 			onFinish: ({ data }) => {
-				if (data != undefined) {
+				if (data) {
-					form.selectedEmoji.updateMultiple(
+					// uncheck all successfully processed emoji
-						// uncheck all successfully processed emoji
+					const processed = data.map((emoji) => {
-						data.map(([id]) => [id, { checked: false }])
+						return [emoji.id, { checked: false }];
-					);
+					});
+					form.selectedEmoji.updateMultiple(processed);
 				}
 			}
 		}

web/source/settings/lib/form/check-list.tsx

@@ -18,15 +18,12 @@
 */
 
 import {
-	useReducer,
 	useRef,
 	useEffect,
 	useCallback,
 	useMemo,
 } from "react";
 
-import { PayloadAction, createSlice } from "@reduxjs/toolkit";
-
 import type {
 	Checkable,
 	ChecklistInputHook,
@@ -34,106 +31,12 @@ import type {
 	HookOpts,
 } from "./types";
 
-// https://immerjs.github.io/immer/installation#pick-your-immer-version
+import {
-import { enableMapSet } from "immer";
+	useChecklistReducer,
-enableMapSet();
+	actions,
-
+} from "../../redux/checklist";
-interface ChecklistState {
-	entries: { [k: string]: Checkable },
-	selectedEntries: Set<string>,
-}
-
-const initialState: ChecklistState = {
-	entries: {},
-	selectedEntries: new Set(),
-};
-
-const { reducer, actions } = createSlice({
-	name: "checklist",
-	initialState, // not handled by slice itself
-	reducers: {
-		updateAll: (state, { payload: checked }: PayloadAction<boolean>) => {
-			const selectedEntries = new Set<string>();
-			const entries = Object.fromEntries(
-				Object.values(state.entries).map((entry) => {
-					if (checked) {
-						// Cheekily add this to selected
-						// entries while we're here.
-						selectedEntries.add(entry.key);
-					}
-
-					return [entry.key, { ...entry, checked } ];
-				})
-			);
-			
-			return { entries, selectedEntries };
-		},
-		update: (state, { payload: { key, value } }: PayloadAction<{key: string, value: Checkable}>) => {
-			if (value.checked !== undefined) {
-				if (value.checked === true) {
-					state.selectedEntries.add(key);
-				} else {
-					state.selectedEntries.delete(key);
-				}
-			}
-
-			state.entries[key] = {
-				...state.entries[key],
-				...value
-			};
-		},
-		updateMultiple: (state, { payload }: PayloadAction<Array<[key: string, value: Checkable]>>) => {
-			payload.forEach(([key, value]) => {
-				if (value.checked !== undefined) {
-					if (value.checked === true) {
-						state.selectedEntries.add(key);
-					} else {
-						state.selectedEntries.delete(key);
-					}
-				}
-
-				state.entries[key] = {
-					...state.entries[key],
-					...value
-				};
-			});
-		}
-	}
-});
-
-function initialHookState({
-	entries,
-	uniqueKey,
-	initialValue,
-}: {
-	entries: Checkable[],
-	uniqueKey: string,
-	initialValue: boolean,
-}): ChecklistState {
-	const selectedEntries = new Set<string>();
-	const mappedEntries = Object.fromEntries(
-		entries.map((entry) => {
-			const key = entry[uniqueKey];
-			const checked = entry.checked ?? initialValue;
-			
-			if (checked) {
-				selectedEntries.add(key);
-			} else {
-				selectedEntries.delete(key);
-			}
-		
-			return [ key, { ...entry, key, checked } ];
-		})
-	);
-
-	return {
-		entries: mappedEntries,
-		selectedEntries
-	};
-}
 
 const _default: { [k: string]: Checkable } = {};
-
 export default function useCheckListInput(
 	/* eslint-disable no-unused-vars */
 	{ name, Name }: CreateHookNames,
@@ -143,12 +46,7 @@ export default function useCheckListInput(
 		initialValue = false,
 	}: HookOpts<boolean>
 ): ChecklistInputHook {
-	const [state, dispatch] = useReducer(
+	const [state, dispatch] = useChecklistReducer(entries, uniqueKey, initialValue);
-		reducer,
-		initialState,
-		(_) => initialHookState({ entries, uniqueKey, initialValue }) // initial state
-	);
-
 	const toggleAllRef = useRef<any>(null);
 
 	useEffect(() => {
@@ -167,17 +65,17 @@ export default function useCheckListInput(
 
 	const reset = useCallback(
 		() => dispatch(actions.updateAll(initialValue)),
-		[initialValue]
+		[initialValue, dispatch]
 	);
 
 	const onChange = useCallback(
-		(key, value) => dispatch(actions.update({ key, value })),
+		(key: string, value: Checkable) => dispatch(actions.update({ key, value })),
-		[]
+		[dispatch]
 	);
 
 	const updateMultiple = useCallback(
-		(entries) => dispatch(actions.updateMultiple(entries)),
+		(entries: [key: string, value: Partial<Checkable>][]) => dispatch(actions.updateMultiple(entries)),
-		[]
+		[dispatch]
 	);
 
 	return useMemo(() => {
@@ -215,5 +113,5 @@ export default function useCheckListInput(
 				onChange: toggleAll
 			}
 		});
-	}, [state, reset, name, onChange, updateMultiple]);
+	}, [state, reset, name, onChange, updateMultiple, dispatch]);
 }

web/source/settings/lib/form/types.ts

@@ -152,7 +152,7 @@ interface _withSomeSelected {
 }
 
 interface _withUpdateMultiple {
-	updateMultiple: (_entries: any) => void;
+	updateMultiple: (entries: [key: string, value: Partial<Checkable>][]) => void;
 }
 
 export interface TextFormInputHook extends FormInputHook<string>,

web/source/settings/lib/query/admin/custom-emoji/index.ts

@@ -199,11 +199,14 @@ const extended = gtsApi.injectEndpoints({
 				});
 
 				if (errors.length !== 0) {
+					const errData = errors.map(e => JSON.stringify(e.data)).join(",");
 					return {
 						error: {
 							status: 400,
 							statusText: 'Bad Request',
-							data: {"error":`One or more errors fetching custom emojis: ${errors}`},
+							data: {
+								error: `One or more errors fetching custom emojis: [${errData}]`
+							},
 						},
 					};	
 				}
@@ -222,14 +225,18 @@ const extended = gtsApi.injectEndpoints({
 
 		patchRemoteEmojis: build.mutation({
 			async queryFn({ action, ...formData }, _api, _extraOpts, fetchWithBQ) {
-				const data: CustomEmoji[] = [];
 				const errors: FetchBaseQueryError[] = [];
+				const selectedEmoji: CustomEmoji[] = formData.selectedEmoji;
 				
-				formData.selectEmoji.forEach(async(emoji: CustomEmoji) => {
+				// Map function to get a promise
-					let body = {
+				// of an emoji (or null).
+				const copyEmoji = async(emoji: CustomEmoji) => {
+					let body: {
+						type: string;
+						shortcode?: string;
+						category?: string;
+					} = {
 						type: action,
-						shortcode: "",
-						category: "",
 					};
 
 					if (action == "copy") {
@@ -243,22 +250,43 @@ const extended = gtsApi.injectEndpoints({
 						method: "PATCH",
 						url: `/api/v1/admin/custom_emojis/${emoji.id}`,
 						asForm: true,
-						body: body
+						body: body,
 					});
+
 					if (emojiRes.error) {
 						errors.push(emojiRes.error);
-					} else {
+						return null;
-						// Got it!
-						data.push(emojiRes.data as CustomEmoji);
 					}
-				});
+					
+					// Instead of mapping to the emoji we just got in emojiRes.data,
+					// we map here to the existing emoji. The reason for this is that
+					// if we return the new emoji, it has a new ID, and the checklist
+					// component calling this function gets its state mixed up.
+					//
+					// For example, say you copy an emoji with ID "some_emoji"; the
+					// result would return an emoji with ID "some_new_emoji_id". The
+					// checklist state would then contain one emoji with ID "some_emoji",
+					// and the new copy of the emoji with ID "some_new_emoji_id", leading
+					// to weird-looking bugs where it suddenly appears as if the searched
+					// status has another blank emoji attached to it.
+					return emoji;
+				};
+
+				// Wait for all the promises to
+				// resolve and remove any nulls.
+				const data = (
+					await Promise.all(selectedEmoji.map(copyEmoji))
+				).flatMap((emoji) => emoji || []);
 
 				if (errors.length !== 0) {
+					const errData = errors.map(e => JSON.stringify(e.data)).join(",");
 					return {
 						error: {
 							status: 400,
 							statusText: 'Bad Request',
-							data: {"error":`One or more errors patching custom emojis: ${errors}`},
+							data: {
+								error: `One or more errors patching custom emojis: [${errData}]`
+							},
 						},
 					};	
 				}

web/source/settings/redux/checklist.ts

@@ -0,0 +1,158 @@
+/*
+	GoToSocial
+	Copyright (C) GoToSocial Authors admin@gotosocial.org
+	SPDX-License-Identifier: AGPL-3.0-or-later
+
+	This program is free software: you can redistribute it and/or modify
+	it under the terms of the GNU Affero General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	This program is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU Affero General Public License for more details.
+
+	You should have received a copy of the GNU Affero General Public License
+	along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+import { PayloadAction, createSlice } from "@reduxjs/toolkit";
+import type { Checkable } from "../lib/form/types";
+import { useReducer } from "react";
+
+// https://immerjs.github.io/immer/installation#pick-your-immer-version
+import { enableMapSet } from "immer";
+enableMapSet();
+
+export interface ChecklistState {
+	entries: { [k: string]: Checkable },
+	selectedEntries: Set<string>,
+}
+
+const initialState: ChecklistState = {
+	entries: {},
+	selectedEntries: new Set(),
+};
+
+function initialHookState({
+	entries,
+	uniqueKey,
+	initialValue,
+}: {
+	entries: Checkable[],
+	uniqueKey: string,
+	initialValue: boolean,
+}): ChecklistState {
+	const selectedEntries = new Set<string>();
+	const mappedEntries = Object.fromEntries(
+		entries.map((entry) => {
+			const key = entry[uniqueKey];
+			const checked = entry.checked ?? initialValue;
+			
+			if (checked) {
+				selectedEntries.add(key);
+			} else {
+				selectedEntries.delete(key);
+			}
+		
+			return [ key, { ...entry, key, checked } ];
+		})
+	);
+
+	return {
+		entries: mappedEntries,
+		selectedEntries
+	};
+}
+
+const checklistSlice = createSlice({
+	name: "checklist",
+	initialState, // not handled by slice itself
+	reducers: {
+		updateAll: (state, { payload: checked }: PayloadAction<boolean>) => {
+			const selectedEntries = new Set<string>();
+			const entries = Object.fromEntries(
+				Object.values(state.entries).map((entry) => {
+					if (checked) {
+						// Cheekily add this to selected
+						// entries while we're here.
+						selectedEntries.add(entry.key);
+					}
+
+					return [entry.key, { ...entry, checked } ];
+				})
+			);
+			
+			return { entries, selectedEntries };
+		},
+		update: (state, { payload: { key, value } }: PayloadAction<{key: string, value: Partial<Checkable>}>) => {
+			if (value.checked !== undefined) {
+				if (value.checked) {
+					state.selectedEntries.add(key);
+				} else {
+					state.selectedEntries.delete(key);
+				}
+			}
+
+			state.entries[key] = {
+				...state.entries[key],
+				...value
+			};
+		},
+		updateMultiple: (state, { payload }: PayloadAction<Array<[key: string, value: Partial<Checkable>]>>) => {						
+			payload.forEach(([ key, value ]) => {								
+				if (value.checked !== undefined) {
+					if (value.checked) {
+						state.selectedEntries.add(key);
+					} else {
+						state.selectedEntries.delete(key);
+					}
+				}
+
+				state.entries[key] = {
+					...state.entries[key],
+					...value
+				};
+			});
+		}
+	}
+});
+
+export const actions = checklistSlice.actions;
+
+/**
+ * useChecklistReducer wraps the react 'useReducer'
+ * hook with logic specific to the checklist reducer.
+ * 
+ * Use it in components where you need to keep track
+ * of checklist state.
+ * 
+ * To update it, use dispatch with the actions
+ * exported from this module.
+ * 
+ * @example
+ * 
+ * ```javascript
+ * // Start with one entry with "checked" set to "false".
+ * const initialEntries = [{ key: "some_key", id: "some_id", value: "some_value", checked: false }];
+ * const [state, dispatch] = useChecklistReducer(initialEntries, "id", false);
+ * 
+ * // Dispatch an action to set "checked" of all entries to "true".
+ * let checked = true;
+ * dispatch(actions.updateAll(checked));
+ * 
+ * // Will log `["some_id"]`
+ * console.log(state.selectedEntries)
+ * 
+ * // Will log `{ key: "some_key", id: "some_id", value: "some_value", checked: true }`
+ * console.log(state.entries["some_id"])
+ * ```
+ */
+export const useChecklistReducer = (entries: Checkable[], uniqueKey: string, initialValue: boolean) => {
+	return useReducer(
+		checklistSlice.reducer,
+		initialState,
+		(_) => initialHookState({ entries, uniqueKey, initialValue })
+	);
+};