su-fit-vut / kachna-online

Students' Club U Kachničky: Member's portal
MIT License
5 stars 5 forks source link

Add proper state manipulation logging #101

Open ondryaso opened 2 years ago

ondryaso commented 2 years ago

In the original design, state modifications did not change the state's MadeById. However, it is actually quite important that the state carries information about the last person that has changed it – because chillzone take-overs and prolongations are perfomed by modifying the current state. On the other hand, this approach doesn't keep history of people opening chillzones which is something that is higly required for the purpose of analysis and rewarding active SU members.

There are two possible options:

I'm really not sure which way is better. Planning new states is a bit less 'natural' – you are not planning a new state, you're just changing the person on the current one. From the user's point of view, it would also bloat the calendar/states list. Also, notification sending would have to be adjusted so that 'modifying' the state this way would not send a notification. This could be partially improved by implementing a form of optional state aggregation on the backend – the frontend could ask for an aggregated version of states list that would group together states of the same type linked by the following state ID attribute. It would have to be optional so that the administrators' state list could still differentiate the states. The thing is, I'm afraid this would bring even more complexity to the already quite hard-to-understand codebase in ClubStateService.

The other option would certainly be much easier to implement. Basically, the serilog logging commands in state management would be replaced/complemented with a simple database insert. Nothing would be changed on the frontend and in the state presentation logic. However, the past states list in the admin section would still provide possibly misleading information about who opened what state – so it should be revamped so that it provides more information about the states – so the states API would have to change anyway (this could be done most non-instrusively by introducing a new endpoint that would return modification history for a single specified state). Also, this would result in certain data redundancy.

I'm probably more leaning towards the second option as it is easier to implement without extensive changes to the states logic. Still, I'll have to think the details out thouroughly.