VSEScala / Scala-Dining

Contains the Scala Dining Website Design
4 stars 5 forks source link

min_diners #127

Closed mhvis closed 1 year ago

mhvis commented 5 years ago

Currently, min_diners doesn't do anything other than hiding the delete button (it doesn't block deletion). It's also possible for dining list owners to always change this value afterwards to the highest possible value in order to make deletion possible again, or simply remove diners until deletion is possible again. In that sense it's actually just a constant for the highest possible value and we could just leave it out and replace it by MAX_SLOT_DINER_MINIMUM. I'd like to rethink this and possibly remove min_diners, because it can be confusing to users because they don't know why they have to provide it and what it is used for, as well as we'd like to have dining list creation and editing as easy as possible, by only presenting useful fields and not confusing someone. (As well as that it can be easily circumvented)

Simple change would be to remove the field and use the MAX_SLOT_DINER_MINIMUM constant instead, i.e. always have it set to 6. However I'd propose to rethink this user requirement to something else as there can always be a reason that a dining list is cancelled and thus it should always be possible to cancel regardless of the number of diners.

~I'm thinking of something like this: it should only be possible to delete a dining list if no (manual) sign ups have taken place yet. As soon as someone else has shown interest in the dining list by joining we disable removal and instead change it into a cancel button that marks the dining list as cancelled. Removing all diners would also not re-enable deletion. Haven't thought this through fully yet though.~

Later edit: probably better would be to completely not allow deletion so that dining lists can only be marked as cancelled, that's still easier for users since there are no 2 different concepts of deletion and cancellation, and cases where a dining list must be deleted will be seldom and can easily be handled manually via the admin interface. E.g. when a user made a test dining list and directly cancelled it afterwards we can manually remove it or even just leave it since it wouldn't bother anyone.

DutcherNL commented 5 years ago

Due to all the processes involved with dining lists, tracking user help statistics, creating transactions, tracking how many members ate, and the limited amount of daily slots, I'm not really a fan of allowing dining lists to be cancelled. Yes, we can rewrite all of those, but why would we. Yes, there could be other cases where a large dining list can be cancelled - we can always do that in the admin or something or allows boards to do that on a new page in their association screen (as there are already plans to allow associations to change owners of dining lists) - but the intention of min-diners was never solely the intention of when users can and can not delete dining lists, but to communicate when dining lists could be deleted if too few people are present. That said, I'm game if we remove min-dining and say in the guidelines that dining lists can be deleted by owners as long as less than 6 members are on the dining list or by board members in special cases.

mhvis commented 5 years ago

What about this?

However I'd propose to rethink this user requirement to something else as there can always be a reason that a dining list is cancelled and thus it should always be possible to cancel regardless of the number of diners.

There can always be a good reason for why a dining list needs to be cancelled, e.g. problems in the kitchen or something like that. Now I get that in these exceptional cases an admin could do that manually, however what will happen is that people are not going to ask an admin to cancel a dining list, instead they will post a message to the dining list with the reason and maybe just remove everyone because that is the easiest solution for this (also the dining list owner can remove people to get under 6 members and still delete the dining list this way..).

To prevent this from happening we should make sure that the easiest solution for the user for removing/canceling a dining list is also the solution that we want to happen. A good way for that is to just have a prominent cancel button (always available) where the user should provide a message.

Note that rewriting to allow dining lists cancellation is not that hard and we should always consider user functionality above technical feasibility. For the rewrite it is probably enough to change the default model manager to only return non-cancelled dining lists, so that the already existing code doesn't ever encounter cancelled dining lists.

mhvis commented 5 years ago

To expand on my last paragraph above: if your only objection against cancellation of dining lists is because it's hard to implement, I can deal with that.

DutcherNL commented 5 years ago

