Reproduce issue

[Tschuppi81]

Org: Fix reservation form crashing on expired session

Move remove_expired_reservation_sessions to run before bound_reservations so an expired session yields a clean 403 instead of potentially crashing when the template accesses attributes of a detached SQLAlchemy object.

TYPE: Bugfix
LINK: https://seantis-gmbh.sentry.io/issues/7432727459/activity/

[Tschuppi81]

@Daverball Claude helped to potentially fix this issue.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.48%. Comparing base (5f39f21) to head (398e614).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Files with missing lines	Coverage Δ
src/onegov/org/views/reservation.py	68.63% <100.00%> (+0.03%)	⬆️

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f39f21...398e614. 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]

That is one the possible ways I suggested solving this in the sentry issue. It may be a little controversial though, that we're mutating things on GET requests now.

I still think the better solution is to move this cleanup into a cronjob and to do it for all resources at once. It's also a little weird that this is a method on the resource. We have all our other maintenance tasks on objects that can expire in cronjobs.

This is fine as a quick fix, but I would probably put a second if request.POST condition around this call, if we're going with this, just so we're not cleaning up more often than we did before.

[Tschuppi81]

That cron job would need to run every 15 minutes then, isnt't it a little heavy as the issue might happen very rarely?

[Daverball]

I think the expiration time is a little too tight right now anyways, especially if there's a lot of information to input. An hour would be perfectly fine, or even multiple hours, I'm honestly not sure why we have set it up this tightly, it's not very user friendly. It might be so it's less likely to block the deletion of an allocation/resource, but I'm not even sure if it does block the deletion of an allocation. And deleting a resource is rare, you can just set it private first and then delete it later, after everything has expired.

With the current implementation it's more likely that some stale reservations will stick around, since we only clean them up on demand in a couple of views. So you need to know which views to hit in order to clean them up.