New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix/TEC-3232-scheduled-imports #3052
Conversation
As the main block (if) returns a value inmediatly the subsequent else block is non required plus doing that decrease the mental model around thew flow of the calls on those functions. Task: [TEC-3232]
The functions expect a specifc type of value to be returned but on those instances none was returned (meaning null) in order to keep consistency with the expected value the expected return value was used instead. Task: [TEC-3232]
As if a class, in this particular case: $record is being compared against his parents meaning that if $record is instance of the Tribe__Events__Aggregator__Record__Abstract is the same as comparing that the variable has the class as one of his parents.
Use HOUR_IN_SECONDS constant to present a more meaningful value
A job is empty if has a null_process or is no longer fetching if has an error
Save the last status as failure as well if the job was not successfull
If an error is returned dees not schedule a queue work and mark the job as a failure. Task: [PRMTR-162]
As the default operator of the queries is an AND the queries will remove results that either does not have the key or the value is other than 1 instead the desired behavior is posts where the querie either does not exists or the value is not 1 and where the origin is not CSV.
Return that was not required as removes the regular flow on the function
Make sure records as pending are pick via the cron job as well Task: [TEC-3232]
@mitogh this PR is based on |
For now yes we are waiting on a release branch or if are holding the PR, it was based from an old branch as is the one that Loxi is using at the moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look pretty darn good.
@mitogh: I've re-started the tests as it seems a REST API issue not related to the changes. |
Co-Authored-By: theAverageDev (Luca Tumedei) <luca@theaveragedev.com>
Co-Authored-By: theAverageDev (Luca Tumedei) <luca@theaveragedev.com>
Co-Authored-By: theAverageDev (Luca Tumedei) <luca@theaveragedev.com>
Co-Authored-By: theAverageDev (Luca Tumedei) <luca@theaveragedev.com>
…be/the-events-calendar into fix/TEC-3232-scheduled-imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that solved the problem with the tests.
Make sure all pending jobs are selected when processing the EA Cron
Task: TEC-3232