Opened 18 months ago
Last modified 17 months ago
#1526 new enhancement
Since upgrade (from 0.9? to 0.9?+1, some time ago) no correct display of upcoming events any more
Reported by: | hoffmann | Owned by: | |
---|---|---|---|
Priority: | high | Milestone: | v2.0 |
Component: | Database | Version: | 1.1 |
Keywords: | Cc: |
Description
We are on 1.1.2 now, but apparently no reindexing or cleanup action has corrected the error.
The "Upcoming Events" list on the home page is incomplete.
We have setup two categories to be observed objects in InDiCo?. One is the base/root category (0), the other one (6) is a category that does not exist any more. It cannot be deleted though.
This problem with deleting an non-existing category may or may not be related to the main problem described below.
Usually none of the upcoming events is displayed in the list at the right border of on the homepage. Sometimes one or two show up, without any obvious systematic reason. Today (April 23rd), I see an upcoming event starting on June 4th. There should be dozens (well, limited to 20 or so) according to the calendar, "today's events" or "this week's events".
Change History (11)
comment:1 Changed 18 months ago by hoffmann
comment:2 Changed 18 months ago by hoffmann
from indico.modules.base import ModuleHolder uem = ModuleHolder()._getIdx()['upcoming_events'] for obj in uem.getObjectList(): wrappedObj = obj.getObject() if isinstance(wrappedObj, Category): print wrappedObj; events = categDateIdx.getObjectsIn(wrappedObj.getId(), date, date + obj.getAdvertisingDelta()) for conf in events: print conf.getStartDate().isoformat() uem.processEvent(date, conf, obj, objDict)
yields
<Category 1@0xafeaeec> <Category 0@0xaf0792c> 2014-05-06T12:00:00+00:00 2014-05-09T08:00:00+00:00 2014-05-13T11:00:00+00:00 2014-06-10T10:00:00+00:00 2014-05-19T14:00:00+00:00 2014-05-16T08:00:00+00:00 2014-05-23T08:00:00+00:00 2014-05-30T08:00:00+00:00 2014-06-04T06:00:00+00:00 2014-06-06T08:00:00+00:00 2014-04-24T13:00:00+00:00 2014-04-24T13:30:00+00:00 2014-04-24T13:00:00+00:00 2014-04-25T08:00:00+00:00 2014-04-28T11:00:00+00:00 2014-05-02T08:00:00+00:00
but
objDict.keys()
is [1.0].
Thus the problem seems to be related to UpcomingEventsModule.processEvent(). It turns out that this event is probably the only one that is public on the site.
The question is now, if the Upcoming Events display is supposed to show a list of events depending on the actual permissions of the user. Of course I confirmed that the display is the same, when I am logged off or logged in :)
Up to you now?
comment:3 Changed 18 months ago by hoffmann
Actually, this is the line that looks offending to me:
https://github.com/indico/indico/blob/master/indico/modules/upcoming.py#L137
if (not eventObj.hasAnyProtection() ...
just eliminates any protected event, whoever is looking at the list. That does not sound right.
comment:4 Changed 18 months ago by hoffmann
Better use eventObj.isAllowedToAccess() instead?
But that is pure speculation now. Experts will know better.
comment:5 Changed 18 months ago by pferreir
Hi again,
Yes, that is indeed the reason. This is intentional, since this is supposed to be a "global" list of stuff that is going to happen site-wide, so the targe audience is quite broad. We also wanted to make it cacheable since it's a list that is not that "cheap" to generate.
If you want to have a "personal" view on events then "My Profile" is the right answer.
So, in short, "upcoming events" is a site-wide feature that we will hopefully get rid of once we move the dashboard to the main page as we intend to do.
Does that sound fair enough? :)
comment:6 Changed 17 months ago by hoffmann
Hm. Is it something that will come with v1.2?
If the list generation is expensive, then maybe it can be cached as a whole, and displayed depending on permissions. At least, I think, the title should be "upcoming public events" in the meantime.
(Can be patched in egg/MaKaC/webinterface/tpls/CategoryDisplay.tpl for reference.)
The critical question for caching is when the cache will be invalidated. Presently any new meeting with or modification to public should invalidate the cache, and more important: any modification from public to something else should invalidate the cache immediately.
Thus, you are only saving the invalidation of the cache for meetings that go from not public to another not public state.
Well, I see the point. But I have to answer the question: "When will it be improved?"
comment:7 Changed 17 months ago by hoffmann
Afterthought: Many meetings/categories inherit read permissions (write permissions being irrelevant). Is it really that expensive/different?
comment:8 follow-up: ↓ 9 Changed 17 months ago by pferreir
"Upcoming Public Events" sounds slightly verbose to me. Maybe we should find something slightly different that conveys a similar meaning.
The problem is not protection "transitions". It's the fact that you would have to have a cache per user, which would kind of defeat the purpose since people would anyway only go once or twice to the home page.
We do now have better caching strategies that could fix this, but the DB change will basically allow us to query things in a much better way.
So, I would schedule this for 2.0 and see if we can come up with a better strategy when we port it to SQLAlchemy.
comment:9 in reply to: ↑ 8 Changed 17 months ago by hoffmann
Replying to pferreir:
"Upcoming Public Events" sounds slightly verbose to me. Maybe we should find something slightly different that conveys a similar meaning.
I thought so. But with my browser fonts it still fits the headline and is not even close to the end.
The problem is not protection "transitions". It's the fact that you would have to have a cache per user,
unless you cache with protection information.
We do now have better caching strategies that could fix this, but the DB change will basically allow us to query things in a much better way.
So, I would schedule this for 2.0 and see if we can come up with a better strategy when we port it to SQLAlchemy.
You are in the best place to judge this. My input came from the feedback of "my" community, where little events are shown in the list, as 99% are protected at least in some way, and only test events usually come up public. So the option to not show this block (like it is possible for "News") would be a good alternative as well.
Anyway, if I need to patch it, I know now that it would not be double work with something going on in mainstream. And for the ideal implementation, it is up to you to decide.
Thanks!
comment:10 Changed 17 months ago by pferreir
- Milestone changed from v1.2 to v2.0
From the code it looks as if the section is fully hidden if there are no upcoming events:
https://github.com/indico/indico/blob/v1.2/indico/MaKaC/webinterface/tpls/CategoryDisplay.tpl#L79
I will schedule this for 2.0, at which point we can better discuss what to do with the homepage.
comment:11 Changed 17 months ago by pferreir
- Type changed from defect to enhancement
Display code is source:master/indico/modules/upcoming.py#L172 (Thanks, Pedro.)
(Not sure, if I manage to make the link correctly here, so I add the full HTTP link: https://github.com/indico/indico/blob/master/indico/modules/upcoming.py#L172)