User tests: Successful: Unsuccessful:
There are several cases of table classes which internally use instances of themselves or other table classes but get these instances without specifying a dbo. In normal Joomla operation, this will never be a problem but, when you try to do something funny, like manipulate tables on some other db, you may have problems.
FOFTable may have the same issue but I don't understand the purpose of that class well enough to touch it right now.
Labels |
Added:
?
|
By the way, do I need to do something with the new tracker now? I'm not so familiar with the new system.
No special actions are needed. issues.joomla.org automatically syncs with GitHub.
Category | ⇒ | Libraries |
Testing this change is basically impossible without a very special setup involving at least two databases and some custom code. I would hope that the nature of these five, identical, single-line changes would be simple and obvious enough to skip that but I can explain what kind of ill effects you can expect.
As an example, let's say you are using a JTableContent
to add or update a row on a #__content
table in an external database (meaning, not the db you get when calling JFactory::getDBO()
. Why would you do this? Who cares, that's not the point.). This table will do a check to make sure you are not saving two items with the same alias in the same category but it does this using another instance of JTableContent
. If you are working with an external database, this other instance should also connect to the same database. Currently it doesn't, it connects to the local one. So the check that it performs is totally meaningless. It might prevent you from saving for no reason at all, it might allow you to save when it really shouldn't.
The case for other tables might be slightly different but the general idea is the same. If the table class needs to do some check using another instance of a JTable class, it needs to make sure it is configured for the same db.
Seems ok for me. Not sure if we need tests here, it is only a usage of the existing API and it makes sense to use the same db. I would say we can merge it.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4318.
Just test that existing functionality still works.
How about FOFTable
? I didn't touch it in this PR but what is it and how is it used? It might need some similar fix.
You would have to submit that PR to the FOF repo as it's a 3rd party library.
But does Joomla use it in any way? Or is there some plan to use it in the future?
Joomla does only use FOF for the postinstall messages and the TFA.
But 3rd party extensions may use FOF also.
Looked over the code and can only give it a thumbs up Looks good and will make testing easier.
Regarding FOF: I guess the longterm plan is to remove it from Joomla again, right? Especially since FOF does not get any support anymore and rather F0F has a community that supports it. Or did I misunderstand that?
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-12-27 15:56:14 |
Merging on review.
Milestone |
Added: |
By the way, do I need to do something with the new tracker now? I'm not so familiar with the new system.