Correct fix for setting expire dates on events and eventually expiring them.#1964
Open
johnwebman wants to merge 9 commits intowpeventmanager:master_backupfrom
Open
Correct fix for setting expire dates on events and eventually expiring them.#1964johnwebman wants to merge 9 commits intowpeventmanager:master_backupfrom
johnwebman wants to merge 9 commits intowpeventmanager:master_backupfrom
Conversation
Removed temporary expiry date fix
Removed temporary expiry date fix
Fixed get_event_expiry_date function
Fixed non-working hooks Fixed set_event_expiry_date function Updated check_for_expired_events function
Contributor
Author
|
@ritakikani This pull request looks a bit more complicated than it really is. Wherever possible I've tried to simplify the code to avoid potential complications in execution. Once you've taken a look I think you'll agree it's a both useful and necessary update. |
Minified version of frontend.css
Fixes lack of gap between multiple event types.
Contributor
Author
|
@ritakikani accidentally got two CSS files involved by mistake so have removed them from this pull request. I hope. :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Getting the expiry date to autocomplete
The original concept for expiring events was perfectly sound. It just wasn't implemented very well. Under Event Manager > Settings > Event Submission is a setting called 'Listing Expire'.
The default looks like this:
or you can select to expire after a certain number of days.
This should have been the end of it, but unfortunately, the code that is supposed to act on this doesn't work. It doesn't work because the code that's supposed to work out the expiry date has bugs, and because that code is being called by an ineffective trigger.
Unless an event expiry date is actually set on an event it will never expire. That's why some code was added to ensure that the expiry date on an event was always completed after publishing the event, unless of course the above 'Listing Duration' is blank. On saving or publishing an event the code should check to see if the expiry date is empty, and if it is, it uses the 'Listing Expire' setting to work out what the expiry day should be.
First of all the abomination of a so called 'fix' in Version 3.1.51 of Event Manager needs to be removed. I can't understand this obsession with some of your developers to make the expiry date the same as the end date, regardless of the users wishes.
Having removed the offending so called 'fix' we can work on the real issues within the original code. There are two parts to this. First the function that attempts to generate an expiry date if none exists on the post. This can be found in the wp-event-manager-functions.php file in the root of the plugin. The function, get_event_expiry_date(), as it stands just doesn't work as it should.
If 'Listing Duration' in settings is blank, then this function returns nothing and so the expiry date remains blank. The function that calls the above function can be found in the wp-event-manager-post-types.php file, which is in the includes folder of the plugin. This function is called set_event_expiry_date(). The existing function, is as usual, over complicated and doesn't always give the desired results.
Now we come to the interesting part. The bit that actually calls the above set_event_expiry_date() function. This can be found further up the same file in a set of triggers, as follows:
Unfortunately none of these will ever work. Whether they ever worked is a mute point, but they certainly don't now. These are called 'transition hooks', and as the name suggests, they capture the post from one state to another. An undocumented feature of these particular hooks is that you cannot make changes to the post being published, at this point. You can send an email or change anything else, but not the post that's being transitioned to the publish state. What happens is this: no matter what you change in the post, when the publish resumes, it picks up from where it left off and overwrites any changes you made with what Wordpress has in cache for that post. So it resets the expiry date to whatever it was before, which of course would usually be nothing. The correct hook to use is the {status}_{post_type} one. Delete the 5 lines above and replace with:
add_action('publish_event_listing', array($this, 'set_event_expiry_date'));Because this hook only targets event listings, there's no need to check the post type in the set_event_expiry_date() function.
Job done. This trigger works on publish and update. So if you create an event and complete the expiry date field, nothing changes. If you leave the field blank it will be completed automatically by my code changes. That is unless you have the 'Listing Duration' blank in your settings. This is how it was always intended to work. If you don't have the 'Listing Duration' blank in your settings and you want individual events to never expire, you simply need to set an expiry date on that event that is well into the future. I use '31/12/9998'.
The last update is to the check_for_expired_events() function in the wp-event-manager-post-types.php file. The expiry date has no time associated with it, so when you check an event to see if it needs to be expired, you add the time of 23:59:30. So if an event finishes at say 11am, it still shows on the calendar and events page as an active event. I don't think this is desirable. So instead of adding 23:59:30 to the expiry date, I've added the end time. I've made a simple change to the loop that checks events to be expired in the check_for_expired_events() function. I believe this is what people might really expect when they select events to expire on the end date. Also it makes more sense of the cron job that checks for expired events every hour. Without this last update there'd be no need to run the cron job more than once a day.
To Test This Update/Fix
Create an event with an empty expiry date and publish, or edit an existing event and clear the existing expiry date, then update. In both cases the expiry date will be set according to the user settings. If the end time is set for 15 minutes or 30 minutes after the time the event is published/updated, the next time the cron job runs the event will be expired, which makes it easier to test the changes to check_for_expired_events().