[4.1] Fix generate web cron key #37868
Conversation
| @@ -375,7 +375,7 @@ public function generateWebcronKey(EventInterface $event): void | |||
| [$context, $table] = $event->getArguments(); | |||
|
|
|||
| if ($context !== 'com_config.component' | |||
| || ($table->name ?? '') !== 'COM_SCHEDULER') | |||
| || ($table->name ?? '') !== 'com_scheduler') | |||
There was a problem hiding this comment.
This can also be moved to one line, or?
There was a problem hiding this comment.
You mean change it to, or ?
if ($context !== 'com_config.component' || ($table->name ?? '') !== 'com_scheduler')
|
Strange that this is the fix as the code has always been this way and it obviously worked previously. I wonder what changed (elsewhere) to break this. Whatever it was, if it broke this then we should treat that as a red flag that it could have broken code elsewhere. |
|
@brianteeman I don't have any clue about it because I haven't looked at the code of schedule tasks feature before. From what I see (by just code reading), |
| @@ -374,8 +374,7 @@ public function generateWebcronKey(EventInterface $event): void | |||
| /** @var Extension $table */ | |||
| [$context, $table] = $event->getArguments(); | |||
|
|
|||
| if ($context !== 'com_config.component' | |||
| || ($table->name ?? '') !== 'COM_SCHEDULER') | |||
| if ($context !== 'com_config.component' || $table->name !== 'com_scheduler') | |||
There was a problem hiding this comment.
Can we do a strtolower on this please - covers both cases with minimal overhead. If this used to work - it's safer
There was a problem hiding this comment.
@wilsonge We can but is it really needed? The install and update SQL, from my quick check, is just com_scheduler. Name of all of our components in extensions table is also lower case, so I don't see the need for that. Strange that this feature worked before.
There was a problem hiding this comment.
It might depend on the collation if a string comparison in MySQl is case sensitive or not. See e.g. https://stackoverflow.com/questions/5629111/how-can-i-make-sql-case-sensitive-string-comparison-on-mysql or https://stackoverflow.com/questions/5629111/how-can-i-make-sql-case-sensitive-string-comparison-on-mysql . So for people using the Joomla standard "utf8mb4_unicode_ci" it might have worked, and for people using some collation ending with "_cs" it moght not have worked.
There was a problem hiding this comment.
I'm not sure with my above comment, it's just an idea.
There was a problem hiding this comment.
Here we just compare data which is read from database, not from an SQL query, so I think it is not related.
There was a problem hiding this comment.
Right, silly me. The comparison happens in PHP and not in a query in SQL. Seems I was blind.
|
I have tested this item ✅ successfully on b09ee6b This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37868. |
|
|
|
Hmm. The patch worked for me for both 4.1 and 4.2, but for my local site, it is a new installation, not updating from old version of Joomla. @brianteeman Could you please changed that line of code to the code below, then test it again to see if it works? if ($context !== 'com_config.component' || strtolower($table->name) !== 'com_scheduler') |
|
I have tested this item ✅ successfully on b09ee6b This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37868. |
|
RTC. Thanks all ! This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37868. |
@brianteeman It broke with your PR #37387 |
|
@heelc29 and now we know why - thanks |
|
Thx |
|
confirmed, it works! Thank you! |
Pull Request for Issue #37806.
Summary of Changes
This PR fixes a small typo in the code cause web cron key not being generated when Web Cron enabled
Testing Instructions
Please note that I have zero experience with web cron feature, just try to fix this issue by reading the code quickly.