[#8946] - Delete unnecesary catslug query from mod_related_items
- Fixed in Code Base
- 13 Nov 2016
- Medium
- Build: staging
- # 8946
- Diff
- shur:patch-10
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This PR just removes part of sql request which retrieves catslug because it's not used anywhere.
There is a query part that retrieves catslug, but this catslug parameter is never used in this module and doesn't impact anything because later another catslug is created.
How to test:
Just print_r($item->catslug); after this line and make sure that this parameter is still present even after we remove this part of the sql request.
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
I have tested this item 
unsuccessfully on d04a7ad
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.
@brianteeman I don't see such behavior in my tests.
Moreover this module doesn't have any ordering option at all.
I can only go by the screenshot
On 20 Jan 2016 1:06 pm, "Alex B." notifications@github.com wrote:
@brianteeman https://github.com/brianteeman I don't see such behavior
in my tests.
Moreover this module doesn't have any ordering option at all.—
Reply to this email directly or view it on GitHub
#8946 (comment).
The difference is because there is no ordering specified in the query, which means the results are sort of random. Changing the query may result in different ordering because MySQL does a different intern processing. So that is expected behavior.
However I don't think it's good to remove the data from the query as it may be used in template overrides. Maybe it's better to just a deprecate note so it can be removed with 4.0 safely without a potential to break something.
It was only becoming obsolete with #5276, before that it was (wrongly) used to generate the URL.
...which means the results are sort of random.
Exactly! That's why only screenshots are not informative. Thanks for the detailed explanation.
However I don't think it's good to remove the data from the query as it may be used in template overrides. Maybe it's better to just a deprecate note so it can be removed with 4.0 safely without a potential to break something.
I was the one who insisted on removing catslug a while ago ( #4363 ). Yes, you are right, it can be used in template overrides. But somehow I confused the version, I thought Joomla 3.5 is the major version that is going to be without catslug.
No problem, I'll adjust this PR and add deprecated note.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.
One more crazy thing in this module. There is a query part that retrieves catslug, but this catslug parameter is never used in this module and doesn't impact anything because later another catslug is created.
That's why I deleted this part of the query.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.
| Title |
|
||||||
| Title |
|
||||||
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.
How to test added to description.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.
| Category | ⇒ | Router / SEF |
Joomla 3.5 is soon to come, but this simple PR hasn't got any response for almost a month.
I'd like to know what is the final decision - include or not in Joomla 3.5?
Or maybe something else is needed from me?
As it has no tests, it will not go into 3.5. Simple as that.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.
I have tested this item
Deprecated message is added and output is unchanged.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8946.
I have tested this item
I applied the patch and don't see any difference in the front-end as it shouldn't.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8946.
Thanks for testing.
dgt41
- | Status | Pending | ⇒ | Ready to Commit |
| Category | Router / SEF | ⇒ | Modules Front End Router / SEF |
roland-d
- | Milestone |
Added: |
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-11-13 05:15:51 |
| Closed_By | ⇒ | roland-d |
Before patch the list of related articles is sorted in alphabetical order
After the patch the list of related articles is not
Before
After
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8946.