jonge-democraten / website

JD website
https://jongedemocraten.nl
MIT License
6 stars 2 forks source link

Occurrence should be Displayable, not Event #85

Closed Pi2048 closed 9 years ago

Pi2048 commented 9 years ago

When trying to figure out the sitemap.xml functionality (part of issue #47), I found something odd in the fullcalendar code. In this issue, I propose changing the Event and Occurrence models. Now, Event is (a subclass of) Displayable and Occurrence is not. I think that should be the other way around.

Mezzanine/Django contains a default way of generating sitemap.xml files. The code is at mezzanine/core/sitemaps.py. It explains what content is included: everything that is subclassed from Displayable. This seems like a natural way to approach this.

Displayable is the superclass to everything that, by and large, has a separate URL on the website (http://mezzanine.jupo.org/docs/content-architecture.html#displayable-vs-page). Therefore, I think Occurrence should be Displayable. This would ensure it is also included in sitemap.xml files.

Events do not have a separate existence on the site (I think?). They are only accessed through their Occurrences. They are mostly containers for common features of the Occurrences. I do not think they need to be subclassed from Displayable. If they are subclassed from Displayable, it should be done in such a way that sitemap.xml files can still be generated automatically. This includes but is not limited to defining a get_absolute_url() function for Events. What the URL of an Event should be, I have no idea (that is kind of the point of non-Displayable content). Perhaps we could have it use the URL of its first occurrence?

In conclusion, I propose to:

I have tried to make these changes myself. However, I was unsuccessful. @sh4wn, I hope you can reflect on my request and hopefully process it yourself.

Pi2048 commented 9 years ago

Since Displayable is the Mezzanine class that arranges the published/draft status of items, I think fixing this would automatically resolve the issue that Occurrences and Events do behave well when in draft.

Pi2048 commented 9 years ago

What I found out so far:

This is actually more complicated than it sounds. Displayable contains several properties that we would want to arrange at the Event level rather than the Occurrence level (e.g. draft status).

I propose the following approach:

However, I can't get this to work because I don't understand enough about the code. @sh4wn, I really think this one is up to you.

Pi2048 commented 9 years ago

I added a get_absolute_url method to Event. Now, sitemap.xml works (but it obviously does not yet include Occurrences, as these are not Displayable yet.

bartromgens commented 9 years ago

I changed Occurrence to be Displayable in https://github.com/jonge-democraten/mezzanine-fullcalendar/commit/ba41de2f8b80e26e6142b03ef805aa9ae2b33ba1

The admin is the same. The status, publish_date, expire_date are taken from the Event, and updated when an event is saved.

Also added some new events demo data.

Requires an update of the dependencies, $ pip install -r requirements.txt, and reloading the demo_data, $ ./recreate_demodata_database.sh.