Skip to content
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

Merged
merged 24 commits into from Feb 7, 2020
Merged

Conversation

@mitogh
Copy link
Member

mitogh commented Feb 5, 2020

Make sure all pending jobs are selected when processing the EA Cron

Task: TEC-3232

mitogh added 11 commits Feb 5, 2020
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.
@mitogh mitogh self-assigned this Feb 5, 2020
Return that was not required as removes the regular flow on the function
@mitogh mitogh added the needs tests label Feb 5, 2020
Make sure records as pending are pick via the cron job as well

Task: [TEC-3232]
@mitogh mitogh removed the needs tests label Feb 6, 2020
@lucatume

This comment has been minimized.

Copy link
Contributor

lucatume commented Feb 6, 2020

@mitogh this PR is based on master, is this the correct branch?

@mitogh

This comment has been minimized.

Copy link
Member Author

mitogh commented Feb 6, 2020

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.

Copy link
Member

borkweb left a comment

These changes look pretty darn good.

src/Tribe/Aggregator/Cron.php Outdated Show resolved Hide resolved
src/Tribe/Aggregator/Cron.php Outdated Show resolved Hide resolved
src/Tribe/Aggregator/Record/Queue.php Outdated Show resolved Hide resolved
src/Tribe/Aggregator/Record/Queue_Cleaner.php Outdated Show resolved Hide resolved
@lucatume

This comment has been minimized.

Copy link
Contributor

lucatume commented Feb 7, 2020

@mitogh: I've re-started the tests as it seems a REST API issue not related to the changes.
Please make sure tests pass before merging.

@mitogh mitogh dismissed lucatume’s stale review Feb 7, 2020

The base branch was changed.

@mitogh mitogh changed the base branch from master to release/B20.02 Feb 7, 2020
mitogh and others added 4 commits Feb 7, 2020
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>
@mitogh mitogh removed the needs release label Feb 7, 2020
@bordoni
bordoni approved these changes Feb 7, 2020
Copy link
Member

bordoni left a comment

I guess that solved the problem with the tests.

@bordoni bordoni added merge and removed code review labels Feb 7, 2020
@mitogh mitogh merged commit 51043b7 into release/B20.02 Feb 7, 2020
1 of 2 checks passed
1 of 2 checks passed
.github/workflows/phpcs.yml .github/workflows/phpcs.yml
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mitogh mitogh deleted the fix/TEC-3232-scheduled-imports branch Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.