Note the assumed duration in the calendar feed (#498)

[gangster]

Fixes #498.

A v-event needs an end time, so addDuration() invents one hour when the organizer didn't give a duration. Nothing said so, which meant a listing with a guessed end time looked exactly as definite as one with a real end time.

This adds a line to the description in that case:

The organizer didn't specify an end time, so this entry assumes 1 hour.

The wording is a guess and easy to change. The issue says "consider adding something", so I picked something plain, in the voice of the confirmation email, using "organizer" as the rest of the app does. Say the word and I will swap it.

The change

ical.js gains a constant and one conditional entry in the description array:

description: [
  news,
  evt.descr, evt.timedetails,
  (!evt.hasDuration() && !evt.timedetails) ? UNSPECIFIED_DURATION : null,
  evt.locend? "Ends at "+ evt.locend: null,
  url
],

escapeBreak already drops null entries, so events with a duration are untouched.

calEvent.js gains the hasDuration() predicate, and loses a stray global:

-    return endTime = (len > 0) ? start.add(len, 'minute') : start.add(1, 'hour');
+    return (len > 0) ? start.add(len, 'minute') : start.add(1, 'hour');

addDuration was assigning an undeclared endTime, creating an implicit global on every call. Same pattern as #1089, in the function this issue required me to reason about, so I fixed it here rather than filing a fourth one-line issue. Happy to split it out if you would rather keep this focused.

Why it also checks timedetails

timedetails is organizer-written free text about timing — "meet 7pm, roll 7:30", "leave 7:30 ish" — and it prints on the line directly above this one. Saying "the organizer didn't specify an end time" immediately under the organizer's own note about the timing reads as the feed contradicting them.

That reading is supported by where the one hour default came from. docs/AllEvents.md traces it to the wishlist in #339, where it arrived in the same batch as the rest of the description:

• default duration of 1 hour
• a link to the listing
• time details (e.g. "Meet 6:00pm, ride at 6:30pm)
• location details (e.g. "Meet at the picnic tables")

The assumed duration and the time details line were added together, so treating timedetails as the thing that already speaks to the timing seems closer to the original intent than an accident of the code.

Worth flagging that this is still a departure from a literal reading of the issue, which says to note the assumption without qualifying when.

The cost of that choice, stated plainly: the DTEND is invented in every case where there's no duration, including the ones with timedetails. So those rides keep an unflagged assumed end time. I went narrow because widening later is a one-line change nobody notices, whereas walking it back means subscribers already saw it on entries where it wasn't wanted.

Which way this should go depends on how often each case occurs across the real calendar — how many published rides have no duration, and how many of those also have no timedetails. You can see that; I can't from here. If it turns out the narrow condition barely ever fires, drop the && !evt.timedetails and it applies to every assumed duration.

Tests

Three cases added to ical_test.js, bringing the suite to 56:

• notes the assumed duration when the organizer gave none (null, 0 and undefined)
• says nothing when the organizer gave a duration
• says nothing when timedetails already covers the timing — and checks the end time is still assumed at +60min, since this suppresses the message, not the behaviour

They go through buildCalEntry rather than HTTP, because all three ical fixtures hardcode eventduration: 60 and changing one would churn the golden strings the existing tests compare against. buildCalEntry is now exported "for testing" alongside escapeBreak and replace.

This also answers the standing todo in "can handle a canceled event":

todo: create a separate test where these values are nil and zero.

Being straight about what these prove: "notes the assumed duration" fails on unpatched source for the right reason. The other two also fail there, but partly because buildCalEntry is not exported yet — they are regression guards rather than evidence of the bug.

Verification

The feed is consumed by real calendar clients, so I checked more than the string appearing.

Structure, across every feed shape, with a small RFC 5545 check (CRLF endings, 75-octet fold limit, BEGIN/END:VEVENT balance, well-formed content lines):

feed	vevents	lines over 75 octets	notes added	result
all events	10	0	3	ok
pedalpalooza (incl. the synthetic closing event)	11	0	3	ok
date range	10	0	3	ok

The fold limit is the one that mattered, since this lengthens a description. The existing escapeBreak handles it.

Whole-feed diff against unpatched main, compared unfolded so folding noise could not hide anything:

lines only in PATCHED : 3   (the three occurrences with no duration)
lines only in BASELINE: 3   (their previous DESCRIPTION lines)
vevent count before/after: 10 / 10

Three lines changed, nothing else — no timestamp drift, and no effect on the events that do have durations.

Parsed with node-ical, to check the output against a real consumer rather than my own reading of the spec. Installed in a scratch directory outside the repo — no dependency added, nothing in any package.json.

BASELINE: parsed OK, 10 VEVENT(s), 0 malformed
PATCHED : parsed OK, 10 VEVENT(s), 0 malformed

Comparing the parsed objects per uid, on start, end, summary, location, status and url:

descriptions changed: 3
other fields changed: 0

And the note lands on exactly the events that need it:

event-1     60min  note=true      event-8    180min  note=false
event-2     60min  note=true      event-4     90min  note=false
event-3     60min  note=true      event-5     90min  note=false
event-6    180min  note=false     event-9    180min  note=false
event-10   270min  note=false     event-7    180min  note=false

The pedalpalooza feed parses too — 11 VEVENTs, 0 malformed, synthetic closing event intact.

What a subscriber sees, as node-ical decodes it:

I'll copy the wireless AGP firewall, that should matrix the API monitor!
The organizer didn't specify an end time, so this entry assumes 1 hour.
http://localhost:3080/calendar/event-1

The comma and newline escapes round-trip correctly. I did not subscribe an actual calendar client (Google, Apple) to the feed, so that remains untested.

One more thing worth a decision

Cancelled occurrences get the note too. That falls out of the code path rather than being a decision: such an entry reads CANCELLED: <title> with STATUS:CANCELLED and then the assumed-duration line. Harmless as far as I can tell, and arguably still accurate, but happy to suppress it there if you would rather.

One thing I noticed but did not change

For an event with no duration the events endpoint returns endtime: null, while the ical feed asserts a real DTEND an hour out — getEndTime() and addDuration() answer the same question differently. calEvent.js already carries a comment doubting the split:

// FIX? just like the php version, if the duration is null the end time is null.
// this seems wrong to me -- it should probably use the minimum 1 hour duration.

Out of scope here, but this issue sits right on top of it, so it seemed worth surfacing.