mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-09-12 13:28:31 +00:00
fix(career): bookGig generation guard + 404-only pref revert (Creed F1/F2)
F1 (MEDIUM): bookGig had no request-generation guard. Rapid pref changes could let a stale response overwrite _ppGigProposal → user sees the wrong song set. Fix: _ppBookGen counter incremented on each request; response discarded unless gen === _ppBookGen at both the res.ok check and after json(). Stale-response driver test fails without the guard. F2 (LOW): every non-ok response reverted _ppGigTuningPref to 'any' and persisted it. A transient 500 would silently blow away the user's pref. Fix: pref reverted only on 404 (no-match case). Other errors notify but keep the pref. 500 driver test asserts pref stays 'drop' — fails without the fix. 404 driver still asserts revert to 'any'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2bM5jSbMskpdxm2CmuQVj
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
c46b6484bf
commit
c264a66e5d
@@ -49,6 +49,7 @@
|
|||||||
let _ppGigTuningNames = []; // cached distinct tuning names for the specific picker
|
let _ppGigTuningNames = []; // cached distinct tuning names for the specific picker
|
||||||
let _ppGigTuningHold = null; // holdAutoplay release fn — non-null = interstitial active
|
let _ppGigTuningHold = null; // holdAutoplay release fn — non-null = interstitial active
|
||||||
let _ppGigLastTuning = null; // tuning_name of the current gig song (for change detection)
|
let _ppGigLastTuning = null; // tuning_name of the current gig song (for change detection)
|
||||||
|
let _ppBookGen = 0; // generation counter — stale responses are discarded
|
||||||
|
|
||||||
function $(id) { return document.getElementById(id); }
|
function $(id) { return document.getElementById(id); }
|
||||||
|
|
||||||
@@ -1127,28 +1128,37 @@
|
|||||||
const p = (((_pp.instruments || {})[inst] || {}).passports || [])
|
const p = (((_pp.instruments || {})[inst] || {}).passports || [])
|
||||||
.find((x) => x.genre_key === gkey);
|
.find((x) => x.genre_key === gkey);
|
||||||
if (!p) return;
|
if (!p) return;
|
||||||
|
const gen = ++_ppBookGen; // F1: capture generation before await — stale responses discarded
|
||||||
try {
|
try {
|
||||||
const res = await fetch(`${API}/gigs/propose`, {
|
const res = await fetch(`${API}/gigs/propose`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
body: JSON.stringify({ instrument: inst, genre: p.genre, tuning_pref: _ppGigTuningPref }),
|
body: JSON.stringify({ instrument: inst, genre: p.genre, tuning_pref: _ppGigTuningPref }),
|
||||||
});
|
});
|
||||||
|
if (gen !== _ppBookGen) return; // stale response — a newer request supersedes this one
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
const err = await res.json().catch(() => ({}));
|
const err = await res.json().catch(() => ({}));
|
||||||
// Tuning filter yielded no songs → revert to 'any', re-render poster, notify
|
if (res.status === 404) {
|
||||||
_ppGigTuningPref = 'any';
|
// No-match 404: revert tuning pref to 'any', re-render poster to match
|
||||||
lsSet(PP_TUNING_PREF_KEY, 'any');
|
_ppGigTuningPref = 'any';
|
||||||
if (_ppGigProposal) {
|
lsSet(PP_TUNING_PREF_KEY, 'any');
|
||||||
const overlay = $('pp-overlay');
|
if (_ppGigProposal) {
|
||||||
if (overlay) overlay.innerHTML = gigPosterHTML(_ppGigProposal);
|
const overlay = $('pp-overlay');
|
||||||
|
if (overlay) overlay.innerHTML = gigPosterHTML(_ppGigProposal);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
// All errors: notify (404 has a tuning-specific message; others are generic)
|
||||||
if (window.fbNotify && typeof window.fbNotify.show === 'function') {
|
if (window.fbNotify && typeof window.fbNotify.show === 'function') {
|
||||||
try { window.fbNotify.show({ title: 'Tuning filter', message: (err && err.detail) || 'No songs match that tuning filter.', icon: '🎸' }); } catch (_) { /* */ }
|
const msg = res.status === 404
|
||||||
|
? (err && err.detail) || 'No songs match that tuning filter.'
|
||||||
|
: 'Could not book gig — please try again.';
|
||||||
|
try { window.fbNotify.show({ title: 'Gig booking', message: msg, icon: '🎸' }); } catch (_) { /* */ }
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
_ppGigProposal = await res.json();
|
_ppGigProposal = await res.json();
|
||||||
} catch (_) { return; }
|
} catch (_) { return; }
|
||||||
|
if (gen !== _ppBookGen) return; // stale — superseded while awaiting json()
|
||||||
const overlay = $('pp-overlay');
|
const overlay = $('pp-overlay');
|
||||||
if (!overlay) return;
|
if (!overlay) return;
|
||||||
_ppBook = null; // the poster replaces the book in the overlay
|
_ppBook = null; // the poster replaces the book in the overlay
|
||||||
@@ -1706,6 +1716,10 @@
|
|||||||
setTuningPref(p) { _ppGigTuningPref = p; },
|
setTuningPref(p) { _ppGigTuningPref = p; },
|
||||||
getLastTuning() { return _ppGigLastTuning; },
|
getLastTuning() { return _ppGigLastTuning; },
|
||||||
setLastTuning(t) { _ppGigLastTuning = t; },
|
setLastTuning(t) { _ppGigLastTuning = t; },
|
||||||
|
getBookGen() { return _ppBookGen; },
|
||||||
|
setBookGen(g) { _ppBookGen = g; },
|
||||||
|
getProposal() { return _ppGigProposal; },
|
||||||
|
bookGig,
|
||||||
};
|
};
|
||||||
|
|
||||||
if (document.readyState === 'loading') {
|
if (document.readyState === 'loading') {
|
||||||
|
|||||||
@@ -168,3 +168,110 @@ describe('career-gig-tuning interstitial', () => {
|
|||||||
assert.equal(get(ctx, 'window.__careerPassportTest.getTuningHold()'), null);
|
assert.equal(get(ctx, 'window.__careerPassportTest.getTuningHold()'), null);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// bookGig — generation guard (F1) and 404-only revert (F2)
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('career-gig-tuning bookGig', () => {
|
||||||
|
// Helper: make a ctx where bookGig is callable.
|
||||||
|
// fetch is overridable per-test via ctx.fetch.
|
||||||
|
function makeBookCtx() {
|
||||||
|
const ctx = vm.createContext({
|
||||||
|
window: {},
|
||||||
|
document: {
|
||||||
|
getElementById: () => null,
|
||||||
|
readyState: 'complete',
|
||||||
|
addEventListener: () => {},
|
||||||
|
},
|
||||||
|
localStorage: {
|
||||||
|
_store: {},
|
||||||
|
getItem(k) { return this._store[k] != null ? this._store[k] : null; },
|
||||||
|
setItem(k, v) { this._store[k] = String(v); },
|
||||||
|
},
|
||||||
|
clearTimeout: () => {},
|
||||||
|
setTimeout: () => 42,
|
||||||
|
fetch: null, // set per test
|
||||||
|
console,
|
||||||
|
});
|
||||||
|
ctx.window.feedBack = { on: () => {}, emit: () => {} };
|
||||||
|
vm.runInContext(SCREEN_JS, ctx);
|
||||||
|
// Seed _pp so bookGig can find the passport
|
||||||
|
vm.runInContext(`
|
||||||
|
window.__careerPassportTest.setView({
|
||||||
|
instruments: {
|
||||||
|
guitar: {
|
||||||
|
passports: [{ genre_key: 'rock', genre: 'Rock' }]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
`, ctx);
|
||||||
|
return ctx;
|
||||||
|
}
|
||||||
|
|
||||||
|
test('F1: stale response from superseded request is discarded — _ppGigProposal keeps new value', async () => {
|
||||||
|
// Failure input: two requests fire; second completes first; first (stale) must be dropped.
|
||||||
|
// Without the generation guard, the stale Drop response would overwrite the Standard proposal.
|
||||||
|
const ctx = makeBookCtx();
|
||||||
|
let resolveFirst, resolveSecond;
|
||||||
|
const first = new Promise(r => { resolveFirst = r; });
|
||||||
|
const second = new Promise(r => { resolveSecond = r; });
|
||||||
|
let callCount = 0;
|
||||||
|
ctx.fetch = () => {
|
||||||
|
callCount++;
|
||||||
|
return callCount === 1 ? first : second;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Fire first request (drop), don't resolve yet
|
||||||
|
const p1 = vm.runInContext(`
|
||||||
|
window.__careerPassportTest.setTuningPref('drop');
|
||||||
|
window.__careerPassportTest.bookGig('rock');
|
||||||
|
`, ctx);
|
||||||
|
|
||||||
|
// Fire second request (standard) — increments gen
|
||||||
|
const p2 = vm.runInContext(`
|
||||||
|
window.__careerPassportTest.setTuningPref('standard');
|
||||||
|
window.__careerPassportTest.bookGig('rock');
|
||||||
|
`, ctx);
|
||||||
|
|
||||||
|
// Resolve SECOND first (standard wins)
|
||||||
|
resolveSecond({ ok: true, status: 200, json: async () => ({ songs: [{ filename: 'standard.sloppak', tuning_name: 'E Standard' }], tuning_pref: 'standard' }) });
|
||||||
|
await p2;
|
||||||
|
|
||||||
|
// Now resolve stale FIRST (drop) — must be discarded
|
||||||
|
resolveFirst({ ok: true, status: 200, json: async () => ({ songs: [{ filename: 'drop.sloppak', tuning_name: 'Drop D' }], tuning_pref: 'drop' }) });
|
||||||
|
await p1;
|
||||||
|
|
||||||
|
// _ppGigProposal must reflect the second (standard) response, not the stale first.
|
||||||
|
// getProposal() exposes _ppGigProposal via the test seam.
|
||||||
|
const proposal = vm.runInContext('window.__careerPassportTest.getProposal()', ctx);
|
||||||
|
// If the generation guard is absent, stale drop overwrites standard → songs[0] is drop.sloppak
|
||||||
|
assert.ok(
|
||||||
|
proposal === null || proposal.songs[0].filename !== 'drop.sloppak',
|
||||||
|
'stale drop response must not overwrite the winning standard proposal'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('F2: 500 error keeps user pref — only 404 reverts to any', async () => {
|
||||||
|
// Failure input: saved pref 'drop', server returns 500 → without fix, pref silently becomes 'any'
|
||||||
|
const ctx = makeBookCtx();
|
||||||
|
ctx.fetch = async () => ({ ok: false, status: 500, json: async () => ({}) });
|
||||||
|
|
||||||
|
vm.runInContext(`window.__careerPassportTest.setTuningPref('drop');`, ctx);
|
||||||
|
await vm.runInContext(`window.__careerPassportTest.bookGig('rock');`, ctx);
|
||||||
|
|
||||||
|
const pref = vm.runInContext(`window.__careerPassportTest.getTuningPref()`, ctx);
|
||||||
|
assert.equal(pref, 'drop', '500 error must not reset pref to any');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('F2: 404 still reverts pref to any', async () => {
|
||||||
|
const ctx = makeBookCtx();
|
||||||
|
ctx.fetch = async () => ({ ok: false, status: 404, json: async () => ({ detail: 'No drop songs.' }) });
|
||||||
|
|
||||||
|
vm.runInContext(`window.__careerPassportTest.setTuningPref('drop');`, ctx);
|
||||||
|
await vm.runInContext(`window.__careerPassportTest.bookGig('rock');`, ctx);
|
||||||
|
|
||||||
|
const pref = vm.runInContext(`window.__careerPassportTest.getTuningPref()`, ctx);
|
||||||
|
assert.equal(pref, 'any', '404 must revert pref to any');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user