Removing unused code for ordering in views

[rdeutz]

Executive summary

This patch removes some unused (javascript/PHP) code in the following views

• Banners
• Banners-Client
• Categories
• Articles
• Users
• Menu Items

Backwards compatibility

Full b/c, no problems are expected

Translation impact

none

Testing instructions

• Install a fresh Joomla! with testing data.
• Log into to backend and go to listed views above and check if ordering works when you click on the table headers and when using the select to chose ordering
• Apply Patch
• Go to listed views above and check if ordering works when you click on the table headers and when using the select to chose ordering
• Nothing should have changed

[gunjanpatel]

Tested and works for me but don't know why I didn't find this issue on http://issues.joomla.org/tracker/joomla-cms/

[rdeutz]

@mbabker can you have a look why this isn't synced with issues.j.o, thanks

[mbabker]

I'm resending failed payloads right now. See joomla/jissues#617 for more.

[brianteeman]

I am confused by this PR - if the code is not used then why was it
refactored and how was it actually tested with PR #5041, #5255 and quote a
few other PR

On 6 February 2015 at 13:56, Michael Babker [email protected]
wrote:

I'm resending failed payloads right now. See joomla/jissues#617
joomla/jissues#617 for more.

—
Reply to this email directly or view it on GitHub
#5994 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

[dgrammatiko]

@brianteeman The refactor of all these views was mainly focused on two parts:
introduce formvalidator
use the joomla API for the inline scripts.
The later (which is the case here) was a one-to-one transformation from inline scripts to addScriptDeclaration(), nothing more nothing less

[brianteeman]

My concern is that the code was "tested" - how could it be if the code isnt
used - Its a general comment about our testing procedures

On 6 February 2015 at 14:11, Dimitris Grammatiko [email protected]
wrote:

@brianteeman https://github.com/brianteeman The refactor of all these
views was mainly focused on two parts:
introduce formvalidator
use the joomla API for the inline scripts.
The later (which is the case here) was a one-to-one transformation from
inline scripts to addScriptDeclaration(), nothing more nothing less

—
Reply to this email directly or view it on GitHub
#5994 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

[dgrammatiko]

Tested => worked without error
Reviewed => Code was inspected
The problem wasn’t the testers in this particular event. Also no blame on the commiter or anyone that reviewed that code, because just by reading the changes made you couldn’t find any flaw because the initial code was useless.
My 2c

[brianteeman]

This is the problem "in general" that concerns me.

People are regularly applying patches and saying it works without
confirming beforehand that they have observed the issue and have observed
the changed behaviour. Joomla lives in a web browser - just reading the
code is not enough

On 6 February 2015 at 14:23, Dimitris Grammatiko [email protected]
wrote:

Tested => worked without error
Reviewed => Code was inspected
The problem wasn’t the testers in this particular event. Also no blame on
the commiter or anyone that reviewed that code, because just by reading the
changes made you couldn’t find any flaw because the initial code was
useless.
My 2c

—
Reply to this email directly or view it on GitHub
#5994 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

[rdeutz]

The problem is that after introducing the search tools the old code wasn't removed. The fact that we don't use the search tools everywhere and in the other situations (not used search tools) these code is needed doesn't makes it easier. Without really looking deep into the code it is hard to spot that this is unused.

[brianteeman]

@test I cant see any changes in ordering so I guess this is a successful test

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5994.}

[brianteeman]

As we have two tests I am setting this RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5994.}