From 420e5502fdd9b91cd877c509046db97c214f0599 Mon Sep 17 00:00:00 2001 From: Josh Deeden Date: Thu, 6 Aug 2026 10:19:04 -0700 Subject: [PATCH] Note the assumed duration in the calendar feed (#498) A v-event needs an end time, so addDuration() adds an hour when the organizer gave no duration. Nothing said so, which left a guessed end time looking exactly as definite as a stated one. Add a line to the description saying the hour was assumed, but only when timedetails is empty. timedetails is free text about timing -- "meet 7pm, roll 7:30" -- and it prints on the line directly above, so repeating the point there reads as contradicting the organizer. Per AllEvents.md the one hour default and the timedetails line arrived together in #339, so they were meant to sit side by side. Also drop a stray global: addDuration was assigning an undeclared endTime on every call, which would throw under strict mode or ESM. Same pattern as #1089, in the function this change had to touch. Adds three cases to ical_test.js, and exports buildCalEntry for testing since the existing fixtures all hardcode a duration. --- app/endpoints/ical.js | 16 +++++++++++++ app/models/calEvent.js | 8 ++++++- app/test/ical_test.js | 52 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/app/endpoints/ical.js b/app/endpoints/ical.js index 82c18353..8d2e9299 100644 --- a/app/endpoints/ical.js +++ b/app/endpoints/ical.js @@ -24,12 +24,19 @@ const { EventsRange } = require("../models/calConst"); const dt = require("../util/dateTime"); const config = require("../config"); +// shown in the description when the organizer didn't give a duration +// and the feed had to assume one. re: #498 +const UNSPECIFIED_DURATION = + "The organizer didn't specify an end time, so this entry assumes 1 hour."; + module.exports = { // endpoint export: get, // export for testing: escapeBreak, replace, + buildCalEntry, + UNSPECIFIED_DURATION, }; // text|escapeBreak("HEADER") => HEADER:text @@ -249,6 +256,15 @@ function buildCalEntry(evt, at) { description: [ news, evt.descr, evt.timedetails, + // a v-event needs an end time, so addDuration() invents one hour when the + // organizer didn't give a duration. say so: otherwise the entry looks just + // as definite as one with a real end time. re: #498 + // + // only when there's no timedetails though: that's free text like + // "meet 7pm, roll 7:30" and it prints on the line above. saying the + // organizer gave no end time directly under their own note about the + // timing reads as a contradiction. + (!evt.hasDuration() && !evt.timedetails) ? UNSPECIFIED_DURATION : null, evt.locend? "Ends at "+ evt.locend: null, url ], diff --git a/app/models/calEvent.js b/app/models/calEvent.js index d004edfc..f58d9176 100644 --- a/app/models/calEvent.js +++ b/app/models/calEvent.js @@ -108,7 +108,13 @@ const methods = { // generates a 1 hour duration if none was specified. addDuration(start) { const len = this.eventduration; - return endTime = (len > 0) ? start.add(len, 'minute') : start.add(1, 'hour'); + return (len > 0) ? start.add(len, 'minute') : start.add(1, 'hour'); + }, + + // true when the organizer gave an explicit duration; + // false when addDuration() would have to invent one. + hasDuration() { + return this.eventduration > 0; }, // remove this record and any associated caldaily(s) from the database. diff --git a/app/test/ical_test.js b/app/test/ical_test.js index 9fc1c723..e58809fe 100644 --- a/app/test/ical_test.js +++ b/app/test/ical_test.js @@ -6,6 +6,8 @@ const testdb = require("./testdb"); const { CalEvent } = require("../models/calEvent"); const { CalDaily } = require("../models/calDaily"); const { EventStatus } = require("../models/calConst"); +const ical = require("../endpoints/ical"); +const dt = require("../util/dateTime"); // const { describe, it, before, after } = require("node:test"); const assert = require("node:assert/strict"); @@ -281,3 +283,53 @@ String.raw`BEGIN:VCALENDAR`, String.raw`END:VCALENDAR`, "" // trailing new line. i think. ].join("\r\n"); + +// buildCalEntry is a pure function, so these need no database. +// re: #498 -- and the "nil and zero" todo in "can handle a canceled event". +describe("ical duration", () => { + const day = dt.fromYMDString("2002-08-01"); + + // build the pieces buildCalEntry() needs for one occurrence. + function entryFor(eventduration, timedetails = null) { + const evt = CalEvent.wrap({ + id: 1, title: "Test Ride", name: "organizer", + descr: "details", eventtime: "19:00:00", eventduration, timedetails, + created: day.toDate(), modified: day.toDate(), changes: 0, + }); + const at = CalDaily.wrap({ + pkid: 201, id: 1, eventdate: day.toDate(), + eventstatus: EventStatus.Active, newsflash: null, + }); + return ical.buildCalEntry(evt, at); + } + + it("notes the assumed duration when the organizer gave none", () => { + for (const none of [null, 0, undefined]) { + const entry = entryFor(none); + assert.ok(entry.description.includes(ical.UNSPECIFIED_DURATION), + `expected the note for eventduration ${JSON.stringify(none)}`); + // the invented end time is one hour after the start + assert.equal(entry.end.diff(entry.start, 'minute'), 60); + } + }); + + it("says nothing when the organizer gave a duration", () => { + const entry = entryFor(90); + assert.ok(!entry.description.includes(ical.UNSPECIFIED_DURATION), + "did not expect the note when a duration was supplied"); + assert.equal(entry.end.diff(entry.start, 'minute'), 90); + }); + + // a ride without a duration may still describe its timing in free text, + // which prints just above this note; repeating it there reads as a + // contradiction. re: #498 + it("says nothing when timedetails already covers the timing", () => { + const entry = entryFor(null, "meet 7pm, roll 7:30"); + assert.ok(!entry.description.includes(ical.UNSPECIFIED_DURATION), + "did not expect the note alongside timedetails"); + assert.ok(entry.description.includes("meet 7pm, roll 7:30"), + "expected the organizer's own timing note to still be there"); + // the end time is still assumed; we just don't say so twice. + assert.equal(entry.end.diff(entry.start, 'minute'), 60); + }); +});