I updated the code comments where it is throwing an exception instead of setting an error and returning false.

Is there something else I should change?

bump

needs two successfull tests to get merged in the core.

• IMHO you should not call quoteName($orderingField) more than once per one method. Create a variable and use it in other places.

• You missed one https://github.com/joomla/joomla-cms/pull/12464/files#diff-8254e2c441a41d3fa26f0f83fbbb3043R1477

Both should be fixed now @csthomas

I have tested this item ✅ successfully on fd08d7b

Tested ordering of articles before and after PR, in both case ordering working fine.

What about other files:

libraries/joomla/table/nested.php:1299:  if (property_exists($this, 'ordering'))
libraries/joomla/table/asset.php:168:    if (property_exists($this, 'ordering'))

I'm not sure but IMHO nested.php should be changed too.

Never used those classes. I bet you are right.

I will run through the rest of the libraries and see if there are others.

Only JTable and JTableNested need to have this abstraction layer. The other subclasses are "final" implementations for each table.

I added the abstraction layer to JTableNested.

I feel like it should be on JTableAsset as well @mbabker since it is using a variable for the table name.

A lot of the JTable subclasses reference the class variables in the methods versus hardcoding things like the table key(s) or name. JTableAsset is the class directly mapping to the #__assets table so it shouldn't need direct mapping like the other two classes have. Unfortunately, we neither declare the base classes (JTable or JTableNested) as abstract nor the "final" subclasses as final, so by that argument every column name supported by the abstraction layer should technically be mapped in the same way in all subclasses in case they are subclassed further by other developers. Personal opinion, I don't think it's needed for instances where it's supposed to be a final object, but I'll let someone else make the final judgement call on it.

Gotcha. I end up extending core classes a lot, so its always nice when the abstraction is already there. However, abstracting everything is likely out of the scope of this PR.

That was just terrible. What I get for rushing.

I tested successful
and on review I have not found any ordering column in JTableNested subclasses.
It is not used by joomla at all but because of B/C it probably has to stay.
That OK.

Code style:
I thought that before if () code line there should be a comment or empty line.
Am I wrong?

No idea. I don't see anything in here though however: http://joomla.github.io/coding-standards/?coding-standards/chapters/php.md

I have tested this item ✅ successfully on 32a6436

Joomla works normal.

How does the HumanTestResults test work?

I have tested this item ✅ successfully on 32a6436

RTC

How does the HumanTestResults test work?

@davidjosephhayes you can take a look here: https://docs.joomla.org/Testing_Joomla!_patches#Recording_test_results

It is a feature of ours custom issue tracker arround GitHub :)

With the last 4 commit i have fixed some CS Issue travis did not mention (why?) and updated this branch to the last staging.

@rdeutz @wilsonge please take a final review and merge decision here.

@davidjosephhayes could you please look at the conflicts?

Is it just me or is the conflict page not loading?

For me it is not working too.

@davidjosephhayes this is the conflicting file:
libraries/joomla/table/table.php

How do I do know what to fix? When I click the Resolive Conflicts, nothing loads.

I should have fixed conflicts here. Can I have one tester to confirm this PR still works please

I have tested this item ✅ successfully on fc626b1