What is the purpose of the event_repeat_calendar_map table? I don't quite understand the point of it. It seems to me that the whole module could be simplified down to one table, event_repeat_node, as follows:

CREATE TABLE event_repeat_node (
  eid int(10) unsigned NOT NULL default '0',
  nid int(10) unsigned NOT NULL default '0',
  expr text not null,
  origin_nid int(10) unsigned NOT NULL default '0',
  PRIMARY KEY (rid),
  INDEX idx_nid ON (nid),
  INDEX idx_origin_nid ON (origin_nid)
);

This table stores the EXDATE, EXRULE, RDATE, and RRULE expressions for the iCalendar recurrance properties of a given node. Also, using the origin_nid column, it stores the nid of the node which contained the original expression. This allows us to keep nodes that are part of a recurrance together.

So, on expression creation, we form and parse the expression, creating new nodes as specified. Each node also gets an entry in the above table, with the origin_nid set to the nid of the initial node.

On expression edit, we form and parse the new expression. We then go through the process of creating new nodes and removing old ones. However, if a node created by the old expression would still be created by the new expression, then we leave the old node intact. This way, we can preserve any custom content for those nodes.

On expression delete, we delete all nodes with the same expression and origin, except for the current node (because presumably you'd want to keep the node you are editing, but ditch the recurrance expression).

On node edit, we ask if the user wants to edit this node or all nodes created with the same orign_nid. Editing all nodes will over-write all the other nodes.

On node delete, we check to see if the node is part of a recurrance. If so, we ask the user if they want to delete this occuarnce or all occurrances. On all Occurances, we delete them all. On just this one occurance, we append an EXDATE expression to all nodes that share the same origin. Then, we call the expression editing function.

The extent of the nodes created should be limited. A sensible default would be one year or 50 occurrances, whichever is greater. These would be configurable values. A cron hook could be used to add new occurances.

Thoughts?

Comments

hunmonk’s picture

hunmonk commented
Status: Active » Closed (works as designed)

you're missing a critical step, and that is the processing of the rules to arrive at the specified dates for creation of repeating nodes. it's wasteful to have this kind of processing happening for every repeating sequence--on a large site with shared resources and many repeat sequences this could cause problems. the calendar map enables the proper repeat dates to be pulled for each repeat sequence with one query (one potential optimization i may add is indexing to the calendar_map table, which will also speed up these queries). since the data that the table stores is practically static (only once per day does any processing happen which changes it), this is a much better use of resources.

also, i don't see why you'd want to push the event_repeat and event_repeat_nodes into one table, either--at the moment the data is normalized, and nicely divided between the metadata for the repeat sequence, and the associated node data. event_repeat_nodes is also all integer values, and iirc, that means it can be searched more quickly.

i'm not seeing any compelling reason to change the current setup.

hotgazpacho’s picture

hotgazpacho commented
Status: Closed (works as designed) » Active

I'm sorry, how, exactly, is it wastefull to process the repeat expressions on node create, edit, delete, and cron runs? Isn't this exactly what your module does? There is no rule processing taking place on node view, which is the majority of what most normal users will be doing, anyway.

Could the DB schema be cleaned up a bit? Sure.

I don't see how you can say my idea is inefficient when I haven't written any code for you to compare to. Let me write some code, then we'll do an actual comparison.

As for the calendar_map allowing for the proper repeat dates to be pulled for each request, that is not entirely true. If I create an event that starts in the past, then edit one of the events in the series, then all the events in the series have their dates remapped incorrectly. This disproves your statement. My proposal seeks to address this, because the repeat statement SHOULD be reevaluated when a node that is part of a sequence is edited or removed.

hunmonk’s picture

hunmonk commented

I'm sorry, how, exactly, is it wastefull to process the repeat expressions on node create, edit, delete, and cron runs? Isn't this exactly what your module does?

no, in fact it does not do processing--not much anyways. the dates to render are pulled via a single query for each repeat sequence. i've examined recurrance code in other calendaring systems, and the processing is much more extensive with a dynamic approach than with the more static map approach i've implemented.

As for the calendar_map allowing for the proper repeat dates to be pulled for each request, that is not entirely true. If I create an event that starts in the past, then edit one of the events in the series, then all the events in the series have their dates remapped incorrectly. This disproves your statement.

it does not. if you'll examine the README file, you'll see that what you describe is listed as one of the two remaining bugs, and this can certainly be addressed within the current implementation. i would very much welcome a patch for this.

I don't see how you can say my idea is inefficient when I haven't written any code for you to compare to. Let me write some code, then we'll do an actual comparison.

i would certainly look at any code that you submitted and give it consideration against the current method. but to be clear, it appears that your starting premise is in error, and i'm doubting that it will prove to be more efficient. i'd much rather have help in areas of the module that could obviously use improvement--like the remaining bugs and the horrendous user interface... :)

hunmonk’s picture

hunmonk commented
Status: Active » Closed (works as designed)

marking as by design for now, as i don't see anything coming of this effort. if any more discussion needs to happen, feel free to reopen.