Skip to content

Commit

Permalink
[bugfix/frontend] fix typo and other oddness in patchRemoteEmojis (#2281
Browse files Browse the repository at this point in the history 
)

* fix emoji test model

* found the bug!

* remove unused 'current' import

* comment useChecklistReducer

* wah

* lint

* fix cleaner tests
  • Loading branch information
@tsmethurst
tsmethurst authored Oct 21, 2023
1 parent 21a101e commit 9114c5c
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 137 deletions.
29 changes: 27 additions & 2 deletions internal/cleaner/emoji_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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),
)
}

Expand Down
4 changes: 2 additions & 2 deletions testrig/testmodels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
11 changes: 6 additions & 5 deletions web/source/settings/admin/emoji/remote/parse-from-toot.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,12 @@ function CopyEmojiForm({ localEmojiCodes, type, emojiList }) {
{
changedOnly: false,
onFinish: ({ data }) => {
if (data != undefined) {
form.selectedEmoji.updateMultiple(
// uncheck all successfully processed emoji
data.map(([id]) => [id, { checked: false }])
);
if (data) {
// uncheck all successfully processed emoji
const processed = data.map((emoji) => {
return [emoji.id, { checked: false }];
});
form.selectedEmoji.updateMultiple(processed);
}
}
}
Expand Down
124 changes: 11 additions & 113 deletions web/source/settings/lib/form/check-list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,122 +18,25 @@
*/

import {
useReducer,
useRef,
useEffect,
useCallback,
useMemo,
} from "react";

import { PayloadAction, createSlice } from "@reduxjs/toolkit";

import type {
Checkable,
ChecklistInputHook,
CreateHookNames,
HookOpts,
} from "./types";

// https://immerjs.github.io/immer/installation#pick-your-immer-version
import { enableMapSet } from "immer";
enableMapSet();

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
};
}
import {
useChecklistReducer,
actions,
} from "../../redux/checklist";

const _default: { [k: string]: Checkable } = {};

export default function useCheckListInput(
/* eslint-disable no-unused-vars */
{ name, Name }: CreateHookNames,
Expand All @@ -143,12 +46,7 @@ export default function useCheckListInput(
initialValue = false,
}: HookOpts<boolean>
): ChecklistInputHook {
const [state, dispatch] = useReducer(
reducer,
initialState,
(_) => initialHookState({ entries, uniqueKey, initialValue }) // initial state
);

const [state, dispatch] = useChecklistReducer(entries, uniqueKey, initialValue);
const toggleAllRef = useRef<any>(null);

useEffect(() => {
Expand All @@ -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(() => {
Expand Down Expand Up @@ -215,5 +113,5 @@ export default function useCheckListInput(
onChange: toggleAll
}
});
}, [state, reset, name, onChange, updateMultiple]);
}, [state, reset, name, onChange, updateMultiple, dispatch]);
}
2 changes: 1 addition & 1 deletion web/source/settings/lib/form/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
56 changes: 42 additions & 14 deletions web/source/settings/lib/query/admin/custom-emoji/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,16 @@ 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}]`
},
},
};
};
}

// Return our ID'd
Expand All @@ -222,14 +225,18 @@ const extended = gtsApi.injectEndpoints({

patchRemoteEmojis: build.mutation({
async queryFn({ action, ...formData }, _api, _extraOpts, fetchWithBQ) {
const data: CustomEmoji[] = [];
const errors: FetchBaseQueryError[] = [];

formData.selectEmoji.forEach(async(emoji: CustomEmoji) => {
let body = {
const selectedEmoji: CustomEmoji[] = formData.selectedEmoji;

// Map function to get a promise
// 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") {
Expand All @@ -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 {
// Got it!
data.push(emojiRes.data as CustomEmoji);
return null;
}
});

// 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}]`
},
},
};
}
Expand Down
Loading

0 comments on commit 9114c5c

Please sign in to comment.