[EventCalendar] Small post saving buglet/ other notes

[Alex Tingle]

Hi David,

On Thu, 24 Aug 2006 16:49:50 +0100
David Nutter wrote:

> Hello,
> 
> While hacking around with EventCalendar3 I stumbled upon a bug which
> in certain circumstances leads to duplicate schedule entries being
> saved. 
> 
> Steps to replicate:
> 
> 1) Create a new post (not in the events category) and save it
> 2) Edit the post, put it into the events category and add a single schedule
> entry
> 3) Save it. 
> 4) Post should now have two identical schedule entries in
> wp_ec3_schedule table
> 
> The reason appears to be these two lines at the end of admin.php:
> 
> add_action('save_post',         array(&$ec3_admin,'save_post')); 
> add_action('edit_post',         array(&$ec3_admin,'save_post')); 
> 
> In the case of a pre-existing post, _both_ these actions are executed,
> resulting in two calls to save_post. This isn't a problem when
> UPDATEing an existing event schedule entry, but INSERTing a new one
> occurs twice, leading to duplicate schedule entries. Creating a
> brand-new post doesn't call edit_post actions, so the solution is
> easy, just comment out the second line. 

This doesn't work for me. Without the second line, I can't change the
schedule date of a pre-existing post.

Perhaps we could count the number of times save_post() is called, and
ignore all after the first?


> I've added a few things to my local copy of event calendar (alluded to
> in the comments thread here
> http://blog.firetree.net/2006/07/21/eventcalendar-31-beta/#comment-5505),
> firstly moving the code to update post dates out of the upgrade()
> function as running this causes date-based permalinks to break. 

Yes. I'm not sure what to do about this one. I agree with the idea, but
I don't like having a new button on the options screen forever.

Perhaps we could delay the upgrade, and have a special 'pre-upgrade'
options screen.


> Secondly, adding a config option to turn the content filter which adds
> the schedule to the post body on and off. This is for users who want
> more flexibility in designing their templates. 

As I said elsewhere: I disagree. I want to keep the options as simple as
possible.

If you are comfortable with editing your template to such a degree, then
you should be comfortable with calling remove_filter() in your template.
We need to add it to the documentation as a hint.


> Finally, I've updated the schedule entry dialog box to be more "user
> friendly".

Yes, brilliant!

I've modified your code extensively, but don't take umbrage. Here's a
list of the main things I did:

1. Coding standards compliance.
   Max 80 cols. No tabs. Braces on lines of their own.

2. Cut out repetitive prepare_XYZ() code, and replaced it with
   a generic function driven by array data.

3. Reorganised save_post() to make it terser, and more similar to the
   original. Broke some of it out into a subroutine.

4. Improved the implementation of implode_assoc() and retrieve_date()
   (and get_month_days() in the JavaScript.)

5. Corrected the JavaScript to handle events properly, so that it works
   in IE.

6. Corrected the JavasScript so it now updates the DAYS field when YEAR
   is changed. (Previously it would allow 2006/02/29).


> Currently I'm tidying these up and producing separate patches for each
> and a further patch containing my local bugfixes. These should be
> ready tomorrow. 

I'm very impressed with your patches. Thanks for putting so much effort
into them.

regards,

-Alex

-- 
:: Let me solve your problems: http://www.firetree.net/consulting/
:: alex.tingle AT firetree.net  +44-7901-552763