Skip to content

feat: decouple gate checkin and add self checkin-checkout kiosk endpo… - #474

Merged
vvshk merged 13 commits into
devfrom
feat/self-checkin-kiosk
Nov 2, 2026
Merged

vvshk merged 13 commits into
devfrom
feat/self-checkin-kiosk

Conversation

@vvshk

@vvshk vvshk commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Self check-in and check-out kiosk API. The kiosk is optional: it is for guests who want a WiFi code. The gate keeps checking guests in and out as before. A new kioskAdmin role can reach the kiosk and nothing else.

flowchart LR
  Tablet["Kiosk tablet<br/>(kioskAdmin login)"] --> L["GET kiosk/bookings/:cardno<br/>stays + checkout preview"]
  L --> CI["PUT checkin / checkout"]
  CI --> W["WiFi temp code<br/>generate_temp_code / fetch_temp_codes"]
  Tablet -. "any other admin route" .-> X["403"]
  Gate["Gate entry / exit (unchanged)"] --> S["stay checked in / out as before"]
  S -. "guest scans at kiosk" .-> W
Loading
  • Lookup returns current stays, including unpaid ones, plus a read-only checkout preview: late fee (₹550 before 3 PM, ₹1,100 after) or early-checkout credit. Credit counts a refund only for completed payments, same rule as the real cancel.
  • Overdue checkouts filter on the late-fee report. Day visits pay no late fee.
  • Expired, malformed or missing admin token answers 401 with a short reason.
  • One spelling per kiosk WiFi route.

Evidence

  • Before: expired kiosk login answered every scan with a 500 and a stack trace; "Payment Incomplete" could never show; a kiosk had no limited role (needed full room-admin).
    After: local backend + test DB + headless browser, fake camera, 107 / 107 checks pass. lint:undef clean.
expired / malformed / wrong-signature / missing token, 4 admin endpoints -> 401, no stack trace
kioskAdmin token: lookup, check-in, check-out, WiFi codes               -> 200
kioskAdmin token: 26 other admin endpoints                              -> 403
early check-out, paid stay    -> preview "₹1100 credit"  == real credit
early check-out, unpaid stay  -> no credit shown
late check-out after 11:00    -> preview "₹550 / ₹1,100"
gate entry, then kiosk scan   -> room + one WiFi code, rescan returns the same code
gate entry alone              -> stay checked in, no kiosk needed
unknown card / unpaid / cash-pending / double scan                      -> handled

Screenshots are on admin PR #225. Two toggleRCBlockMode tests fail the same way on plain dev; the full jest run is flaky on a shared test DB, also on plain dev.

Merge Danger

Door: two-way. Code only, no migration.

The kioskAdmin role is one row added by hand: a super admin inserts role kioskAdmin (status active), then creates the account with only that role. Logins last 24 hours. A kiosk login always lands on check-in; the check-out tablet opens its page by URL.

Blast Radius: kiosk and admin login

  • The gate is untouched. Kiosk check-in is optional, so guests who skip it lose nothing except the WiFi code.
  • Bad admin tokens now answer 401 instead of 500 on every admin endpoint.
  • Accepted for now: anyone can type a card number (no second factor); check-in and check-out look the booking up by id without checking it belongs to the scanned card, so someone with the kiosk login could act on another guest's booking. Fix later: send the card number with each request.

Deploy this before admin #225.

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 1m 26s —— View job


Review of PR #474: kiosk check-in/out and temp WiFi

  • Read the diff and the repo rules (AGENTS.md)
  • Read the related code: route auth, WiFi controller, gate entry/exit, manualCheckin
  • Posted 5 inline comments
  • Posted this summary

Should fix before merge:

  1. The kiosk needs a full room-admin token (routes/admin/roomManagement.routes.js:61-63). The new kiosk routes sit behind the office, super and room admin roles. So the unattended kiosk device has to hold a credential that can also book rooms, block rooms and RC, revoke late-checkout fees, and check in or out any booking id. I suggest a dedicated kiosk role with its own small route file. The server should also check that the booking being checked in or out belongs to the scanned card.
  2. Gate entry no longer checks anyone in (gateManagement.controller.js:275). A guest who only scans at the gate now stays pending_checkin. That breaks their temp WiFi request in the member app, and the guest is missing from checkedin-based reports until staff or the kiosk checks them in. gateExit still auto-checks-out flat bookings, so entry and exit now behave differently. The removal also leaves RoomBooking, ROOM_STATUS_PENDING_CHECKIN and logger imported but unused.

Worth tidying:

  • The ?kiosk=true branch of fetchRoomBookingsByCard returns a different response shape from the same endpoint. It sends the card's entire booking history with all columns, has no req.log line, and returns 200 instead of 404 when the card doesn't exist. A separate kiosk endpoint limited to current bookings would be cleaner.
  • The admin WiFi handlers don't match each other. Generate trims cardno but fetch doesn't. Fetch skips the res_status check that fetchTempCodes does. Neither handler uses req.log, which matters for auditing an unattended kiosk. I left a suggestion block for the fetch handler.
  • The day-visit WiFi rule looks correct for rooms. It isn't applied to flat bookings, and because fetchBookings is shared, it also changes the member-app WiFi endpoints. Please say so in the PR description.

What's good: Moving the logic into generateTempCodeForCard lets the member app and the kiosk share it. Locking the card row (FOR UPDATE) stops two parallel requests from both getting past the one-code limit, and every WiFi query now runs in the same transaction. Using the IST date in fetchBookings removes a timezone edge case at midnight.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

}
});

