Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dwasyl opened this issue Feb 26, 2023 · 5 comments
Open

Comments

@dwasyl
Copy link

dwasyl commented Feb 26, 2023

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
Copy link
Owner

llazzaro commented Mar 3, 2023

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

@h3x4git
Copy link

h3x4git commented Mar 26, 2023

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
Copy link
Owner

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
Copy link

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

@h3x4git
Copy link

h3x4git commented Apr 14, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants