diff --git a/backend/coreapp/tests/test_scratch.py b/backend/coreapp/tests/test_scratch.py index 49beba8d8..caf13f48b 100644 --- a/backend/coreapp/tests/test_scratch.py +++ b/backend/coreapp/tests/test_scratch.py @@ -484,7 +484,7 @@ def test_fork_scratch(self) -> None: ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - new_claim_token = response.json()["claim_token"] + self.assertNotIn("claim_token", response.json()) new_slug = response.json()["slug"] scratch = Scratch.objects.get(slug=slug) @@ -496,9 +496,10 @@ def test_fork_scratch(self) -> None: # Make sure the name carried over to the fork self.assertEqual(scratch.name, fork.name) - # Ensure the new scratch has a (unique) claim token - self.assertIsNotNone(new_claim_token) - self.assertIsNot(new_claim_token, old_claim_token) + # Ensure the new scratch is owned by the profile that forked it. + self.assertEqual(fork.owner_id, self.client.session["profile_id"]) + self.assertEqual(response.json()["owner"]["id"], fork.owner_id) + self.assertNotEqual(fork.claim_token, old_claim_token) class ScratchDetailTests(BaseTestCase): diff --git a/backend/coreapp/views/scratch.py b/backend/coreapp/views/scratch.py index b2a7a0ce3..619f7d238 100644 --- a/backend/coreapp/views/scratch.py +++ b/backend/coreapp/views/scratch.py @@ -505,6 +505,7 @@ def fork(self, request: Request, pk: str) -> Response: libraries = [Library(**lib) for lib in ser.validated_data["libraries"]] new_scratch = ser.save( parent=parent, + owner=request.profile, target_assembly=parent.target_assembly, platform=parent.platform, libraries=libraries, @@ -513,7 +514,7 @@ def fork(self, request: Request, pk: str) -> Response: compile_scratch_update_score(new_scratch) return Response( - ClaimableScratchSerializer(new_scratch, context={"request": request}).data, + ScratchSerializer(new_scratch, context={"request": request}).data, status=status.HTTP_201_CREATED, ) diff --git a/frontend/src/app/scratch/[slug]/ScratchEditor.tsx b/frontend/src/app/scratch/[slug]/ScratchEditor.tsx index 6bda1a0f0..5805d52d3 100644 --- a/frontend/src/app/scratch/[slug]/ScratchEditor.tsx +++ b/frontend/src/app/scratch/[slug]/ScratchEditor.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import useSWR, { type Middleware, SWRConfig } from "swr"; @@ -9,6 +9,7 @@ import useWarnBeforeScratchUnload from "@/components/Scratch/hooks/useWarnBefore import SetPageTitle from "@/components/SetPageTitle"; import * as api from "@/lib/api"; import { scratchUrl } from "@/lib/api/urls"; +import { ignoreNextWarnBeforeUnload } from "@/lib/hooks"; function ScratchPageTitle({ scratch }: { scratch: api.Scratch }) { const isSaved = api.useIsScratchSaved(scratch); @@ -26,10 +27,12 @@ function ScratchEditorInner({ offline, }: Props) { const [scratch, setScratch] = useState(initialScratch); + const [isDeleting, setIsDeleting] = useState(false); + const isDeletingRef = useRef(false); const currentScratchUrl = scratchUrl(scratch); const initialScratchUrl = scratchUrl(initialScratch); - useWarnBeforeScratchUnload(scratch); + useWarnBeforeScratchUnload(scratch, !isDeleting); // If the static props scratch changes (i.e. router push / page redirect), reset `scratch`. useEffect(() => { @@ -46,7 +49,7 @@ function ScratchEditorInner({ // 4. Notice the scratch owner (in the About panel) has changed to your newly-logged-in user const ownerMayChange = !scratch.owner || scratch.owner.is_anonymous; const cached = useSWR( - ownerMayChange && currentScratchUrl, + ownerMayChange && !isDeleting && currentScratchUrl, api.get, )?.data; useEffect(() => { @@ -66,29 +69,72 @@ function ScratchEditorInner({ // was updated, so the originally-loaded initialScratch prop becomes stale. // https://github.com/decompme/decomp.me/issues/711 useEffect(() => { + if (isDeleting) return; + let isCurrent = true; - api.get(initialScratchUrl).then((updatedScratch: api.Scratch) => { - if (!isCurrent) return; + api.get(initialScratchUrl) + .then((updatedScratch: api.Scratch) => { + if (!isCurrent) return; + + const updateTime = new Date(updatedScratch.last_updated); - const updateTime = new Date(updatedScratch.last_updated); + setScratch((scratch) => { + const scratchTime = new Date(scratch.last_updated); - setScratch((scratch) => { - const scratchTime = new Date(scratch.last_updated); + if (scratchTime < updateTime) { + console.info( + "Client got updated scratch", + updatedScratch, + ); + return updatedScratch; + } - if (scratchTime < updateTime) { - console.info("Client got updated scratch", updatedScratch); - return updatedScratch; + return scratch; + }); + }) + .catch((error) => { + if (!isCurrent) return; + + if ( + error instanceof api.ResponseError && + error.status === 404 && + isDeletingRef.current + ) { + return; } - return scratch; + throw error; }); - }); return () => { isCurrent = false; }; - }, [initialScratchUrl]); + }, [initialScratchUrl, isDeleting]); + + const deleteScratch = useCallback(async () => { + isDeletingRef.current = true; + setIsDeleting(true); + + try { + await api.delete_(currentScratchUrl, {}); + } catch (error) { + isDeletingRef.current = false; + setIsDeleting(false); + throw error; + } + + ignoreNextWarnBeforeUnload(); + window.location.href = scratch.project ? `/${scratch.project}` : "/"; + }, [currentScratchUrl, scratch.project]); + + if (isDeleting) { + return ( +
+
Deleting scratch...
+
+ ); + } return ( <> @@ -103,6 +149,7 @@ function ScratchEditorInner({ return { ...scratch, ...partial }; }); }} + deleteScratch={deleteScratch} offline={offline} /> diff --git a/frontend/src/components/Scratch/Scratch.tsx b/frontend/src/components/Scratch/Scratch.tsx index a8c505868..fa4768fbb 100644 --- a/frontend/src/components/Scratch/Scratch.tsx +++ b/frontend/src/components/Scratch/Scratch.tsx @@ -180,6 +180,7 @@ function applyDefaultDiffTab( export type Props = { scratch: Readonly; onChange: (scratch: Partial) => void; + deleteScratch: () => Promise; parentScratch?: api.Scratch; initialCompilation?: Readonly; offline: boolean; @@ -188,6 +189,7 @@ export type Props = { export default function Scratch({ scratch, onChange, + deleteScratch, parentScratch, initialCompilation, offline, @@ -564,6 +566,7 @@ export default function Scratch({ scratch={scratch} setScratch={setScratch} saveCallback={saveCallback} + deleteScratch={deleteScratch} setDecompilationTabEnabled={setDecompilationTabEnabled} /> {matchProgressBarEnabledSetting && ( diff --git a/frontend/src/components/Scratch/ScratchToolbar.module.scss b/frontend/src/components/Scratch/ScratchToolbar.module.scss index e1f7733d0..c1a1bfe16 100644 --- a/frontend/src/components/Scratch/ScratchToolbar.module.scss +++ b/frontend/src/components/Scratch/ScratchToolbar.module.scss @@ -117,6 +117,10 @@ nav.breadcrumbs { color: var(--g2000); background: var(--g400); } + + @media (width >= 768px) { + min-width: 64px; + } } } } diff --git a/frontend/src/components/Scratch/ScratchToolbar.tsx b/frontend/src/components/Scratch/ScratchToolbar.tsx index 8ddfc9429..c5f9050e7 100644 --- a/frontend/src/components/Scratch/ScratchToolbar.tsx +++ b/frontend/src/components/Scratch/ScratchToolbar.tsx @@ -9,6 +9,7 @@ import { } from "react"; import { + CheckIcon, DownloadIcon, FileIcon, IterationsIcon, @@ -32,9 +33,6 @@ import PlatformLink from "../PlatformLink"; import { SpecialKey, useShortcut } from "../Shortcut"; import UserAvatar from "../user/UserAvatar"; -import useFuzzySaveCallback, { - FuzzySaveAction, -} from "./hooks/useFuzzySaveCallback"; import styles from "./ScratchToolbar.module.scss"; const ACTIVE_MS = 1000 * 60; @@ -52,12 +50,6 @@ function exportScratchZip(scratch: api.Scratch) { a.click(); } -async function deleteScratch(scratch: api.Scratch) { - await api.delete_(scratchUrl(scratch), {}); - - window.location.href = scratch.project ? `/${scratch.project}` : "/"; -} - function EditTimeAgo({ date }: { date: string }) { const isActive = Date.now() - new Date(date).getTime() < ACTIVE_MS; @@ -191,29 +183,47 @@ function Actions({ scratch, setScratch, saveCallback, + deleteScratch, setDecompilationTabEnabled, }: Props) { const userIsYou = api.useUserIsYou(); const forkScratch = api.useForkScratchAndGo(scratch); - const [fuzzySaveAction, fuzzySaveScratch] = useFuzzySaveCallback( - scratch, - setScratch, - ); + const saveScratchRequest = api.useSaveScratch(scratch); const [isSaving, setIsSaving] = useState(false); const [isForking, setIsForking] = useState(false); - const canSave = scratch.owner && userIsYou(scratch.owner); + const canSave = !!(scratch.owner && userIsYou(scratch.owner)); + const isSaved = api.useIsScratchSaved(scratch); const platform = api.usePlatform(scratch.platform); - const fuzzyShortcut = useShortcut( - [SpecialKey.CTRL_COMMAND, "S"], - async () => { - setIsSaving(true); - await fuzzySaveScratch(); + const saveScratch = async () => { + if (!canSave || isSaved || isSaving) return; + + setIsSaving(true); + try { + setScratch(await saveScratchRequest()); + saveCallback(); + } finally { setIsSaving(false); + } + }; + + const forkCurrentScratch = async () => { + if (isForking) return; + + setIsForking(true); + try { + await forkScratch(); saveCallback(); - }, + } finally { + setIsForking(false); + } + }; + + const fuzzyShortcut = useShortcut( + [SpecialKey.CTRL_COMMAND, "S"], + canSave ? saveScratch : forkCurrentScratch, ); const compileShortcut = useShortcut([SpecialKey.CTRL_COMMAND, "J"], () => { @@ -227,35 +237,23 @@ function Actions({
    • + {canSave && ( +
  • + : } + /> +
    • + )}
  • { - setIsSaving(true); - await fuzzySaveScratch(); - setIsSaving(false); - saveCallback(); - }} - disabled={!canSave || isSaving} - title={fuzzyShortcut} - text={"Save"} - icon={} - /> -
    • -
  • - { - setIsForking(true); - await forkScratch(); - setIsForking(false); - saveCallback(); - }} + onClick={forkCurrentScratch} disabled={isForking} - title={ - fuzzySaveAction === FuzzySaveAction.FORK - ? fuzzyShortcut - : undefined - } - text="Fork" + title={!canSave ? fuzzyShortcut : undefined} + text={!canSave ? "Fork to save" : "Fork"} icon={} />
    • @@ -269,7 +267,7 @@ function Actions({ "Are you sure you want to delete this scratch? This action cannot be undone.", ) ) { - deleteScratch(scratch); + void deleteScratch(); } }} text="Delete" @@ -343,6 +341,7 @@ export type Props = { scratch: Readonly; setScratch: (scratch: Partial) => void; saveCallback: () => void; + deleteScratch: () => Promise; setDecompilationTabEnabled: (enabled: boolean) => void; }; diff --git a/frontend/src/components/Scratch/hooks/useFuzzySaveCallback.ts b/frontend/src/components/Scratch/hooks/useFuzzySaveCallback.ts deleted file mode 100644 index 26ba1de22..000000000 --- a/frontend/src/components/Scratch/hooks/useFuzzySaveCallback.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { useCallback } from "react"; - -import * as api from "@/lib/api"; - -export enum FuzzySaveAction { - SAVE = 0, - FORK = 1, - NONE = 2, -} - -export default function useFuzzySaveCallback( - scratch: api.Scratch, - setScratch: (partial: Partial) => void, -): [FuzzySaveAction, () => Promise] { - const saveScratch = api.useSaveScratch(scratch); - const forkScratch = api.useForkScratchAndGo(scratch); - const userIsYou = api.useUserIsYou(); - - let action = FuzzySaveAction.NONE; - if (userIsYou(scratch.owner)) { - action = FuzzySaveAction.SAVE; - } else { - action = FuzzySaveAction.FORK; - } - - return [ - action, - useCallback(async () => { - switch (action) { - case FuzzySaveAction.SAVE: - setScratch(await saveScratch()); - break; - case FuzzySaveAction.FORK: - await forkScratch(); - break; - } - }, [action, forkScratch, saveScratch, setScratch]), - ]; -} diff --git a/frontend/src/components/Scratch/hooks/useWarnBeforeScratchUnload.ts b/frontend/src/components/Scratch/hooks/useWarnBeforeScratchUnload.ts index 243596403..f25bc9d1d 100644 --- a/frontend/src/components/Scratch/hooks/useWarnBeforeScratchUnload.ts +++ b/frontend/src/components/Scratch/hooks/useWarnBeforeScratchUnload.ts @@ -1,12 +1,15 @@ import * as api from "@/lib/api"; import { useWarnBeforeUnload } from "@/lib/hooks"; -export default function useWarnBeforeScratchUnload(scratch: api.Scratch) { +export default function useWarnBeforeScratchUnload( + scratch: api.Scratch, + enabled = true, +) { const userIsYou = api.useUserIsYou(); - const isSaved = api.useIsScratchSaved(scratch); + const isSaved = api.useIsScratchSaved(scratch, enabled); useWarnBeforeUnload( - !isSaved, + enabled && !isSaved, userIsYou(scratch.owner) ? "You have not saved your changes to this scratch. Discard changes?" : "You have edited this scratch but not saved it in a fork. Discard changes?", diff --git a/frontend/src/components/TimeAgo.tsx b/frontend/src/components/TimeAgo.tsx index 5c0498ec7..47cb5410c 100644 --- a/frontend/src/components/TimeAgo.tsx +++ b/frontend/src/components/TimeAgo.tsx @@ -1,3 +1,7 @@ +"use client"; + +import { useEffect, useState } from "react"; + import DateObject from "react-date-object"; import ReactTimeAgo from "react-timeago"; @@ -12,7 +16,22 @@ export default function TimeAgo({ }: { date: string; }) { + const [mounted, setMounted] = useState(false); + const dateTime = new Date(date).toISOString(); + const fallbackTitle = dateTime.substring(0, 19).replace("T", " "); const title = formatDateString(date); + useEffect(() => { + setMounted(true); + }, []); + + if (!mounted) { + return ( + + ); + } + return ; } diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 702862718..c1aaaf288 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -86,10 +86,14 @@ export function useUserIsYou(): ( ); // eslint-disable-line react-hooks/exhaustive-deps } -export function useSavedScratch(scratch: Scratch): Scratch { - const { data: savedScratch, error } = useSWR(scratchUrl(scratch), get, { - fallbackData: scratch, // No loading state, just use the local scratch - }); +export function useSavedScratch(scratch: Scratch, enabled = true): Scratch { + const { data: savedScratch, error } = useSWR( + enabled ? scratchUrl(scratch) : null, + get, + { + fallbackData: scratch, // No loading state, just use the local scratch + }, + ); if (error) throw error; @@ -142,7 +146,12 @@ export async function claimScratch(scratch: ClaimableScratch): Promise { export async function forkScratch(parent: TerseScratch): Promise { const scratch = await post(`${scratchUrl(parent)}/fork`, parent); - await claimScratch(scratch); + + if (scratch.owner) { + await mutate("/user", scratch.owner, { revalidate: false }); + } + await mutate(scratchUrl(scratch), scratch, { revalidate: false }); + return scratch; } @@ -157,8 +166,8 @@ export function useForkScratchAndGo(parent: TerseScratch): () => Promise { }, [parent, router]); } -export function useIsScratchSaved(scratch: Scratch): boolean { - const saved = useSavedScratch(scratch); +export function useIsScratchSaved(scratch: Scratch, enabled = true): boolean { + const saved = useSavedScratch(scratch, enabled); return isScratchSaved(scratch, saved); } diff --git a/frontend/src/lib/hooks.ts b/frontend/src/lib/hooks.ts index 009f99824..060739fba 100644 --- a/frontend/src/lib/hooks.ts +++ b/frontend/src/lib/hooks.ts @@ -61,7 +61,7 @@ export function useWarnBeforeUnload( enabledRef.current = enabled; messageRef.current = message; - useEffect(() => { + useLayoutEffect(() => { navigationWarning.enabled = enabledRef.current; navigationWarning.message = messageRef.current; let shouldLeaveAfterPop = false;