await t.commit();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behaviour change: gate scans no longer check anyone in. Before this PR, a gate entry scan on the check-in day moved pending_checkin room and flat bookings to checkedin. After it, only manualCheckin / flatCheckin (staff or the kiosk) do that. That has two effects:

  1. A guest who only scans at the gate stays pending_checkin. fetchBookings() in wifi.controller.js needs checkedin, so the guest's app-side temp WiFi request now fails with "user not checked in yet". Anything else that reads checkedin (occupancy, day-wise reports) is affected the same way until someone checks the guest in.
  2. gateExit (around L317–329) still auto-checks-out flat bookings. Entry and exit now treat bookings differently. If you meant to fully decouple gate scans from booking status, move the exit-side update too, or note in the PR why it stays.

Also, this removal leaves RoomBooking, ROOM_STATUS_PENDING_CHECKIN and logger imported but unused in this file (L5, L16, L18). lint:undef won't flag them, so please drop them.

Comment thread routes/admin/roomManagement.routes.js Outdated
Comment on lines +61 to +63
// Kiosk WiFi actions use the scanned guest card number, not the admin's card.
router.post('/kiosk/wifi/generate-temp-code', CatchAsync(generateTempCodeForAdmin));
router.get('/kiosk/wifi/fetch-temp-codes/:cardno', CatchAsync(fetchTempCodesForAdmin));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: the self-service kiosk needs a full room-admin login. These routes sit under router.use(authorizeRoles(ROLE_OFFICE_ADMIN, ROLE_SUPER_ADMIN, ROLE_ROOM_ADMIN)). The kiosk device, which guests use unattended, therefore has to hold an office-, room- or super-admin token. With that token anyone at the kiosk can call every other route in this file: bookForMumukshu, block_room, block_rc, update_room_booking, late-checkout-fees/revoke, and the rest. It also allows the kiosk to checkin or checkout any booking id, not just the scanned guest's.

Please consider:

  • a dedicated ROLE_KIOSK (or similar), and a small kiosk.routes.js that exposes only fetch-bookings-by-card, check in, check out and the two WiFi actions;
  • checking server-side that the bookingid being checked in or out belongs to the scanned cardno.

Smaller points:

  • generate-temp-code and fetch-temp-codes use kebab-case, but the other routes here use snake_case (fetch_room_bookings, block_rc).
  • There is a doubled blank line after L63 (and after the import at L42).

Comment on lines +969 to +989
if (req.query.kiosk === 'true') {
const [roomBookings, flatBookings, cardDetails] = await Promise.all([
RoomBooking.findAll({ where: { cardno }, order: [['checkin', 'ASC']] }),
FlatBooking.findAll({ where: { cardno }, order: [['checkin', 'ASC']] }),
CardDb.findOne({
where: { cardno },
attributes: ['cardno', 'issuedto']
})
]);

return res.status(200).send({
message: 'Fetched bookings',
data: {
room_booking: roomBookings,
flat_booking: flatBookings,
card_details: cardDetails
}
});
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some issues with the ?kiosk=true branch:

  • One endpoint, two response shapes. With the flag, data is an object; without it, data is an array. Clients have to branch on a query flag to know which they got. A separate handler and route (for example GET /kiosk/bookings/:cardno) is clearer, and it fits the dedicated-kiosk-role suggestion on the routes file.
  • It returns more than a kiosk needs. It sends every room and flat booking the card has ever had, with all columns. A guest at an unattended screen only needs current and upcoming bookings, such as checkout >= today with status pending_checkin or checkedin. You could also limit attributes.
  • No logging or 404. This path skips the fetch_room_bookings_by_card_success log line. If the card does not exist, it returns 200 with card_details: null instead of a 404.
  • There are two extra blank lines after the block (L988–989).

Comment on lines +175 to +184
export const fetchTempCodesForAdmin = async (req, res) => {
const { cardno } = req.params;
if (!cardno) throw new APIError(400, 'cardno is required');

const rows = await fetchTempCodesForCard(cardno);
return res.status(200).send({
message: rows.length ? 'Wifi Passwords' : 'No active WiFi passwords found',
data: rows
});
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three small inconsistencies between the admin fetch and generate paths:

  • generateTempCodeForAdmin trims cardno (L166), but this handler does not. A scan with stray whitespace can therefore generate a code and then fetch nothing.
  • fetchTempCodes checks res_status (mumukshu or guest) before returning codes. fetchTempCodesForCard doesn't. Behaviour is the same today, because only eligible cards can ever get a row, but the two paths are asymmetric.
  • AGENTS.md asks request controllers to use req.log. Neither new admin handler logs anything, so it's hard to trace which admin account generated a code for which card. For an unattended kiosk, req.log.info('kiosk_wifi_temp_code_generated', { cardno, by: req.user.username }) in generateTempCodeForAdmin would help with audits.
Suggested change
export const fetchTempCodesForAdmin = async (req, res) => {
const { cardno } = req.params;
if (!cardno) throw new APIError(400, 'cardno is required');
const rows = await fetchTempCodesForCard(cardno);
return res.status(200).send({
message: rows.length ? 'Wifi Passwords' : 'No active WiFi passwords found',
data: rows
});
};
export const fetchTempCodesForAdmin = async (req, res) => {
const cardno = req.params.cardno?.trim();
if (!cardno) throw new APIError(400, 'cardno is required');
const rows = await fetchTempCodesForCard(cardno);
req.log.info('kiosk_wifi_fetch_temp_codes', { cardno, count: rows.length });
return res.status(200).send({
message: rows.length ? 'Wifi Passwords' : 'No WiFi passwords found',
data: rows
});
};

Comment on lines +394 to +403
// Day visits (nights 0) store checkout = checkin + 1; judge them by checkin
// so a visit today is not eligible tomorrow.
const roomWhereClause = {
cardno,
status: ROOM_STATUS_CHECKEDIN,
[Sequelize.Op.or]: [
{ nights: { [Sequelize.Op.gt]: 0 }, checkout: { [Sequelize.Op.gte]: today } },
{ nights: 0, checkin: { [Sequelize.Op.gte]: today } }
]
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This day-visit rule looks right. manualCheckin blocks checkin > today, so gte behaves the same as eq in practice. Two notes:

  • The new rule applies to rooms only. FlatBooking also has nights and still uses commonWhereClause. A flat day visit (if those exist) would still be WiFi-eligible the next day.
  • Because fetchBookings is shared, this also changes the member-app generateTempCode and fetchTempCodes paths, not just the kiosk. That is probably what you want, but please mention it in the PR description.

Optional: { nights: 0, checkin: today } states the intent more clearly than gte.

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 1m 14s —— View job


Re-review of PR #474 after 14fbfd5

  • Read the new commit
  • Checked how it handles the earlier findings
  • Posted 5 inline comments
  • Posted this summary

Fixed from the last review:

  • Gate entry and exit are now consistent: neither changes booking status. The unused imports (RoomBooking, FlatBooking, logger, the status constants) are removed.
  • There is now a separate fetchKioskBookingsByCard handler. It trims the card number, returns 404 for an unknown card, logs, limits attributes, and returns only current or upcoming bookings.
  • The admin WiFi handlers now trim cardno and validate it the same way, and both log with by: req.user.username.
  • The day-visit rule is now { nights: 0, checkin: today }.

Still should fix:

  1. The kiosk still needs a full room-admin token (routes/admin/roomManagement.routes.js:61-64). Nothing has changed here since the last review. Please add a dedicated kiosk role and route file, plus a server-side check that the booking belongs to the scanned card. If you're accepting the risk for now, say so in the PR description.
  2. STATUS_PENDING is not a valid booking status (roomManagement.controller.js:990). 'pending' isn't in the RoomBooking or FlatBooking enums, so that filter can never match. I left a suggestion block to drop it.

Worth tidying:

  • Duplicate routes: both kebab-case and snake_case paths are registered for each kiosk WiFi route. Pick one; snake_case matches this file.
  • Leftover ?kiosk=true branch on fetch_room_bookings (L1035): it is now redundant with /kiosk/bookings/:cardno. Remove it unless a client already depends on it.
  • if (req.log) guards in wifi.controller.js: they aren't needed, since other controllers call req.log directly.
  • Flat bookings never get checked out automatically. Now that gateExit doesn't do it, only flatCheckout does, and no cron covers it. Guests who skip the kiosk stay checkedin, and their old stays keep showing up in the kiosk list. Please note this in the PR description, or add a cleanup cron.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

[Sequelize.Op.or]: [
{ status: ROOM_STATUS_CHECKEDIN },
{
status: [ROOM_STATUS_PENDING_CHECKIN, STATUS_PENDING],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STATUS_PENDING ('pending') is not one of the RoomBooking or FlatBooking status values. Both enums allow only waiting, payment pending, pending checkin, checkedin, checkedout, cancelled and admin cancelled. That part of the filter can never match, so it's dead code and misleading. If you meant unpaid bookings, the constant is STATUS_PAYMENT_PENDING. But manualCheckin and flatCheckin only accept pending checkin, so the kiosk would list bookings it can't check in. The simplest fix is to drop it and remove the STATUS_PENDING import at L33:

Suggested change
status: [ROOM_STATUS_PENDING_CHECKIN, STATUS_PENDING],
status: ROOM_STATUS_PENDING_CHECKIN,

Also, the checkedin branch has no date bound. That's fine for overdue stays, but it means a stale checkedin row from months ago will appear at the kiosk, because gate exit no longer checks flats out (see the gate comment).

Comment thread routes/admin/roomManagement.routes.js Resolved
};

export const fetchRoomBookingsByCard = async (req, res) => {
if (req.query.kiosk === 'true') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that /kiosk/bookings/:cardno exists, is the ?kiosk=true branch on fetch_room_bookings still needed? Keeping it means two URLs serve the kiosk response, and this endpoint still returns two response shapes depending on a query flag. If no client already ships with ?kiosk=true, please remove the branch so this handler goes back to its original form.

Comment thread controllers/wifi/wifi.controller.js Outdated
Comment on lines +170 to +175
if (req.log) {
req.log.info('kiosk_wifi_temp_code_generated', {
cardno: cleanCardNo,
by: req.user?.username
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if (req.log) guard isn't needed. The request logger middleware always sets req.log, and other controllers (such as manualCheckin and fetchKioskBookingsByCard) call it directly. The same applies at L192. To match the surrounding code:

Suggested change
if (req.log) {
req.log.info('kiosk_wifi_temp_code_generated', {
cardno: cleanCardNo,
by: req.user?.username
});
}
req.log.info('kiosk_wifi_temp_code_generated', {
cardno: cleanCardNo,
by: req.user?.username
});

...(createdAt && { createdAt })
},
{ transaction: t }
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entry and exit are now consistent, and the unused imports are gone, thanks. One effect to note in the PR description: nothing else ever moves a flat booking to checkedout. cron.js only handles unpaid past bookings, so only flatCheckout (staff or kiosk) does it. Flat guests who leave through the gate without using the kiosk will stay checkedin indefinitely. That affects occupancy reports and the WiFi eligibility check (checkout >= today stops matching, so WiFi is fine, but the kiosk list from fetchKioskBookingsByCard will keep showing the old flat stay). Consider adding a daily cron that checks out stale checkedin bookings, or at least flagging them in a report.

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 54s —— View job


Re-review of PR #474 after 359ac53

  • Read the new commit
  • Checked the new overdue-checkouts filter
  • Posted 2 inline comments
  • Posted this summary

Fixed from the last review:

  • The invalid STATUS_PENDING filter and its import are removed.
  • The ?kiosk=true branch on fetch_room_bookings is removed, so that handler is back to one response shape.
  • The if (req.log) guards are gone from wifi.controller.js.
  • Stale stays: checkedin rows now drop off the kiosk list after 14 days, and the new overdue_checkouts report lets staff find guests who never checked out.

Should fix:

  1. The overdue report shows late fees for day visits (roomManagement.controller.js:2445-2452). handleSameDayCheckout never charges a fee when nights < 1, but this report shows ₹… (Est.) for those bookings. I left a suggestion block.
  2. amount changes type in this mode. The other payment_type values return a number. overdue_checkouts returns strings like "₹500 (Est.)" and "3d overdue". A client that sums or formats amount will break. I suggest keeping amount numeric or null and adding est_late_fee and days_overdue fields.

Minor:

  • Replace the 14 at L989 with a named constant, and build the cutoff from the existing today variable. My suggestion block uses KIOSK_CHECKEDIN_LOOKBACK_DAYS, which doesn't exist yet, so declare it before applying the suggestion. Note that a guest more than 14 days overdue can't check out at the kiosk; staff have to do it.

Still open (deferred by the author):

  • The kiosk still needs a full room-admin token, and the duplicate kebab-case and snake_case kiosk WiFi routes are still registered. Please note the deferred kiosk-role work in the PR description so it isn't lost.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

Comment on lines +2445 to +2452
const isToday = rb.checkout === today;
const isHalfDay = nowTime <= LATE_CHECKOUT_HALF;
const estLateFee = isToday ? calcLateCheckoutFee(rb.roomtype, isHalfDay) : null;
const daysOverdue = moment(today).diff(moment(rb.checkout), 'days');

return {
id: null,
amount: isToday ? `₹${estLateFee} (Est.)` : (daysOverdue > 0 ? `${daysOverdue}d overdue` : 'Overdue'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two problems with the estimated fee:

  1. Day visits show a fee they won't be charged. handleSameDayCheckout (L102) never charges a late fee when booking.nights < 1. A day visit stores checkout = checkin + 1, so a guest still checkedin after 11:00 on that date shows ₹… (Est.) here, but checking them out creates no fee. Please apply the same nights >= 1 rule here.
  2. amount has a different type in this mode. The other payment_type modes return the transaction's numeric amount. This one returns display strings ("₹500 (Est.)", "3d overdue"). A client that sums or formats amount will break. I'd keep amount numeric (or null) and add separate fields such as est_late_fee and days_overdue, so the UI can decide how to show them.
Suggested change
const isToday = rb.checkout === today;
const isHalfDay = nowTime <= LATE_CHECKOUT_HALF;
const estLateFee = isToday ? calcLateCheckoutFee(rb.roomtype, isHalfDay) : null;
const daysOverdue = moment(today).diff(moment(rb.checkout), 'days');
return {
id: null,
amount: isToday ? `₹${estLateFee} (Est.)` : (daysOverdue > 0 ? `${daysOverdue}d overdue` : 'Overdue'),
const isToday = rb.checkout === today;
const isHalfDay = nowTime <= LATE_CHECKOUT_HALF;
const estLateFee =
isToday && rb.nights >= 1 ? calcLateCheckoutFee(rb.roomtype, isHalfDay) : null;
const daysOverdue = moment(today).diff(moment(rb.checkout), 'days');
return {
id: null,
amount: estLateFee,
est_late_fee: estLateFee,
days_overdue: daysOverdue,

(If you make this change, give the flat mapper the same amount: null, days_overdue shape.)

[Sequelize.Op.or]: [
{
status: ROOM_STATUS_CHECKEDIN,
checkout: { [Sequelize.Op.gte]: moment().tz('Asia/Kolkata').subtract(14, 'days').format('YYYY-MM-DD') }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 14-day cap keeps stale checkedin rows off the kiosk, which is good. Together with the new overdue report, this covers my earlier concern about flats that never get checked out. Two small points:

  • 14 is a magic number. A named constant such as KIOSK_CHECKEDIN_LOOKBACK_DAYS with a one-line comment would explain it. Keep in mind that a guest whose stay is more than 14 days overdue can no longer check out at the kiosk. Staff have to handle it from the overdue report.
  • today is already computed from nowIST a few lines up. Deriving the cutoff from it avoids calling moment() a second time.
Suggested change
checkout: { [Sequelize.Op.gte]: moment().tz('Asia/Kolkata').subtract(14, 'days').format('YYYY-MM-DD') }
checkout: { [Sequelize.Op.gte]: moment(today).subtract(KIOSK_CHECKEDIN_LOOKBACK_DAYS, 'days').format('YYYY-MM-DD') }

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 22s —— View job


Re-review of PR #474 after a0ed315

  • Read the new commit
  • Checked it against the last review's findings
  • No new inline comments needed
  • Posted this summary

Fixed from the last review:

  • Day visits no longer show a late fee. estLateFee now needs rb.nights >= 1. That matches handleSameDayCheckout, which never charges a fee for day visits.
  • amount is a number again. In overdue_checkouts mode it is now null or a numeric fee, like the other payment_type modes. The new fields est_late_fee and days_overdue hold the extra detail. The flat mapper returns the same shape, and its unused isToday is removed.
  • The 14-day lookback is a named constant. KIOSK_CHECKEDIN_LOOKBACK_DAYS has a comment explaining why, and the cutoff is now built from the existing today instead of a second moment() call.

I found no new problems in this commit.

Still open (deferred by the author):

  1. The kiosk still needs a full room-admin token (routes/admin/roomManagement.routes.js:61-64). An unattended kiosk holding that token can call every route in the file and check in or out any bookingid. You said this will be resolved later. Please write it in the PR description or open a follow-up issue so it isn't lost after merge.
  2. Duplicate kiosk WiFi routes. Both kebab-case and snake_case paths are still registered for each route. This is minor, and the suggestion block from the earlier review still applies.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

@claude

claude Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vendz's task in 1m 13s —— View job


Re-review of PR #474 after e9da3dc

  • Read the new commit (kiosk role, checkout preview, 401 on bad tokens)
  • Checked it against the earlier findings and the checkout and credit code
  • Posted 3 inline comments
  • Posted this summary

Fixed from the last review:

  • The kiosk no longer needs a room-admin token. The new kioskAdmin role is limited to five route groups in auth: kiosk bookings, the two WiFi routes, and room and flat check-in and check-out. Any other route returns 403. I checked how the path is built: req.baseUrl + req.path gives /api/v1/admin/stay/..., so the regexes match. A path they don't recognise (such as a trailing slash) is refused with 403. authorizeRoles skips its check only when req.kioskRoute is set, and that only happens for accounts whose roles are all kioskAdmin.
  • A bad or expired JWT now returns 401 instead of 500.

Should fix:

  1. The kiosk can check in or out any bookingid (middleware/AdminAuth.js:13). The handlers never check that the booking belongs to the scanned card. An early checkout cancels the guest's transaction and books credits, so a wrong or tampered id affects another guest. Fix: when req.kioskRoute is set, require the cardno and add it to the findOne filter. Fix this →
  2. The checkout preview can show a credit the guest won't get (roomManagement.controller.js:982-991). cancelTransaction refunds amount + discount only for completed payments. For an unpaid or cancelled transaction, the preview still shows a credit. I left a suggestion block.

Worth checking:

  • payment pending bookings now appear in the kiosk list (L1030), but check-in only accepts pending checkin. Please confirm the kiosk UI hides the check-in button for these bookings.
  • "Card QR prefix" is in the commit message, but this diff doesn't change it. Did that change get left out of the commit?
  • Creating kioskAdmin accounts: this commit adds no migration or seed for the role. I assume it is assigned through the sudo admin-controls endpoint. Please confirm that endpoint accepts it.

Still open (minor): the duplicate kebab-case and snake_case kiosk WiFi routes are still registered. The kiosk regex accepts both, so removing one later will need a client check.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

Comment thread middleware/AdminAuth.js
['GET', /^\/stay\/kiosk\/bookings\/[^/]+$/],
['POST', /^\/stay\/kiosk\/wifi\/generate[-_]temp[-_]code$/],
['GET', /^\/stay\/kiosk\/wifi\/fetch[-_]temp[-_]codes\/[^/]+$/],
['PUT', /^\/stay\/(flat_)?check(in|out)\/[^/]+$/]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kiosk can still check in or check out any booking. Kiosk-only accounts are now limited to these routes, which is a big improvement. But PUT /stay/(flat_)?check(in|out)/:bookingid doesn't check who the booking belongs to. Anyone with the kiosk token, or physical access to the kiosk device, can check out another guest's booking by its id. On an early checkout, that cancels the guest's transaction and books credits.

Neither manualCheckin/manualCheckout nor flatCheckin/flatCheckout take a cardno today. A small fix: when req.kioskRoute is set, require the scanned cardno (in the body or query) and add it to the findOne where clause, so a mismatch returns 404. Staff requests stay as they are.

Comment on lines +982 to +991
const transaction = await Transactions.findOne({
where: {
bookingid: booking.bookingid,
...(isFlat && { category: TYPE_FLAT })
}
});
if (!transaction) return null;
const nights = await calculateNights(booking.checkin, today);
const newAmount = roomCharge(isFlat ? 'nac' : booking.roomtype) * nights;
const credit = Number(transaction.amount) + Number(transaction.discount) - newAmount;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The credit preview ignores the transaction's status, so it can show a credit the guest won't get. On checkout, cancelTransaction (helpers/transactions.helper.js:192-197) gives back amount + discount only when the transaction is payment completed or cash completed. Otherwise it gives back only discount. So the kiosk shows a credit for:

  • a booking with an unpaid (cash pending or payment pending) transaction. At checkout the guest gets a smaller bill, not a credit.
  • a transaction that is already cancelled or admin cancelled.

Use the same rule as cancelTransaction (you'll also need to import STATUS_CASH_COMPLETED):

Suggested change
const transaction = await Transactions.findOne({
where: {
bookingid: booking.bookingid,
...(isFlat && { category: TYPE_FLAT })
}
});
if (!transaction) return null;
const nights = await calculateNights(booking.checkin, today);
const newAmount = roomCharge(isFlat ? 'nac' : booking.roomtype) * nights;
const credit = Number(transaction.amount) + Number(transaction.discount) - newAmount;
const transaction = await Transactions.findOne({
where: {
bookingid: booking.bookingid,
...(isFlat && { category: TYPE_FLAT })
}
});
if (!transaction) return null;
const isPaid = [STATUS_PAYMENT_COMPLETED, STATUS_CASH_COMPLETED].includes(transaction.status);
if (!isPaid) return null;
const nights = await calculateNights(booking.checkin, today);
const newAmount = roomCharge(isFlat ? 'nac' : booking.roomtype) * nights;
const credit = Number(transaction.amount) + Number(transaction.discount) - newAmount;

Minor: this runs one Transactions query plus calculateNights for each booking. That's fine for the few bookings a single card has.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the latest commit: the preview now follows the same rule as the real cancel. It counts the full amount only when the payment is completed (online or cash), and only the discount otherwise.

Comment on lines +1030 to +1033
{
status: STATUS_PAYMENT_PENDING,
checkout: { [Sequelize.Op.gte]: today }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kiosk now lists payment pending bookings it can't act on. manualCheckin and flatCheckin only accept pending checkin, so tapping check-in on one of these returns 404 ERR_BOOKING_NOT_FOUND. If the kiosk should show "please complete payment" for these, that's fine, but the client needs to hide the check-in button based on status. Please confirm the kiosk UI does this, or drop this branch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended. The kiosk lists these stays so it can say 'Payment Incomplete' instead of 'No booking'. The kiosk page shows that message and offers no check-in button for them (covered in the browser test with an unpaid stay).

@claude

claude Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vendz's task in 37s —— View job


Re-review of PR #474 after 9f1e21b

  • Read the new commit
  • Compared it with cancelTransaction and the real early-checkout paths
  • No new inline comments needed
  • Posted this summary

Fixed from the last review:

  • The credit preview now uses the same rule as the real checkout. kioskCheckoutPreview counts amount + discount only when the status is payment completed or cash completed, and counts only discount otherwise. This is the same rule cancelTransaction uses (helpers/transactions.helper.js:191-196). The real checkout cancels the transaction and then opens a new pending one for newAmount, so credit = refund - newAmount is the right number to show. STATUS_CASH_COMPLETED is imported.

I found no new problems in this commit.

Still open:

  1. The kiosk can still check in or out any bookingid (middleware/AdminAuth.js:13). manualCheckin, manualCheckout, flatCheckin and flatCheckout still look a booking up by id only. On an early checkout, a wrong or tampered id cancels another guest's transaction. The earlier fix still applies: when req.kioskRoute is set, require the scanned cardno and add it to the findOne filter. If the owner accepts this risk, please add it to "Not changed on purpose" in the PR description. Fix this →
  2. The card QR prefix is not handled in this backend. No file in this PR parses CardNumber=. If the admin app (hotfix/utsav-admin-creating-mumukshu-registration-bugs #225) strips the prefix before calling /stay/kiosk/bookings/:cardno, that works. In that case, please change the PR description so it doesn't say this backend accepts the prefix.
  3. Minor: the kebab-case and snake_case kiosk WiFi routes are both still registered.

For information: payment pending bookings appear in the kiosk list but can't be checked in. The PR description says the kiosk handles unpaid stays, so I assume the UI hides the check-in button for them.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

@claude

claude Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vendz's task in 27s —— View job


Re-review of PR #474 after 958a276

Fixed from the last review:

I found no new problems in this commit.

Still open:

  1. The kiosk can check in or check out any bookingid (middleware/AdminAuth.js:13). manualCheckin, manualCheckout, flatCheckin and flatCheckout still look the booking up by id alone. On an early checkout, a wrong or tampered id cancels another guest's transaction and books credits for them. Fixing this needs changes in both repos: the kiosk page in hotfix/utsav-admin-creating-mumukshu-registration-bugs #225 calls PUT /stay/checkin/:id and /stay/checkout/:id without sending the card number. If the owner accepts this risk, please add it under "Not changed on purpose" in the PR description. Fix this →

Resolved since the last summary: the PR description now says the admin page strips the CardNumber= prefix, and #225 shows no check-in button for unpaid stays.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

@claude

claude Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vendz's task in 25s —— View job


Re-review of PR #474 after 4fee390

  • Read the new commit
  • Checked that the gate controller now matches dev
  • No new inline comments needed (this PR no longer changes the gate controller)
  • Posted this summary

What changed: controllers/admin/gateManagement.controller.js now matches origin/dev exactly, so this PR no longer touches it. Gate entry still checks in pending checkin room and flat bookings whose check-in date is today, and gate exit still checks out flat bookings with checkout <= today. The imports are back too, and all of them are used. I found no code problems in this commit.

Please update the PR description. Parts of it are now wrong:

  • The diagram's Gate … no longer checks in / out → stay stays 'pending checkin' branch.
  • The Summary line "Check-in moves off the gate."
  • The whole Blast Radius: gate check-in bullet: guests who skip the kiosk do not stay pending checkin, and nothing changes for their WiFi or maintenance requests. "Approved by the owner" refers to a change this PR no longer makes.

With the revert, the real blast radius is the kioskAdmin role, the 401 on bad admin tokens, the kiosk endpoints, and the day-visit WiFi rule in fetchBookings.

Worth checking in admin #225: a guest who scanned in at the gate on their check-in day is already checkedin when they reach the kiosk. Two things follow:

  • PUT /stay/checkin/:id returns 404 ERR_BOOKING_NOT_FOUND for that booking, because manualCheckin only accepts pending checkin.
  • The lookup returns the stay as checkedin with an early-checkout credit preview, because today < checkout.

The check-in tablet should show "Already checked in" for these guests, not an error and not a checkout or credit prompt. This case was rare before the revert, but it will now be the usual one.

Still open (risk accepted in the PR description): kiosk accounts can check in or out any bookingid, because the handlers don't check that the booking belongs to the scanned card.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

@claude

claude Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 55s —— View job


Re-review of PR #474 after 28afd26

  • Read the new commit (batched kiosk queries, booker and payer names)
  • Checked it against the models and the real early-checkout paths
  • Posted 2 inline comments
  • Posted this summary

What changed: fetchKioskBookingsByCard used to run one Transactions query per booking. It now runs a single batched query, and the card lookup runs in parallel with the booking queries. The response gains booker_name, and the credit preview gains is_self_booked, payer_cardno and payer_name.

What's correct:

  • bookedBy exists on both RoomBooking and FlatBooking, so the new attribute is safe.
  • The preview picks the payer as transaction.cardno || booking.bookedBy || booking.cardno. That is the same card that handleEarlyCheckout and flatCheckout give the credit to.
  • Keying txMap by bookingid is safe. Booking ids are UUIDs, and late-fee transactions get their own new UUID.
  • A card that doesn't exist still returns 404.

Should fix:

  1. The kiosk response now leaks another person's card number (L995–1005, L1129–1132). Anyone can type any card number at the kiosk (accepted risk). The response now returns payer_cardno, and the name fields fall back to the booker's card number when no name is found. So one typed card number reveals a second one, which can then be looked up too. I left a suggestion block that keeps only the payer's name and returns null when it's missing.

Minor:

  • The extra payer-card query runs after the other queries finish, and its card list isn't deduplicated. Fetching the transactions first, then the cards, needs 2 round trips instead of 3.
  • The preview now filters transactions to category IN ('room','flat'), but manualCheckout looks them up by bookingid only. Both find the same row in practice.

Still open (risk accepted in the PR description): kiosk accounts can check in or out any bookingid. Also, the earlier note about guests who already checked in at the gate still applies to admin #225.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

Comment on lines +995 to +1005
const payerCardno = transaction.cardno || booking.bookedBy || booking.cardno;
const isSelfBooked = !payerCardno || payerCardno === booking.cardno;
const payerName = !isSelfBooked
? (bookerNameMap?.get(payerCardno) || payerCardno)
: null;

return {
credit,
is_self_booked: isSelfBooked,
payer_cardno: payerCardno,
payer_name: payerName

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Privacy: the kiosk lookup now returns another person's card number. Anyone can type a card number at the kiosk, with no second factor (the PR description accepts this risk). The response now includes payer_cardno, the booker's card number, whenever someone else booked or paid for the stay. When the name lookup misses, payer_name and booker_name (L1129–1132) also fall back to that card number. So one typed card number reveals a second card number, and that number can then be looked up at the kiosk too.

The kiosk only needs to show "credit goes to ". Please drop the card number from the response and fall back to null:

Suggested change
const payerCardno = transaction.cardno || booking.bookedBy || booking.cardno;
const isSelfBooked = !payerCardno || payerCardno === booking.cardno;
const payerName = !isSelfBooked
? (bookerNameMap?.get(payerCardno) || payerCardno)
: null;
return {
credit,
is_self_booked: isSelfBooked,
payer_cardno: payerCardno,
payer_name: payerName
const payerCardno = transaction.cardno || booking.bookedBy || booking.cardno;
const isSelfBooked = payerCardno === booking.cardno;
const payerName = isSelfBooked ? null : bookerNameMap?.get(payerCardno) || null;
return {
credit,
is_self_booked: isSelfBooked,
payer_name: payerName
};

(payerCardno is never empty, because booking.cardno is always set, so the !payerCardno branch can go.) Apply the same || null fallback to booker_name at L1131, and check that admin #225 doesn't read payer_cardno.

Comment on lines +1112 to +1125
// Also if any transaction was paid by another card not in bookerNameMap, resolve it
const missingPayerCardnos = transactions
.map((t) => t.cardno)
.filter((c) => c && c !== cardno && !bookerNameMap.has(c));

if (missingPayerCardnos.length > 0) {
const extraPayerCards = await CardDb.findAll({
where: { cardno: missingPayerCardnos },
attributes: ['cardno', 'issuedto']
});
for (const c of extraPayerCards) {
bookerNameMap.set(c.cardno, c.issuedto);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this third, sequential CardDb query can be removed. Batch 2 runs the booker-card query in parallel with the transactions query, so it can't know the payer cards yet. To save a round trip, fetch the transactions first, then run one CardDb.findAll over the union of bookedBy and transaction.cardno values. That's 2 round trips in total instead of up to 3. Also, missingPayerCardnos isn't deduplicated, so wrap it in new Set(...) if you keep this block.

Also note: the room preview now filters category IN ('room','flat'), but manualCheckout (L382) looks the transaction up by bookingid only. Booking ids are UUIDs, so in practice both find the same row, but the preview and the real checkout no longer use exactly the same query.

@claude

claude Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 35s —— View job


Re-review of PR #474 after 3fa97d7

  • Read the new commit
  • Checked it against the last review's findings
  • Posted 1 inline comment
  • Posted this summary

Fixed from the last review:

  • payer_cardno is gone from the checkout preview. payer_name and booker_name now fall back to null instead of a card number.
  • Fewer round trips. The handler now fetches transactions first, then makes one deduplicated CardDb.findAll over the booker and payer cards. That is 2 sequential steps after the first batch instead of up to 3.
  • Transaction lookup matches manualCheckout. The preview now finds transactions by bookingid only, the same as manualCheckout (L383). flatCheckout also filters by category: TYPE_FLAT. Booking ids are UUIDs, so both queries still find the same row.

Should fix:

  1. The booker's card number still reaches the kiosk (L1115). bookedBy is in bookingAttributes (L1044), and withPreview spreads ...plain into each booking. So when someone else booked the stay, the response still includes their card number, under bookedBy instead of payer_cardno. The inline comment has a suggestion that strips it before the spread. Fix this →

Minor: isSelfBooked = !payerCardno || … (L996): payerCardno can't be empty, because booking.cardno is always set. The !payerCardno check never runs.

Still open (risk accepted in the PR description): kiosk accounts can check in or check out any bookingid. Also, admin #225 should show "Already checked in" for guests who scanned in at the gate.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

: null;
const tx = txMap.get(plain.bookingid) || null;
return {
...plain,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The booker's card number still leaks here. payer_cardno is gone and the name fallbacks are now null, but ...plain spreads every selected attribute into the response. That includes bookedBy (L1044), so when someone else booked the stay, the kiosk lookup still returns their card number. That card number can then be typed into the kiosk too, which is the problem the last commit set out to fix.

bookedBy is only needed on the server, to work out booker_name and the payer. Remove it from the response:

Suggested change
...plain,
...omitBookedBy(plain),

with a small helper next to withPreview:

const omitBookedBy = ({ bookedBy, ...rest }) => rest;

Or destructure it inline: const { bookedBy, ...publicFields } = booking.get({ plain: true });, then use bookedBy for booker_name and spread publicFields. Please also check that admin #225 doesn't read bookedBy from this response.

@VitraagVigyaan
VitraagVigyaan temporarily deployed to feat/self-checkin-kiosk - aashray-backend PR #474 November 2, 2026 11:33 — with Render Destroyed
@claude

claude Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @vvshk's task in 41s —— View job


Re-review of PR #474 after 5308882 (and 4bb08aa)

  • Read the two new commits
  • Checked them against the last review's findings
  • Posted 1 inline comment
  • Posted this summary

Fixed from the last review:

  • The kiosk response no longer includes the booker's card number. withPreview now destructures bookedBy out of the plain booking and spreads only publicFields. bookedBy is still used on the server for booker_name, and kioskCheckoutPreview still gets the model instance, so the payer logic doesn't change. I found no other place where this response returns a card number other than the scanned one.

Should fix:

  1. Check-in and check-out are off the gate again, but the PR description still says they aren't (controllers/admin/gateManagement.controller.js). 4bb08aa removes gate-entry check-in and gate-exit flat check-out again, which undoes the 4fee390 revert. The description still says "The gate is untouched", "Kiosk check-in is optional", and "gate entry alone -> stay checked in". With this code, a guest who skips the kiosk stays pending checkin. That breaks member-app WiFi and checkedin reports. Flat stays are never checked out automatically either. Please either revert 4bb08aa, or update the description, evidence and Blast Radius and get the owner's sign-off, since admin hotfix/utsav-admin-creating-mumukshu-registration-bugs #225 was built on the opposite assumption.

Minor (from the last review, still there): isSelfBooked = !payerCardno || …: payerCardno can never be empty, so the !payerCardno check never does anything.

Still open (risk accepted in the PR description): kiosk accounts can check in or check out any bookingid.

I did not run lint or tests: AGENTS.md says dependencies and the test suite aren't available in GitHub Actions reviews.

}
});

await t.commit();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4bb08aa takes check-in and check-out off the gate again, but the PR description still says "The gate is untouched". With this commit, only manualCheckin and flatCheckin move a stay to checkedin. Only manualCheckout and flatCheckout move it to checkedout. That happens through staff or the kiosk, and no cron covers it. This breaks several things the description promises:

  • "Kiosk check-in is optional, so guests who skip it lose nothing except the WiFi code." That is no longer true. A guest who only scans at the gate stays pending checkin. They don't show up in reports that count checkedin stays. fetchBookings in wifi.controller.js needs checkedin, so the member-app WiFi request fails as well, not just the kiosk one.
  • The evidence line "gate entry alone -> stay checked in, no kiosk needed" no longer matches the code.
  • Flat guests who leave through the gate without using the kiosk stay checkedin until staff use the overdue report.

Please do one of these:

  • If this commit was intended (the PR title says "decouple gate checkin"), rewrite the Summary, diagram, evidence and Blast Radius to say the kiosk or staff check-in is now required. Then get the owner's sign-off, because hotfix/utsav-admin-creating-mumukshu-registration-bugs #225 and the 4fee390 revert both assumed the opposite.
  • If it wasn't intended, revert 4bb08aa so this file matches origin/dev again.

@vvshk
vvshk merged commit 4f7d418 into dev Nov 2, 2026
3 checks passed

This branch was successfully deployed

No deployments
feat/self-checkin-kiosk - aashray-backend PR #474 — 53088824 Deployed Nov 2, 2026 by VitraagVigyaan
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants