Edit scheduleEntry on activity view without redirection problem

[manuelmeister]

Now we have a better fallback for wrong/missing ids for activities and scheduleEntries. If a scheduleEntry is not found, but the activity is found, we load the first occurring scheduleEntry of the activity. If for whatever reason we can't find the activityId, then we try to find it using the scheduleEntryId.

This now also works if we navigate from scheduleEntry A to scheduleEntry B, then delete scheduleEntry A and use the browser back. Before this would just display our 404.

Replaces #4500

[github-actions]

✅ Feature branch deployment ready!

Name	Link
😎 Deployment	https://pr4975.ecamp3.ch/
🔑 Login	[email protected] / test
🕒 Last deployed at	Fri Feb 21 2025 23:37:37 GMT+0100
🔨 Latest commit	fcb536f9e8044ae0fd4f1d8e5ebcd13a4eb8dfb3
🔍 Latest deploy log	https://github.com/ecamp/ecamp3/actions/runs/13466343674/job/37633065920
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

[BacLuc]

Looks pretty good.
Is robust, even switches the day when you switch the day of the currently displayed scheduleentry.

but if someone else deletes the scheduleEntry you currently display, and then you press "Submit" (or aktualisieren), then it ends in an infinite loop. But it works if you navigate to a scheduleEntry which was deleted.
maybe you need one + renavigation more.

[manuelmeister]

but if someone else deletes the scheduleEntry you currently display, and then you press "Submit" (or aktualisieren), then it ends in an infinite loop. But it works if you navigate to a scheduleEntry which was deleted.

maybe you need one + renavigation more.

I don't understand. Wasn't this a problem before? For that we would probably need a callback on the entity, that triggers on deletion.

[BacLuc]

but if someone else deletes the scheduleEntry you currently display, and then you press "Submit" (or aktualisieren), then it ends in an infinite loop. But it works if you navigate to a scheduleEntry which was deleted.
maybe you need one + renavigation more.

I don't understand. Wasn't this a problem before? For that we would probably need a callback on the entity, that triggers on deletion.

No before, this lead to a 404. (which is correct i think)
It is a weird edgecase.
No: you yourself cannot delete the scheduleentry you are displaying not in this tab.
But you can do it in another tab (with a completely different javascript runtime), so callbacks on delete don't help.

[manuelmeister]

@BacLuc so how would you fix this? Do you think this is a big issue?

[BacLuc]

You could catch the 404 for the not existing scheduleentry and change the route to an error page.
I don't know if we need to fix this...its for the team to decide.

[BacLuc]

Here is the video for you

Screencast.from.2025-02-10.23-39-19.webm

[manuelmeister]

@BacLuc it works now. I just check each scheduleEntry patch. If the request returns 404 and it is the current one, we redirect to an existing one.

[BacLuc]

shouldn't scheduleEntry be here, when its used as a prop?

[BacLuc]

minor: i find it a little silly that you mix the await syntax with then

[carlobeltrame]

I find it perfectly valid in use cases where writing it with only await would be more complicated, especially when you have to use then and catch at the same time (which would translate to await with a try {} catch {} block). In particular, I think then is more useful and readable when you have to continue working with a promise (e.g. when chaining multiple transformations or steps), while await is useful when you are only interested in the result payload of the promise.
Regardless of code style preferences, we have this mixed syntax already in various places in the frontend as well as in hal-json-vuex.

[simfeld]

I don't understand it fully but something doesn't quite add up in the "browser back when previous schedule entry deleted" case: it finds another scheduleEntry of the activity, but some data is not refreshed properly. See screenshot where the title and time slot matches with a green LS entry but the badge says blue LP.