Public Access
feat(backlog): save only the changed fields
This commit is contained in:
1 parent
9ddc50f819
commit
628920a006
6 files changed
+133
-83
No files matched your search
@@ -10,7 +10,7 @@ import {Select, SelectContent, SelectItem, SelectTrigger, SelectValue} from '@/c
|
|||||||
import {Textarea} from '@/components/ui/textarea';
|
import {Textarea} from '@/components/ui/textarea';
|
||||||
import {cn} from '@/lib/utils';
|
import {cn} from '@/lib/utils';
|
||||||
import {STATUS_META} from './GameStatusBadge.jsx';
|
import {STATUS_META} from './GameStatusBadge.jsx';
|
||||||
import {NO_PRIORITY, saveBacklogEntry, toBacklogRequest} from '../utils/backlog.js';
|
import {changedFields, NO_PRIORITY, saveBacklogEntry} from '../utils/backlog.js';
|
||||||
import {GAME_PRIORITIES, GAME_STATUSES, MAX_NOTES_LENGTH} from '../utils/limits.js';
|
import {GAME_PRIORITIES, GAME_STATUSES, MAX_NOTES_LENGTH} from '../utils/limits.js';
|
||||||
import useUnsavedChanges from '../hooks/useUnsavedChanges.js';
|
import useUnsavedChanges from '../hooks/useUnsavedChanges.js';
|
||||||
|
|
||||||
@@ -33,14 +33,15 @@ export default function BacklogCard({game, onSaved}) {
|
|||||||
}, [game.status]);
|
}, [game.status]);
|
||||||
const [busy, setBusy] = useState(false);
|
const [busy, setBusy] = useState(false);
|
||||||
|
|
||||||
const dirty = JSON.stringify(form) !== JSON.stringify(saved);
|
const changes = changedFields(saved, form);
|
||||||
|
const dirty = Object.keys(changes).length > 0;
|
||||||
useUnsavedChanges(dirty);
|
useUnsavedChanges(dirty);
|
||||||
const set = (field) => (value) => setForm((f) => ({...f, [field]: value}));
|
const set = (field) => (value) => setForm((f) => ({...f, [field]: value}));
|
||||||
|
|
||||||
const save = async () => {
|
const save = async () => {
|
||||||
setBusy(true);
|
setBusy(true);
|
||||||
try {
|
try {
|
||||||
const response = await saveBacklogEntry(game.appid, toBacklogRequest(form));
|
const response = await saveBacklogEntry(game.appid, changes);
|
||||||
if (response.status === 401) return;
|
if (response.status === 401) return;
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
toast.error(t('BACKLOG.SAVE_FAILED'));
|
toast.error(t('BACKLOG.SAVE_FAILED'));
|
||||||
@@ -67,72 +68,75 @@ export default function BacklogCard({game, onSaved}) {
|
|||||||
{t('BACKLOG.TITLE')}
|
{t('BACKLOG.TITLE')}
|
||||||
</h2>
|
</h2>
|
||||||
|
|
||||||
<div className="grid gap-5 sm:grid-cols-3">
|
{/* Locked while saving: the answer replaces the form, so an edit made meanwhile would be lost. */}
|
||||||
<div className="space-y-2">
|
<fieldset disabled={busy} className="contents">
|
||||||
<Label htmlFor="backlog-status">{t('BACKLOG.STATUS_LABEL')}</Label>
|
<div className="grid gap-5 sm:grid-cols-3">
|
||||||
<Select value={form.status} onValueChange={set('status')}>
|
<div className="space-y-2">
|
||||||
<SelectTrigger id="backlog-status" className="w-full"><SelectValue/></SelectTrigger>
|
<Label htmlFor="backlog-status">{t('BACKLOG.STATUS_LABEL')}</Label>
|
||||||
<SelectContent>
|
<Select value={form.status} onValueChange={set('status')}>
|
||||||
{GAME_STATUSES.map((s) => {
|
<SelectTrigger id="backlog-status" className="w-full"><SelectValue/></SelectTrigger>
|
||||||
const {icon: Icon, text} = STATUS_META[s];
|
<SelectContent>
|
||||||
return (
|
{GAME_STATUSES.map((s) => {
|
||||||
<SelectItem key={s} value={s}>
|
const {icon: Icon, text} = STATUS_META[s];
|
||||||
<Icon className={text}/>
|
return (
|
||||||
{t(`BACKLOG.STATUS.${s}`)}
|
<SelectItem key={s} value={s}>
|
||||||
</SelectItem>
|
<Icon className={text}/>
|
||||||
);
|
{t(`BACKLOG.STATUS.${s}`)}
|
||||||
})}
|
</SelectItem>
|
||||||
</SelectContent>
|
);
|
||||||
</Select>
|
})}
|
||||||
</div>
|
</SelectContent>
|
||||||
|
</Select>
|
||||||
|
</div>
|
||||||
|
|
||||||
<div className="space-y-2">
|
<div className="space-y-2">
|
||||||
<Label htmlFor="backlog-priority">{t('BACKLOG.PRIORITY_LABEL')}</Label>
|
<Label htmlFor="backlog-priority">{t('BACKLOG.PRIORITY_LABEL')}</Label>
|
||||||
<Select value={form.priority} onValueChange={set('priority')}>
|
<Select value={form.priority} onValueChange={set('priority')}>
|
||||||
<SelectTrigger id="backlog-priority" className="w-full"><SelectValue/></SelectTrigger>
|
<SelectTrigger id="backlog-priority" className="w-full"><SelectValue/></SelectTrigger>
|
||||||
<SelectContent>
|
<SelectContent>
|
||||||
<SelectItem value={NO_PRIORITY}>{t('BACKLOG.PRIORITY.NONE')}</SelectItem>
|
<SelectItem value={NO_PRIORITY}>{t('BACKLOG.PRIORITY.NONE')}</SelectItem>
|
||||||
{GAME_PRIORITIES.map((p) => (
|
{GAME_PRIORITIES.map((p) => (
|
||||||
<SelectItem key={p} value={p}>{t(`BACKLOG.PRIORITY.${p}`)}</SelectItem>
|
<SelectItem key={p} value={p}>{t(`BACKLOG.PRIORITY.${p}`)}</SelectItem>
|
||||||
|
))}
|
||||||
|
</SelectContent>
|
||||||
|
</Select>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div className="space-y-2">
|
||||||
|
<Label id="backlog-rating">{t('BACKLOG.RATING_LABEL')}</Label>
|
||||||
|
<div role="group" aria-labelledby="backlog-rating" className="flex h-9 items-center gap-1">
|
||||||
|
{[1, 2, 3, 4, 5].map((n) => (
|
||||||
|
<button
|
||||||
|
key={n}
|
||||||
|
type="button"
|
||||||
|
aria-label={t('BACKLOG.RATING_STARS', {count: n})}
|
||||||
|
aria-pressed={form.rating === n}
|
||||||
|
onClick={() => set('rating')(form.rating === n ? 0 : n)}
|
||||||
|
className="rounded-sm p-0.5 outline-none focus-visible:ring-[3px] focus-visible:ring-ring/50"
|
||||||
|
>
|
||||||
|
<Star className={cn('size-6 transition-colors',
|
||||||
|
n <= form.rating ? 'fill-amber-400 text-amber-400' : 'text-muted-foreground/40 hover:text-amber-400')}/>
|
||||||
|
</button>
|
||||||
))}
|
))}
|
||||||
</SelectContent>
|
</div>
|
||||||
</Select>
|
|
||||||
</div>
|
|
||||||
|
|
||||||
<div className="space-y-2">
|
|
||||||
<Label id="backlog-rating">{t('BACKLOG.RATING_LABEL')}</Label>
|
|
||||||
<div role="group" aria-labelledby="backlog-rating" className="flex h-9 items-center gap-1">
|
|
||||||
{[1, 2, 3, 4, 5].map((n) => (
|
|
||||||
<button
|
|
||||||
key={n}
|
|
||||||
type="button"
|
|
||||||
aria-label={t('BACKLOG.RATING_STARS', {count: n})}
|
|
||||||
aria-pressed={form.rating === n}
|
|
||||||
onClick={() => set('rating')(form.rating === n ? 0 : n)}
|
|
||||||
className="rounded-sm p-0.5 outline-none focus-visible:ring-[3px] focus-visible:ring-ring/50"
|
|
||||||
>
|
|
||||||
<Star className={cn('size-6 transition-colors',
|
|
||||||
n <= form.rating ? 'fill-amber-400 text-amber-400' : 'text-muted-foreground/40 hover:text-amber-400')}/>
|
|
||||||
</button>
|
|
||||||
))}
|
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
|
||||||
|
|
||||||
<div className="space-y-2">
|
<div className="space-y-2">
|
||||||
<div className="flex items-baseline justify-between">
|
<div className="flex items-baseline justify-between">
|
||||||
<Label htmlFor="backlog-notes">{t('BACKLOG.NOTES_LABEL')}</Label>
|
<Label htmlFor="backlog-notes">{t('BACKLOG.NOTES_LABEL')}</Label>
|
||||||
<span className="text-xs text-muted-foreground tabular-nums">{form.notes.length} / {MAX_NOTES_LENGTH}</span>
|
<span className="text-xs text-muted-foreground tabular-nums">{form.notes.length} / {MAX_NOTES_LENGTH}</span>
|
||||||
|
</div>
|
||||||
|
<Textarea
|
||||||
|
id="backlog-notes"
|
||||||
|
rows={3}
|
||||||
|
maxLength={MAX_NOTES_LENGTH}
|
||||||
|
placeholder={t('BACKLOG.NOTES_PLACEHOLDER')}
|
||||||
|
value={form.notes}
|
||||||
|
onChange={(e) => set('notes')(e.target.value)}
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
<Textarea
|
</fieldset>
|
||||||
id="backlog-notes"
|
|
||||||
rows={3}
|
|
||||||
maxLength={MAX_NOTES_LENGTH}
|
|
||||||
placeholder={t('BACKLOG.NOTES_PLACEHOLDER')}
|
|
||||||
value={form.notes}
|
|
||||||
onChange={(e) => set('notes')(e.target.value)}
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
|
|
||||||
<div className="flex justify-end gap-2">
|
<div className="flex justify-end gap-2">
|
||||||
<Button variant="ghost" onClick={() => setForm(saved)} disabled={!dirty || busy}>
|
<Button variant="ghost" onClick={() => setForm(saved)} disabled={!dirty || busy}>
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
// @vitest-environment jsdom
|
// @vitest-environment jsdom
|
||||||
import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
|
import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
|
||||||
import {cleanup, fireEvent, render, screen, waitFor} from '@testing-library/react';
|
import {act, cleanup, fireEvent, render, screen, waitFor} from '@testing-library/react';
|
||||||
import {MemoryRouter} from 'react-router-dom';
|
import {MemoryRouter} from 'react-router-dom';
|
||||||
import {fetchRoutes, json} from '../test/fetchRoutes.js';
|
import {fetchRoutes, json} from '../test/fetchRoutes.js';
|
||||||
import BacklogCard from './BacklogCard.jsx';
|
import BacklogCard from './BacklogCard.jsx';
|
||||||
@@ -14,7 +14,8 @@ const saveButton = () => screen.getByRole('button', {name: 'BACKLOG.SAVE'});
|
|||||||
describe('BacklogCard', () => {
|
describe('BacklogCard', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
localStorage.setItem('tbp_session', '1');
|
localStorage.setItem('tbp_session', '1');
|
||||||
fetchRoutes({'PUT /api/library/games/220/backlog': (url, init) => json(200, JSON.parse(init.body))});
|
// Core answers the whole entry; the fields not sent come from the server.
|
||||||
|
fetchRoutes({'PATCH /api/library/games/220/backlog': (url, init) => json(200, {...game, ...JSON.parse(init.body)})});
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -42,8 +43,44 @@ describe('BacklogCard', () => {
|
|||||||
await waitFor(() => expect(onSaved).toHaveBeenCalled());
|
await waitFor(() => expect(onSaved).toHaveBeenCalled());
|
||||||
const [url, init] = fetch.mock.calls[0];
|
const [url, init] = fetch.mock.calls[0];
|
||||||
expect(url).toMatch(/\/api\/library\/games\/220\/backlog$/);
|
expect(url).toMatch(/\/api\/library\/games\/220\/backlog$/);
|
||||||
expect(init.method).toBe('PUT');
|
expect(init.method).toBe('PATCH');
|
||||||
expect(JSON.parse(init.body)).toEqual({status: 'PLAYING', rating: null, notes: null, priority: null});
|
expect(JSON.parse(init.body)).toEqual({rating: null, notes: null});
|
||||||
|
expect(saveButton().disabled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('sends only the changed notes, so an older tab does not overwrite the other fields', async () => {
|
||||||
|
const onSaved = vi.fn();
|
||||||
|
render(<MemoryRouter><BacklogCard game={game} onSaved={onSaved}/></MemoryRouter>);
|
||||||
|
|
||||||
|
fireEvent.change(screen.getByRole('textbox'), {target: {value: 'new notes'}});
|
||||||
|
fireEvent.click(saveButton());
|
||||||
|
|
||||||
|
await waitFor(() => expect(onSaved).toHaveBeenCalled());
|
||||||
|
expect(JSON.parse(fetch.mock.calls[0][1].body)).toEqual({notes: 'new notes'});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('locks the fields while the save runs, so nothing typed then is lost', async () => {
|
||||||
|
let answer;
|
||||||
|
fetchRoutes({'PATCH /api/library/games/220/backlog': (url, init) => new Promise((resolve) => {
|
||||||
|
answer = () => resolve(json(200, {...game, ...JSON.parse(init.body)}));
|
||||||
|
})});
|
||||||
|
render(<MemoryRouter><BacklogCard game={game}/></MemoryRouter>);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', {name: 'BACKLOG.RATING_STARS:5'}));
|
||||||
|
fireEvent.click(saveButton());
|
||||||
|
|
||||||
|
// :disabled, not .disabled: the fieldset locks them, not their own attribute.
|
||||||
|
expect(screen.getByRole('textbox').matches(':disabled')).toBe(true);
|
||||||
|
expect(screen.getByRole('button', {name: 'BACKLOG.RATING_STARS:1'}).matches(':disabled')).toBe(true);
|
||||||
|
await act(async () => answer());
|
||||||
|
expect(screen.getByRole('textbox').matches(':disabled')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not count blank notes instead of none as a change', () => {
|
||||||
|
render(<MemoryRouter><BacklogCard game={{...game, notes: null}}/></MemoryRouter>);
|
||||||
|
|
||||||
|
fireEvent.change(screen.getByRole('textbox'), {target: {value: ' '}});
|
||||||
|
|
||||||
expect(saveButton().disabled).toBe(true);
|
expect(saveButton().disabled).toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -5,7 +5,7 @@ import {Gamepad2, Loader2} from 'lucide-react';
|
|||||||
|
|
||||||
import {Button} from '@/components/ui/button';
|
import {Button} from '@/components/ui/button';
|
||||||
import {Card} from '@/components/ui/card';
|
import {Card} from '@/components/ui/card';
|
||||||
import {saveBacklogEntry} from '../utils/backlog.js';
|
import {saveGameStatus} from '../utils/backlog.js';
|
||||||
import {formatPlaytime} from '../utils/games.js';
|
import {formatPlaytime} from '../utils/games.js';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -22,9 +22,7 @@ export default function PlayingHint({game, onChanged}) {
|
|||||||
const setPlaying = async () => {
|
const setPlaying = async () => {
|
||||||
setBusy(true);
|
setBusy(true);
|
||||||
try {
|
try {
|
||||||
const response = await saveBacklogEntry(game.appid, {
|
const response = await saveGameStatus(game.appid, 'PLAYING');
|
||||||
status: 'PLAYING', rating: game.rating ?? null, notes: game.notes ?? null, priority: game.priority ?? null,
|
|
||||||
});
|
|
||||||
if (response.status === 401) return;
|
if (response.status === 401) return;
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
toast.error(t('BACKLOG.SAVE_FAILED'));
|
toast.error(t('BACKLOG.SAVE_FAILED'));
|
||||||
|
|||||||
@@ -14,7 +14,8 @@ const renderHint = (props) => render(<MemoryRouter><PlayingHint game={game} onCh
|
|||||||
describe('PlayingHint', () => {
|
describe('PlayingHint', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
localStorage.setItem('tbp_session', '1');
|
localStorage.setItem('tbp_session', '1');
|
||||||
fetchRoutes({'PUT /api/library/games/220/backlog': (url, init) => json(200, JSON.parse(init.body))});
|
// Core changes only the status and answers the whole entry.
|
||||||
|
fetchRoutes({'PUT /api/library/games/220/status': (url, init) => json(200, {...game, ...JSON.parse(init.body)})});
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -31,15 +32,15 @@ describe('PlayingHint', () => {
|
|||||||
expect(screen.queryByRole('button')).toBeNull();
|
expect(screen.queryByRole('button')).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('sets the game to PLAYING and keeps the other fields', async () => {
|
it('sends only the status, so Core keeps the other fields', async () => {
|
||||||
const onChanged = vi.fn();
|
const onChanged = vi.fn();
|
||||||
renderHint({onChanged});
|
renderHint({onChanged});
|
||||||
|
|
||||||
fireEvent.click(screen.getByRole('button', {name: 'GAME_DETAIL.SET_PLAYING'}));
|
fireEvent.click(screen.getByRole('button', {name: 'GAME_DETAIL.SET_PLAYING'}));
|
||||||
|
|
||||||
await waitFor(() => expect(onChanged).toHaveBeenCalledWith({status: 'PLAYING', rating: 3, notes: 'keep', priority: 'HIGH'}));
|
await waitFor(() => expect(onChanged).toHaveBeenCalledWith(expect.objectContaining({status: 'PLAYING'})));
|
||||||
const [url, init] = fetch.mock.calls[0];
|
const [url, init] = fetch.mock.calls[0];
|
||||||
expect(url).toMatch(/\/api\/library\/games\/220\/backlog$/);
|
expect(url).toMatch(/\/api\/library\/games\/220\/status$/);
|
||||||
expect(JSON.parse(init.body)).toEqual({status: 'PLAYING', rating: 3, notes: 'keep', priority: 'HIGH'});
|
expect(JSON.parse(init.body)).toEqual({status: 'PLAYING'});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -116,7 +116,8 @@ describe('GameDetail', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('keeps unsaved notes in the backlog card when the hint sets the game to playing', async () => {
|
it('keeps unsaved notes in the backlog card when the hint sets the game to playing', async () => {
|
||||||
answer(game, {'PUT /api/library/games/220/backlog': echo});
|
// Core changes only the status and answers the whole entry.
|
||||||
|
answer(game, {'PUT /api/library/games/220/status': (url, init) => json(200, {...game, ...JSON.parse(init.body)})});
|
||||||
renderDetail();
|
renderDetail();
|
||||||
const notes = await screen.findByLabelText('BACKLOG.NOTES_LABEL');
|
const notes = await screen.findByLabelText('BACKLOG.NOTES_LABEL');
|
||||||
fireEvent.change(notes, {target: {value: 'my draft'}});
|
fireEvent.change(notes, {target: {value: 'my draft'}});
|
||||||
|
|||||||
+15
-6
@@ -14,15 +14,24 @@ export const toBacklogRequest = ({status, rating, notes, priority}) => ({
|
|||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* PUT /api/library/games/{appid}/backlog. The caller handles the response status.
|
* The fields of `form` that differ from `saved`, in request form: what saveBacklogEntry sends.
|
||||||
*
|
|
||||||
* @param entry a backlog entry in request form, see toBacklogRequest
|
|
||||||
*/
|
*/
|
||||||
export const saveBacklogEntry = (appid, entry) =>
|
export const changedFields = (saved, form) => {
|
||||||
|
const before = toBacklogRequest(saved);
|
||||||
|
return Object.fromEntries(Object.entries(toBacklogRequest(form)).filter(([field, value]) => value !== before[field]));
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* PATCH /api/library/games/{appid}/backlog with only the changed fields, so an older tab keeps the newer ones; null
|
||||||
|
* clears a field. Answers the whole entry. The caller handles the response status.
|
||||||
|
*
|
||||||
|
* @param changes see changedFields
|
||||||
|
*/
|
||||||
|
export const saveBacklogEntry = (appid, changes) =>
|
||||||
apiFetch(`${import.meta.env.VITE_CORE_API_URL}/api/library/games/${appid}/backlog`, {
|
apiFetch(`${import.meta.env.VITE_CORE_API_URL}/api/library/games/${appid}/backlog`, {
|
||||||
method: 'PUT',
|
method: 'PATCH',
|
||||||
headers: {'Content-Type': 'application/json'},
|
headers: {'Content-Type': 'application/json'},
|
||||||
body: JSON.stringify(entry),
|
body: JSON.stringify(changes),
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in new issue
Block a user