User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Attempt to assign property "last_check_timestamp" on null
JROOT/administrator/components/com_joomlaupdate/src/Model/UpdateModel.php:119
Remove error on NULL if no Joomla Update Site is found in database for the Joomla Update.
This PR just test if the record is null and do nothing more.
TODO : Check why update_site is not present and/or insert it
Happen for me after each manual update of Alpha, Beta, Release Candidate
No update site found for Joomla Update. Playing with configuration to modify the url change nothing.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_joomlaupdate |
It is better for sure... but it is not critical and displaying error is not the solution.
NEVER do a IF without checking the validity of the variable (if you are not 200% sure of its value), it's not so complicated as that. After we can do what you want if the variable is not valid.
In this case adding a message to the user is a minimum and can be done in a second time... ELSE or TRY/CATCH or both, I leave the decisions to those who decide.
It's not a SQL issue but a Queries which is returning NULL => Update site entry not found => what do we want !?
First solution (the badest for me) :
else
{
Factory::getApplication()->enqueueMessage('COM_JOOMLAUPDATE_NODOWNLOAD_NODATABASE_ENTRY_ERROR', 'error');
}
COM_JOOMLAUPDATE_NODOWNLOAD_NODATABASE_ENTRY_ERROR="We can't find the database entry for the download URL"
Second solution : Insert the Update Site URL because in all cases the update checking is broken.
The entry is not rebuild with in the updates sites list.
else
{
$inserted = false;
try
{
// TODO : Write the good insert with Table row or SQL query
// Check the result, not sure it is necessary.
// In this case insert should be ALWAYS true or throw error
$inserted = true;
catch
{
Factory::getApplication()->enqueueMessage($e->getMessage(), 'error');
}
finally
{
if($insert)
{
Factory::getApplication()->enqueueMessage(Text::_('COM_JOOMLAUPDATE_REBUILD_UPDATE_URL'), 'warning');
}
else
{
Factory::getApplication()->enqueueMessage($this->_message = Text::_('COM_JOOMLAUPDATE_FAILED_TO_REBUILD_UPDATE_URL'), 'warning');
}
}
Surely optimisation can be done after that.
Like I have said in the comment :
TODO : Check why update_site is not present and/or insert it.
I suggest to validate and close this PR and open a new one when one solution will be adopted.
Labels |
Added:
?
|
I have make the missing insert if the Joomla Update Sites row is not present in the database !
Just test it and validate it please, it's anoying bug because Joomla update is completly broken ;)
I have make the missing insert if the Joomla Update Sites row is not present in the database !
Just test it and validate it please, it's anoying bug because Joomla update is completly broken ;)
can you try to rebuild the update sites on that site? Do you know why the entry has been removed in the first place? I would like to avoid to fight symtoms other than the real issue.
I have try all you want... rebuild is not working and bug always show the error page after a manual update, nothing is inserted in the database before this PR.
I would like to avoid to fight symtoms other than the real issue.
I really understand and it's the good manner to do, but another good practice :
In ALL case CHECK THE VARIABLE please !!!!!!!!! Don't leave a Attempt to assign property on NULL error.
It's a real function issue.
Rebuild doesn't work if you don't have the entry in the database !
Rebuild is not a bug correction !
An ending user should not have to see this king of error pages.
10 000 reasons why....
I don't know how the entry has disappear from the database the first time,..
PhpMyadmin and delete button do correctly the job :)
A query in an external extension can accidently (or not) also do it ;)
In this case the entry doesn't exists, so we don't care, we can correct it by creating the entry and voilà.
If rebuild doesn't do the job correctly, it's another thing.
Note : the Joomla Update Site can't be deleted by the user like others extensions update sites. That is working.
This pull request has automatically rebased to 4.2-dev.
This pull requests has been automatically converted to the PSR-12 coding standard.
Labels |
Added:
PBF
?
|
This pull request has been automatically rebased to 4.3-dev.
This pull request has been automatically rebased to 4.4-dev.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-12-29 16:25:34 |
Closed_By | ⇒ | shim-sao | |
Labels |
Added:
Updates Requested
bug
Small
PR-4.4-dev
Removed: PBF ? ? |
I have tested this item ✅ successfully on 78e6166
Shouldnt we better catch the issue that the SQL call failed and show a message to the user vs silently skip this here?