Updates Requested bug Small PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar shim-sao
shim-sao
2 Feb 2022

Pull Request for Issue # .
Attempt to assign property "last_check_timestamp" on null
JROOT/administrator/components/com_joomlaupdate/src/Model/UpdateModel.php:119

Summary of Changes

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

Testing Instructions

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.

Actual result BEFORE applying this Pull Request

2022-02-02 15 25 27 j4test fedora35 05839c4e61b2

Expected result AFTER applying this Pull Request

2022-02-02 15 28 35 j4test fedora35 a639318de52c

Documentation Changes Required

avatar shim-sao shim-sao - change - 2 Feb 2022
Status New Pending
avatar shim-sao shim-sao - open - 2 Feb 2022
avatar joomla-cms-bot joomla-cms-bot - change - 2 Feb 2022
Category Administration com_joomlaupdate
avatar shim-sao shim-sao - change - 2 Feb 2022
The description was changed
avatar shim-sao shim-sao - edited - 2 Feb 2022
avatar zero-24
zero-24 - comment - 6 Feb 2022

Shouldnt we better catch the issue that the SQL call failed and show a message to the user vs silently skip this here?

avatar shim-sao
shim-sao - comment - 6 Feb 2022

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.

avatar shim-sao shim-sao - change - 8 Feb 2022
Labels Added: ?
avatar shim-sao
shim-sao - comment - 15 Feb 2022

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 ;)

avatar zero-24
zero-24 - comment - 15 Feb 2022

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.

avatar shim-sao
shim-sao - comment - 15 Feb 2022

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.

avatar HLeithner
HLeithner - comment - 27 Jun 2022

This pull request has automatically rebased to 4.2-dev.

avatar joomla-bot
joomla-bot - comment - 27 Jun 2022

This pull requests has been automatically converted to the PSR-12 coding standard.

avatar HLeithner HLeithner - change - 27 Jun 2022
Labels Added: PBF ?
avatar chmst
chmst - comment - 22 Oct 2022

@shim-sao your code does nor repect Joomla coding conventions,

  • We don't use transactions,
  • We use single quotes
    Please compare similiar code in Joomla.
avatar HLeithner
HLeithner - comment - 2 May 2023

This pull request has been automatically rebased to 4.3-dev.

avatar HLeithner
HLeithner - comment - 30 Sep 2023

This pull request has been automatically rebased to 4.4-dev.

avatar shim-sao shim-sao - change - 29 Dec 2023
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 ? ?
avatar shim-sao shim-sao - close - 29 Dec 2023
avatar rmittl rmittl - test_item - 28 Jan 2024 - Tested successfully
avatar rmittl
rmittl - comment - 28 Jan 2024

I have tested this item ✅ successfully on 78e6166


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36921.

Add a Comment

Login with GitHub to post a comment