The code is shown once, here. Only its hash is stored, so it cannot be read again afterwards.
+Codes stay readable here so you can copy one again — an invitation usually has to be given to somebody later than the moment it was made. A code that has been used, revoked or expired can be deleted.
diff --git a/public/js/admin.js b/public/js/admin.js index 47f7d5fc..adcf0b47 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -1869,12 +1869,15 @@ initImageSettings(); // Used, or expired without being redeemed. A revoked code keeps its row: it // records a decision somebody took, and it is not cluttering anything the way // a pile of expired codes does. - var SPENT_STATUS = ['used', 'expired']; + // Spent: it can no longer be redeemed, whatever ended it. Revoked was held + // back at first — revoke stops a code and leaves the row — but a revoked code + // is already dead, so keeping it only fills the list. + var SPENT_STATUS = ['used', 'expired', 'revoked']; // Spent invitations in one go, which is what a cluttered list actually wants. // Confirmed first: it is a delete, even if everything it removes is finished. function clearUsedInvites() { - showConfirm('Delete every used and expired invitation? Live and revoked ones are kept.', function() { + showConfirm('Delete every used, revoked and expired invitation? Ones that can still be redeemed are kept.', function() { fetch('/api/admin/invites/spent', { method: 'DELETE', headers: getAuthHeaders() }) .then(function(r) { return r.json(); }) .then(function(data) { diff --git a/src/utils/registrationInvites.js b/src/utils/registrationInvites.js index dc409d6e..1d20621a 100644 --- a/src/utils/registrationInvites.js +++ b/src/utils/registrationInvites.js @@ -137,16 +137,16 @@ async function revoke(id, adminUserId) { return result.changes > 0; } -// Spent: used, or run out of time without being used. The one thing never -// deletable is a code that could still be redeemed — deleting that takes it off -// the list without taking it out of anybody's inbox, so the holder keeps -// something that looks valid, it quietly stops working, and nothing is left to -// say who had it. Revoke is what stops a live code, and it leaves the row. +// Spent: used, revoked, or run out of time. The one thing never deletable is a +// code that could still be redeemed — deleting that takes it off the list +// without taking it out of anybody's inbox, so the holder keeps something that +// looks valid, it quietly stops working, and nothing is left to say who had it. // -// Written to match the status the list shows rather than as -// "used OR expires_at <= NOW()", which would also catch a revoked code whose -// date had passed — one the screen still labels revoked, with no delete on it. -var SPENT = '(used_at IS NOT NULL OR (revoked_at IS NULL AND expires_at <= NOW()))'; +// Revoked codes were held back from this at first, on the reasoning that revoke +// stops a code and leaves the row. But a revoked code is already dead: it +// cannot be redeemed, and keeping it only fills the list. Revoke and delete are +// two steps of the same thought, and the second was missing. +var SPENT = '(used_at IS NOT NULL OR revoked_at IS NOT NULL OR expires_at <= NOW())'; /** * Delete a spent invitation. diff --git a/test/backend-hardening.test.js b/test/backend-hardening.test.js index f610a13f..774d064f 100644 --- a/test/backend-hardening.test.js +++ b/test/backend-hardening.test.js @@ -156,14 +156,17 @@ test('an invitation is deletable only once it can no longer be used', () => { // A code that could still be redeemed may be sitting in somebody's inbox. // Deleting the row takes it off the list without taking it out of their // hands: it quietly stops working and nothing is left to say who held it. - // Revoke does that job and leaves the row behind, marked. - assert.match(src, /var SPENT = '\(used_at IS NOT NULL OR \(revoked_at IS NULL AND expires_at <= NOW\(\)\)\)';/); + // Revoke is what stops such a code — and a revoked one is then dead, so it + // is deletable like any other spent code. + assert.match(src, /var SPENT = '\(used_at IS NOT NULL OR revoked_at IS NOT NULL OR expires_at <= NOW\(\)\)';/); assert.match(src, /DELETE FROM registration_invites WHERE id = \$1 AND ' \+ SPENT/); assert.match(src, /DELETE FROM registration_invites WHERE ' \+ SPENT/, 'and in bulk'); - // Written to match the status the list shows: "used OR expired" would also - // catch a revoked code past its date, which the screen still calls revoked - // and offers no delete on. - assert.match(src, /revoked_at IS NULL AND expires_at <= NOW\(\)/); + // Scoped to the rule itself. revoked_at IS NULL appears elsewhere and + // belongs there: revoke() will not re-revoke, and claim() will not redeem a + // revoked code. It is only in *this* rule that it meant "keep it forever". + const spentRule = src.slice(src.indexOf('var SPENT ='), src.indexOf('var SPENT =') + 120); + assert.doesNotMatch(spentRule, /revoked_at IS NULL/, 'a revoked code is spent, not protected'); + assert.match(spentRule, /revoked_at IS NOT NULL/); // Refused with the reason, not as a missing row: the row is very likely there. assert.match(route, /That invitation can still be used\. Revoke it instead\./); @@ -173,9 +176,9 @@ test('an invitation is deletable only once it can no longer be used', () => { // And the button is only offered where it can work. const js = read('public/js/admin.js'); - assert.match(js, /var SPENT_STATUS = \['used', 'expired'\];/); + assert.match(js, /var SPENT_STATUS = \['used', 'expired', 'revoked'\];/); assert.match(js, /SPENT_STATUS\.indexOf\(row\.status\) !== -1\s*\n?\s*\? '