llazzaro / django-scheduler

A calendaring app for Django.
BSD 3-Clause "New" or "Revised" License
1.27k stars 391 forks source link

Merge in Abstract bases, and remove SCHEDULER_BASE_CLASSES from docs? #550

Open dwasyl opened 1 year ago

dwasyl commented 1 year ago

Just taking a look at this tool, seems quite useful! However, I need to extend the Event with some extra fields and having the key models as abstract bases would be very useful here, I noticed the PR #488 which does exactly that. Any chance this could get merged in, or what would be required for it? The code looks like a fairly straight forward addition.

At the same time, it's probably a good idea to drop SCHEDULER_BASE_CLASSES from the docs since it's been gone for a few years.

llazzaro commented 1 year ago

Hi! Sounds like a good idea. I will try to get some time to check the PR

h3x4git commented 1 year ago

Love this solution! It will make your Event class a bit fat though, but making it transparent to the schedule package sounds very convenient. I'd rather go for polymorphism if that is possible. I'm developing a solution with polymorphed models now, I may propose a PR when I'm finished with a bug I'm stuck with (not related to polymorphed models)

llazzaro commented 1 year ago

Hi! I don't have lot of time to maintain this project. I like the PR #488 but I'm not sure about it's impact on other users of this project. Feel free to create a PR, but can you share the idea first?

I'm will try to merge that old MR since I like the idea.

Przemek625 commented 1 year ago

@llazzaro I checked this PR. In my opinion this is a great idea and we should get this into the project asap.

h3x4git commented 1 year ago

Hi! I don't have lot of time to maintain this project. I like the PR #488 but I'm not sure about it's impact on other users of this project. Feel free to create a PR, but can you share the idea first?

I'm trying to adapt this package to a non-corporate public calendar project with a lot of users, in my case different kinds of events address an entire community (nation-wide).

I think it's likely that an event may represent very different things nowadays, although any event will share some basic properties and methods of the base class, chances are there will be specific extra fields needed to define what kind of event and model you will be dealing with. So an Abstract base is a good solution, as well as polymorphism. For instance: an event could very likely be a videoconference (maybe a workshop class) that will need a URL field and may have a limited set of participants or some kind of authentication password, while another could be a public conference where participants don't need to enroll but the event will certainly need fields for the specific city and address for the event to function. They will all have different calendaring scenarios then and I would like to make this package a bit more versatile.

Both setting an abstract model base class and polymorphing the base model class of your package would let us decide on app-level how to use your scheduling engine without having to adapt much to a single model class, by instead extending it for our app-specific needs. Separate models with Event metadata tied to an Event instance via a ForeignKey looks a bit of an overkill to me when it comes to time-based queries maintenance (if they're possible at all).

Polymorphing, as opposed to the Abstract model solution, lets you use your scheduling engine for different kinds of events on the same project while keeping the universal model-query API methods in order to perform filters to all the event objects in your project. For instance: I want to look for all the events (no matter what kind of event) that have a field "city" so that I can present a calendar of all the events taking place in a city on a specific day, while I could still offer online events on my website that are not bound geographically, maybe to be combined with the rest of the data in a calendar elsewhere based on other criteria (topic for instance). I will just use the Event base class manager to perform a standard Django filter query on all the downcasted models I created throughout my project. It just works, I've tried. No syntax changes needed.

This may not strictly be an Abstract-base drawback as I could still go through all of the abstracted-based models defined resulting in several more yet more precise lookups (more queries and more code though, very project-specific too, however less data to look up simultaneously).

Polymorphic models are just as straightforward to implement as the Abstract base, by using django-polymorphic.

When filtering the events for a calendar based on extra fields a custom calendar view class is necessary in both forks at hand, from my understanding. Regardless which of the two solutions suggested you will choose, I would still suggest or work on a PR myself including some handy custom calendar class views based on field type for any event model extension scenario. E.g. CalendarByPeriodsByBooelanView, CalendarByPeriodsByIntegerView, CalendarByPeriodsByForeignKeyView etc.

For my project I'm experimenting with polymorphed models and I wrote my own "CalendarByPeriodsByCityView", then one for regions etc. I could transparently query the events of get_context_data by doing calendar.event_set.all()

I'm hoping to be able to apply this calendaring pattern and re-use this view to all the kinds of geographically-bound events I'm going to include in my project. I would imagine this could be a common use case too, just my opinion.