Skip to content

Travel list: adhyayan session + round-trip#153

Open
vendz wants to merge 4 commits into
devfrom
feat/travel-adhyayan-roundtrip
Open

Travel list: adhyayan session + round-trip#153
vendz wants to merge 4 commits into
devfrom
feat/travel-adhyayan-roundtrip

Conversation

@vendz

@vendz vendz commented Jul 8, 2026

Copy link
Copy Markdown
Member

Travel list: adhyayan session + round-trip

Staff dashboard changes for the travel feature (pairs with the aashray-backend PR).

What's included

  • Replaced the old self-reported "Attending Adhyayan" Yes/No column with the real "Adhyayan (Session)" — shows the traveler's registered study session (name + dates) derived by the backend, or .
  • Added a "Round Trip" column (badge when the booking's legs are linked).
  • Removed the deprecated leaving_post_adhyayan edit control from the booking edit modal.

Requires the aashray-backend PR (returns adhyayan + trip_group_id on the travel list). Header/row column counts kept in sync.

🤖 Generated with Claude Code

…list

Replaces the self-reported "Attending Adhyayan" Yes/No column with the
real study-session registration (name + date range) returned by the
backend, adds a Round Trip badge column, and removes the now-ignored
leaving_post_adhyayan edit control from the transaction edit modal.
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

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


Review of PR #153 — Travel list: adhyayan session + round-trip

  • Gather context (diff, CLAUDE.md, related files)
  • Understand the request (code review)
  • Review fetchUpcomingBookings.html
  • Review fetchUpcomingBookings.js
  • Post inline findings
  • Post summary

Summary

Reviewed the diff against origin/dev. The header/row column counts are correctly kept in sync (1 <th>/<td> removed, 2 added), the deprecated leaving_post_adhyayan field and its edit-modal control are cleanly removed with no leftover references anywhere in the repo, and formatDate/formatDateTime are both available via the shared style/js/formatDate.js include, so no missing-function issue there.

One issue found, posted as an inline comment:

  • admin/travel/fetchUpcomingBookings.js:307-317 — The "Download Excel" button (setupDownloadButton) exports raw travelReport rows keyed by each column's data-key. Since data-key="adhyayan" now points at b.adhyayan, an object ({name, start_date, end_date}) rather than a scalar, XLSX.utils.json_to_sheet will serialize it as "[object Object]" in the exported spreadsheet — a regression vs. the old scalar leaving_post_adhyayan value. The "Round Trip" column has a smaller version of the same mismatch: it will export the raw trip_group_id instead of the "Round trip"/"—" text shown on screen. Suggested a fix of precomputing string display fields for these two columns so the export matches what's rendered.

Everything else (badge markup for "Round trip", modal payload changes, en-dash date range formatting) looked correct and consistent with the rest of the file.

Comment on lines 307 to 317
<td>
${b.leaving_post_adhyayan == 1
? 'Yes'
: 'No'}
${b.adhyayan
? `${b.adhyayan.name} (${formatDate(b.adhyayan.start_date)}–${formatDate(b.adhyayan.end_date)})`
: '—'}
</td>

<td>
${b.trip_group_id
? '<span class="badge bg-info">Round trip</span>'
: '—'}
</td>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Excel export will show [object Object] for the new Adhyayan column.

The "Download Excel" button (setupDownloadButton, line ~416) exports the raw travelReport objects using data-key as the field name:

formattedRow[label] = row[key] ?? '';

Since data-key="adhyayan" now points at b.adhyayan, which is an object ({name, start_date, end_date}) rather than a scalar, XLSX.utils.json_to_sheet will stringify it as "[object Object]" in the exported spreadsheet. Previously leaving_post_adhyayan was a plain 0/1 value, so this regression is specific to this change.

Similarly, the "Round Trip" column will export the raw trip_group_id value instead of the "Round trip"/"—" text shown in the UI — a smaller inconsistency, but still a mismatch between what's displayed and what's exported.

Suggested fix: precompute display-friendly string fields (e.g. adhyayan_label, trip_group_label) on each booking when travelReport is populated, and point the data-key attributes at those instead of the raw nested/id fields, so the export mirrors what's rendered on screen.

Fix this →

…s display strings, not [object Object]/raw id
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

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


Review of PR #153 — Travel list: adhyayan session + round-trip (updated after new commit)

  • Review latest diff (git diff origin/dev...HEAD)
  • Verify the previously-flagged Excel export regression was fixed by bac0f76
  • Re-check header/row column-count sync and script includes
  • Post summary

Summary

Commit bac0f76 correctly resolves the earlier finding:

  • travelReport.forEach (fetchUpcomingBookings.js:197-202) now precomputes adhyayan_label ("<name> (<start>–<end>)" or ) and trip_group_label ("Round trip" or ) on each booking.
  • The header cells were updated to data-key="adhyayan_label" / data-key="trip_group_label" (fetchUpcomingBookings.html:161-162), matching how getColumnOrderAndLabels/downloadExcelFromJSON in style/js/exportToExcel.js pull row[key] by the th's data-key. The Excel export now mirrors the rendered table instead of dumping [object Object]/raw ids.
  • Header (th) count and rendered td count both come out to 25, so the sync is still correct after adding the new trip_group_label column.
  • formatDate/exportToExcel.js are both included via <script> tags in the HTML, so no missing-dependency issue.

No new bugs introduced by the fix. One very minor, non-blocking observation: the "<name> (<start>–<end>)" template string is now duplicated in two places (fetchUpcomingBookings.js:199 and :317) — could be factored into a small helper if this file grows further, but not worth blocking on for a change this size.

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

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


Review of PR #153 — Travel list: adhyayan session + round-trip (re-review after commit 27f5198)

  • Fetch latest diff (git diff origin/dev...HEAD)
  • Review new commit 27f5198 (hyphen placeholders / regex escape)
  • Re-verify previous findings still hold
  • Post inline findings (if any)
  • Post summary

Summary

Commit 27f5198 is a small, purely cosmetic follow-up:

  • Empty-state placeholders for adhyayan_label / trip_group_label (both the precomputed export fields at fetchUpcomingBookings.js:199-201 and the rendered <td> markup at :317-321) changed from an em dash () to a plain hyphen (-). The two locations stay consistent with each other, so the on-screen table and the Excel export still match.
  • The normalize() dash-stripping regex at fetchUpcomingBookings.js:212 was rewritten from literal [–—] characters to explicit / escapes — functionally identical (still matches en dash + em dash), just avoids embedding non-ASCII dash glyphs in the source.
  • normalize() is only applied to pickup_point/drop_point search matching (lines 227-228), so this change has no interaction with the adhyayan/trip-group fields.

No new bugs introduced. The Excel-export regression flagged in the earlier review round (raw object/id being exported instead of display text) remains fixed by bac0f76, and header (th)/row (td) counts are still in sync (25/25).

One purely cosmetic nit, not worth blocking on: the date-range separator in the rendered label (${formatDate(...)}–${formatDate(...)}, lines 199 and 317) still uses a literal en dash character rather than a plain hyphen — inconsistent with this commit's stated goal of avoiding non-hyphen dash glyphs, though it doesn't affect functionality.

No inline comments filed for this round — the change is safe to merge as-is.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


Review of PR #153 — Travel list: adhyayan session + round-trip

  • Fetch latest diff (git diff origin/dev...HEAD)
  • Review new commit 42836e8 (Vendor guest type option)
  • Re-verify previous findings still hold
  • Post inline findings
  • Post summary

Summary

Re-reviewed after the latest commit 42836e8, which is unrelated to the adhyayan/round-trip travel feature — it adds a "Vendor" option to the Guest Type dropdown in admin/card/createCard.html and admin/card/updateCard.html.

  • createCard.html:194 — live change, works as intended.
  • updateCard.html:197 — flagged inline: this edit lands inside an HTML comment block (`` spanning lines 189–200), so the new <option> has no effect on the rendered update form. Likely unintentional given the addition mirrors the live createCard.html change.

All previously-flagged issues remain resolved:

  • The Excel export regression (raw adhyayan object / trip_group_id exporting as [object Object]/raw id) is still fixed via the precomputed adhyayan_label/trip_group_label fields (fetchUpcomingBookings.js:196-203), and header/row data-keys match.
  • Header (th) / row (td) counts remain in sync.
  • The dash-normalization regex and hyphen placeholders introduced in earlier commits are unaffected by this change.

No other issues found in this round.

<option value="Friend">Friend</option>
<option value="Driver">Driver</option>
<option value="VIP">VIP</option>
<option value="Vendor">Vendor</option>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This <option value="Vendor">Vendor</option> is added inside an HTML comment block — the surrounding guestType select (lines 189-200) is entirely wrapped in ``, so this change has no effect on the rendered page. The equivalent addition in admin/card/createCard.html:194 is live since that file's `guest_type` select isn't commented out.

If Vendor should actually be selectable on the update-card form, the whole commented-out block needs to be uncommented first (which is a bigger change than adding one option) — otherwise this edit can be dropped since it's a no-op.

Fix this →

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.

2 participants