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

BUG: Protect against URLsToProcess being falsy #147

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

textagroup
Copy link

@textagroup textagroup commented Oct 5, 2022

If URLsToProcess is null then a error will be thrown when trying to create a Delete cache job because array_keys expects an array.

[Emergency] Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given
POST /admin/queuedjobs/Symbiote-QueuedJobs-DataObjects-QueuedJobDescriptor/EditForm
Line 107 in /var/www/stats/vendor/silverstripe/staticpublishqueue/src/Job.ph

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also include a test where a job is processed with no URLs to process.

src/Job.php Outdated Show resolved Hide resolved
@GuySartorelli
Copy link
Member

The only change I've made is resolving a linting issue

@GuySartorelli
Copy link
Member

@chrispenny You've been doing a lot of looking into this module recently and I'm not very familiar with it. This PR looks okay but can you please give it a quick glance and let me know if there's anything obviously problematic about this change?

@chrispenny
Copy link
Contributor

Sorry team, I've been away on annual leave.

The title of the PR makes it sound like we're going to make sure URLsToProcess is cast to an array, but instead it seems we're just checking that it is an array before attempting to loop over it. Either way sounds fine to me.

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me then. Thanks

@GuySartorelli GuySartorelli merged commit 67d6dcd into silverstripe:5 Sep 21, 2023
9 checks passed
@GuySartorelli GuySartorelli changed the title BUG: Typecast URLsToProcess to an array BUG: Protect against URLsToProcess being falsy Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants