Adds reservation cancellation path to resources

[Tschuppi81]

Reservation: Adds reservation cancellation path to resources

TYPE: Feature
LINK: ogc-3064

[linear]

OGC-3064

[codecov]

Codecov Report

❌ Patch coverage is 94.40559% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.55%. Comparing base (c4d89df) to head (a70be16).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/onegov/org/views/reservation.py	95.09%	5 Missing ⚠️
src/onegov/town6/views/ticket.py	75.00%	2 Missing ⚠️
src/onegov/org/models/ticket.py	92.85%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/onegov/org/forms/__init__.py	100.00% <100.00%> (ø)
src/onegov/org/forms/resource.py	80.22% <100.00%> (+0.07%)	⬆️
src/onegov/org/forms/resource_recipient.py	96.07% <100.00%> (+0.07%)	⬆️
src/onegov/org/forms/ticket.py	97.43% <100.00%> (+0.56%)	⬆️
src/onegov/org/models/recipient.py	100.00% <100.00%> (ø)
src/onegov/org/views/resource_recipient.py	88.23% <ø> (ø)
src/onegov/org/views/ticket.py	85.45% <ø> (ø)
src/onegov/reservation/models/resource.py	90.77% <100.00%> (+0.04%)	⬆️
src/onegov/org/models/ticket.py	89.14% <92.85%> (+0.04%)	⬆️
src/onegov/town6/views/ticket.py	79.16% <75.00%> (-0.53%)	⬇️
... and 1 more

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4d89df...a70be16. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Daverball]

Currently this seems to be limited to cancelling all reservations in a ticket, which should cover individual reservations nicely, but does that cover 90+% of customer requests or is it more common, or almost as common that for large half year reservations, they want to cancel a single date?

If so, we probably need to allow customers to select which dates they want to cancel, which would also necessarily change the UI in the ticket.

[Tschuppi81]

Now cancellation requests can select single reservations to be cancelled and the manager sees the request cancellation dates, then approve it.

[Daverball]

Looks good overall, although we should consider if and how we should notify a ticket's assignee and/or resource recipients.

There's also couple of minor improvement suggestions and a couple of questionable conditions, that make the feature less flexible.

[Tschuppi81]

Now, as I review the changes i think the reply_to address shall not be notified about cancellation requests, as it is meant to reply to from an email. As we now have the ability to subscribe to cancellation request we should rather keep it separate.

[Daverball]

Looks good

There's still a couple of things we should consider. E.g. what happens if a customer requests a cancellation and by the time the ticket gets processed the reservation has already happened.

Technically it's harmless to cancel past reservations. But for billing purposes, if there's a mix of past reservations that were cancelled too late and future ones that can still be cancelled. You may want only the future ones to get cancelled, when cancellation gets approved. It might be fine to teach supporters to manually cancel reservations that can still be cancelled and reject the request, but it's definitely worth a discussion whether that's good enough, especially since the customer-facing notifications are going to be confusing, so they'd probably also want to mute the ticket and then send a manual message at the end.

[Daverball]

This is one of those cases where I would just write the update statement in raw SQL, it's far easier to get it right that way, we don't want any ambiguity about what the statement looks like at the end and whether or not it will have parameter bindings or is fully baked.

In this case the query should be fully baked, but I think this will probably result in a query with parameter bindings.

See e.g. the following upgrade task:

onegov-cloud/src/onegov/reservation/upgrade.py

Lines 153 to 167 in e94d5f1

	@upgrade_task('Translate default views to their new names')
	def translate_default_views_to_their_new_names(
	context: UpgradeContext
	) -> None:
	if context.has_table('resources'):
	context.operations.execute(text("""
	UPDATE resources SET content = jsonb_set(
	content, '{default_view}', '"dayGridMonth"'
	) WHERE content->>'default_view' = 'month';
	"""))
	context.operations.execute(text("""
	UPDATE resources SET content = jsonb_set(
	content, '{default_view}', '"timeGridWeek"'
	) WHERE content->>'default_view' = 'agendaWeek';
	"""))