Updates to fix updater system #676

Merged
merged 1 commit into from Jan 2, 2012

7 participants

@pasamio

This pull request:

  • fixes CLIENT undefined index problem
  • remove an undefined index warning
  • removes the INFOURL addition until CMS has accepted the change
  • and also removes a commented debugging statement that somehow made it in.

"CLIENT" undefined index warning

In pull request 667 the concept of changing "CLIENT" to "CLIENT_ID" is introduced to remove an undefined index "CLIENT" problem. This is actually a side effect of CMS tracker 24338 [2,3] where the appropriate entry in the update_cols was mercilessly renamed from "CLIENT" To "CLIENT_ID" leading us to the predicament we presently find ourselves in.

I suspect that the initial change is itself a reaction to CMS Issue # 24305 [4,5] which goes to show the law of unintended consequences is truly afoot.

Undefined index warning

An extra guard is added around the last_check_timestamp check in the update cache code that was added. There are situations when this value may not exist and could trigger a warning.

Removal of INFOURL addition

The removal of the "INFOURL" tag is two fold. First it's in the wrong place, it should live in the _update_cols of the updateadapter class. Secondly until the CMS accepts the DB schema change[6], it breaks loading extensions which made the changes in this pull request rather hard to test using the current Joomla! 2.5 beta. Given this change was merged into the platform in July and it is now December, it is like that the CMS is presently broken but people just don't realise (the collection type is unaffected).

Refs:
[1] #667
[2] 2881906#libraries/joomla/updater/updateadapter.php
[3] http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=24338
[4] 120f114
[5] http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=24305
[6] http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=26635

@pasamio pasamio Updates to fix updater system: remove an undefined index warning, rem…
…ove INFOURL addition until CMS has accepted the change and fix CLIENT undefined index problem
a35a765
@joomla-jenkins

Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11142 assertions.
Checkstyle analysis reported 199 warnings and 0 errors.

@elinw

Given that it was only done for 11.3 which was only merged maybe a month ago, it's not suprising that the cms hasn't necessarily caught up perhaps.Date of commit to the platform is not really relevant if the platform hasn't been merged. :)
How about if we just get the CMS to commit the change?
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=26635

@mbabker
Joomla! member

It's been a while since I did the INFOURL change, but if I remember correctly, I originally started out by putting it in the _updatecols property, but the column wouldn't get handled correctly. I can always test again to see if that holds true or if it was something at the time that I was missing, but that's why it is where it's at now.

@infograf768
Joomla! member

Please indeed test, Michael and change cms patch to include a new sql update.

@gnomeontherun

This appears to make sense, and the patch for this fixes the update collection notifier in the CMS, since it now returns an empty array instead of the error messages.

@pasamio

@elinw the point is that 2.5 beta is broken and the change isn't the best; if the CMS tests and applies that change then we don't have an issue however at the moment the CMS has a bug

@mbabker not sure why it wouldn't be working if you added it there. I can't see code wise why it would make a difference as the "OR" should be the same as adding it to the array logically speaking.

@mbabker
Joomla! member

Well, it's working now with INFOURL in the _updatecols property. I'm regenerating the CMS patch right now with that change, so I suppose it can be added here as well.

@elinw

Ok so we are making progress?

@mbabker
Joomla! member

CMS Pull Request @ joomla/joomla-cms#65

@chdemko chdemko merged commit a48003a into joomla:staging Jan 2, 2012
@chdemko

Fix #667

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment