svthalia / concrexit

Thalia Website built on Django.
https://thalia.nu
Other
23 stars 11 forks source link

Move event attendance out of members app #1342

Open JobDoesburg opened 3 years ago

JobDoesburg commented 3 years ago

Describe the change

Currently, the Profile model contains a field that states whether Members are allowed to attend borrels and/or events. This information, however, is only used/relevant for the events app. Therefore it would be nicer to separate logic, just as we did with PaymentUsers in payments:

Basically just redo #1320 / #1277 on payments

Motivation

Separation of duties. The Member model shouldn't contain information that only has a meaning in Events

se-bastiaan commented 3 years ago

Personally I do not believe you really need a separate admin with actions for this, since there are only a few users that are affected by these changes.

se-bastiaan commented 3 years ago

I'm not sure if this is really the best idea for the can_attend_borrels value. Since we don't use it in the website it should either be removed or kept the way it is.

I have no clue if it is even used.

JobDoesburg commented 3 years ago

We are using a permission for can use Thalia Pay so I still think this could be nice...

Maybe we should refactor can attend events to can place event registrations as to explain the functionality a bit better.

JobDoesburg commented 3 years ago

Also, maybe we should disallow payments for borrels via the sales app... Hmmm...

se-bastiaan commented 3 years ago

No. That is a bad idea. Then you can never use the sales app for anything else. If you want to do that I suggest you start thinking about doing it in a way that does not increase the circular dependencies.

But yes, the permission idea is still better. But I am really against subclassing the Member object in another place.

JobDoesburg commented 3 years ago

No. That is a bad idea. Then you can never use the sales app for anything else. If you want to do that I suggest you start thinking about doing it in a way that does not increase the circular dependencies.

Yes that's exactly the problem. I see the sarcasm went missing in my comment (also because I do think it's actually a problem, but not a problem that can be solved in a nice way and definitely not by using that permission check)

JobDoesburg commented 3 years ago

But I am really against subclassing the Member object in another place.

Absolutely, it's a real mess no doubt

pingiun commented 2 years ago

We can do this the same way as blocklisted thaliapay users

JobDoesburg commented 2 years ago

Use a event blacklist, similar to the Thalia Pay user blacklist

se-bastiaan commented 2 years ago
class BlockedUser(models.Model):
    member = models.OneToOneField(Member, on_delete=models.CASCADE,)
    categories = models.ManyToManyField(EventCategory, on_delete=models.SET_NULL,)