On the discussion of cancellation vs removal: It's not that it's hard to implement, it's that it's annoying to take into account as it creates another layer of complexity (which, yeah, is covered by calling .filter(cancelled=False), but still. We also need to rewrite views to no longer be visible on cancelled dining list, make sure that the link loads the right view (incase there is a cancelled and non-cancelled dining list of the same association on the same day etc etc). However, I do not know the upside of cancellation vs removal. There is no reason to keep the cancelled dining list in the view so it mainly exists in the database (with or without all user entries) where it has no function. In the UX it doesn't appear to make a difference (or I have missed something). For database maintenance I see no reason why we should keep a cancelled dining list. The information on who was on that list offers very little use. As a result I do not think that the positives outweigh the effort required to take cancelled dining lists into account. If you want to track some dining list removal info, we can always just create a new model DeletedList(date, owner, association, diner_count, deletion_reason) which might be nicer in code anyhow.

On the limit of diners for deletion A thing which I haven't adressed in this thread, though possibly did many months ago. If you present an option, people will start thinking with that option. In this case, if you present the option to delete a dining list (even though there are 12+ people on it), the option to delete it will always be there. Now I'm not exactly sure what my stance is on this view to be honest. I see upsides and downsides. Maybe associations will claim a slot and depening on who joins ask around who can cook, if no cooks are found cancell the dining list. It's basically what the Knights used to do in the bunker. It's not a bad system, though with multiple associations present now it could make dining lists more fickle and less trustworthy.

mhvis commented 5 years ago

It's not that it's hard to implement, it's that it's annoying to take into account as it creates another layer of complexity (which, yeah, is covered by calling .filter(cancelled=False), but still. We also need to rewrite views to no longer be visible on cancelled dining list, make sure that the link loads the right view (incase there is a cancelled and non-cancelled dining list of the same association on the same day etc etc).

It's easier than this. One can change the QuerySet of the default model manager (objects) to only return non-cancelled dining lists. Then each existing query will use that model manager and you don't need to add filter(cancelled=False) to each existing query. You also don't need to rewrite any views, e.g. the cancelled/non-cancelled on the same day conflict won't occur as the cancelled dining list is not returned in the query, as that query uses the default model manager which doesn't return cancelled dining lists. To have certain views display cancelled dining lists we can add a different model manager that has a queryset with cancelled dining lists and use this whenever we want to explicitly display these cancelled dining lists.

This solution also goes for the dining entries.

However, I do not know the upside of cancellation vs removal. There is no reason to keep the cancelled dining list in the view so it mainly exists in the database (with or without all user entries) where it has no function. In the UX it doesn't appear to make a difference (or I have missed something). For database maintenance I see no reason why we should keep a cancelled dining list. The information on who was on that list offers very little use. As a result I do not think that the positives outweigh the effort required to take cancelled dining lists into account. If you want to track some dining list removal info, we can always just create a new model DeletedList(date, owner, association, diner_count, deletion_reason) which might be nicer in code anyhow.

It's indeed to track (and display!) the info, so that the dining list is not fully gone and the user not only knows about it via e-mail. A new model for that could definitely be a good solution, in this case I could try implementing both methods and see which we prefer. If the above solution of not having a separate model does not lead to massive rewrites it has the advantage that the code will likely be shorter since there's no separate table that has almost the same columns, as well as that all data is kept, for whenever we might need to investigate something. Other than having to rewrite, which indeed might be hard so we'll have to see about that, there's no performance penalty and usually always better to keep as much data as possible (possibly anonimizing to obey with privacy rules).

On the limit of diners for deletion A thing which I haven't adressed in this thread, though possibly did many months ago. If you present an option, people will start thinking with that option. In this case, if you present the option to delete a dining list (even though there are 12+ people on it), the option to delete it will always be there.

We could change the button to a small button in the bottom right corner of the change settings page, I kinda like that actually so I'll do that.

Now I'm not exactly sure what my stance is on this view to be honest. I see upsides and downsides. Maybe associations will claim a slot and depening on who joins ask around who can cook, if no cooks are found cancell the dining list. It's basically what the Knights used to do in the bunker. It's not a bad system, though with multiple associations present now it could make dining lists more fickle and less trustworthy.

Good point, I have to think about this

mhvis commented 5 years ago

I've been thinking about the last point. Still, it remains that there might be genuine reasons why a dining list needs to be closed, no matter the number of people that signed up. We wouldn't want in that case to have the dining list owner do some tricks like removing all entries or simply posting a message and leaving the dining list open (which I think happened once for a Q dining list, but not sure). When we allow cancellations we will of course require that the user provides a good reason for the cancellation. In the user agreement and on the cancellation page we could include that a user needs a genuine reason to cancel a dining list, which does not include that there are no cooks available. Then when we encounter a cancellation which does not appear genuine, that cancellation would then need to be investigated and maybe result in penalties. That's obviously not what we want. We can however expect from people that if we communicate the terms for cancellation clearly (e.g. using a message on the cancellation page), that the majority of the people will not be mean and handle according to the terms.