? ? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
24 Jan 2018

Pull Request to improve #19397

Summary of Changes

  1. Remove redundant view from SEF link generated by mod_articles_archive.
  • before PR index.php/en/archived-articles/2013/10?view=archive
  • after PR index.php/en/archived-articles/2013/10
  1. Delete redundant segments /0/0 from second and next pages, when no year, nor month have been selected.
  • before PR index.php/en/archived-articles/0/0?view=archive&start=5
  • after PR index.php/en/archived-articles?start=5
  1. Allow to use short link to display the archive view for selected year.
  • before PR index.php/en/archived-articles/2011/0?view=archive - works
  • before PR index.php/en/archived-articles/2011 - error 404 or display article with id 2011
  • after PR index.php/en/archived-articles/2011 - works

Testing Instructions

  1. Install the latest staging joomla with sample data "Test English (GB) Sample Data"
  2. Archive at least 5 articles. In total should be more than 5.
  3. Go to the archive view index.php/archived-articles
  4. Change Limit field from All to 5.
  5. Check above examples from summary.

Expected result

Joomla generate and parse nicer links, old links still works.

Actual result

Joomla adds redundant parameters and segments to SEF link.

Documentation Changes Required

No

@Quy, @alikon please take a look

avatar csthomas csthomas - open - 24 Jan 2018
avatar csthomas csthomas - change - 24 Jan 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Jan 2018
Category Front End com_content
avatar Quy
Quy - comment - 24 Jan 2018

Getting these;
Undefined index: view in \components\com_content\helpers\legacyrouter.php on line 99

avatar csthomas csthomas - change - 24 Jan 2018
Labels Added: ?
avatar csthomas
csthomas - comment - 24 Jan 2018

Fixed.

avatar Quy
Quy - comment - 24 Jan 2018

Please confirm that this is the expected behavior with different URLs to the same pages.

Link in the Archived Articles module.
http://localhost/joomla-cms-staging/en/archived-articles/2011/1
pagination: http://localhost/joomla-cms-staging/en/archived-articles/2011/1?start=5

Using the archive filter form:
http://localhost/joomla-cms-staging/en/archived-articles?month=1&year=2011&limitstart=0
http://localhost/joomla-cms-staging/en/archived-articles?month=1&year=2011&start=5

avatar csthomas
csthomas - comment - 24 Jan 2018

Currently yes. Staging has the same.
This happen after the second commit but the real problem is probably somewhere else.

avatar Quy
Quy - comment - 24 Jan 2018

I have tested this item successfully on 88122a0


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19447.

avatar Quy Quy - test_item - 24 Jan 2018 - Tested successfully
avatar csthomas
csthomas - comment - 24 Jan 2018

I found a place in code where I can fix it but I won't add it to this PR.

This PR is ready to test.

avatar Quy
Quy - comment - 24 Jan 2018

Please provide an example for the last commit.

avatar csthomas
csthomas - comment - 24 Jan 2018

When SEF is ON go to page like index.php?option=com_content&view=archive&Itemid=256&lang=en&month=0 (adjust to your Itemid for index.php/en/archived-articles)

Then check URL for the page 2.
Before: index.php/en/archived-articles?month=0&start=5
After: index.php/en/archived-articles?start=5

At the moment, it's a bit pedantic, but when I fix below issue

Using the archive filter form:
http://localhost/joomla-cms-staging/en/archived-articles?month=1&year=2011&limitstart=0
http://localhost/joomla-cms-staging/en/archived-articles?month=1&year=2011&start=5

it will be more useful.

avatar Quy
Quy - comment - 24 Jan 2018

I still get this index.php/en/archived-articles?month=0&start=5 with/without the PR.

avatar csthomas csthomas - change - 25 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 25 Jan 2018
avatar csthomas csthomas - change - 25 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 25 Jan 2018
avatar csthomas
csthomas - comment - 25 Jan 2018

Please follow instruction.

The last commit hide parameter month=0 in a next page link when you are first time of non SEF URL.
The link is not SEF but your joomla has SEF enabled and limit field value 5 is saved in session.

  1. Go to index.php/en/archived-articles and set limit to 5.
  2. Next go to non SEF URL index.php?option=com_content&view=archive&Itemid=256&lang=en&month=0, which is equivalent of index.php/en/archived-articles.
  3. Scroll to the bottom and check link for page 2:
    • before commit index.php/en/archived-articles?month=0&start=5
    • after commit index.php/en/archived-articles?start=5

Additional, if you apply below patch (I'm still working on it):

diff --git a/libraries/src/Router/SiteRouter.php b/libraries/src/Router/SiteRouter.php
index 6dbabc1ea0..35b5dfa881 100644
--- a/libraries/src/Router/SiteRouter.php
+++ b/libraries/src/Router/SiteRouter.php
@@ -404,6 +404,9 @@ class SiteRouter extends Router
                                        }
                                }
 
+                               // Set the information in the request
+                               $vars = array_merge($found->query, $vars);
+
                                $vars['Itemid'] = $found->id;
                                $vars['option'] = $found->component;
                        }
@@ -441,14 +444,6 @@ class SiteRouter extends Router
 
                        $route = implode('/', $segments);
                }
-               else
-               {
-                       // Set active menu item
-                       if ($item = $this->menu->getActive())
-                       {
-                               $vars = $item->query;
-                       }
-               }
 
                $uri->setPath($route);

then this will affect SEF links and after you click on filter you should never see parameters &month=0&year=0 on page links.

avatar Quy
Quy - comment - 25 Jan 2018

I have tested this item successfully on c15f303


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19447.

avatar Quy Quy - test_item - 25 Jan 2018 - Tested successfully
avatar grhcj
grhcj - comment - 26 Jan 2018

I have tested this item successfully on c15f303


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19447.

avatar grhcj grhcj - test_item - 26 Jan 2018 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 26 Jan 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 26 Jan 2018

Ready to Commit after two successful tests.

avatar csthomas
csthomas - comment - 26 Jan 2018

It would be nice to add this PR to 3.8.4 milestone because this PR could be treated as regression.

Example:
index.php/en/archived-articles/2011 returns error 404 on staging but works in 3.8.3 and in this PR.

I'm wrong. I did not remember correctly the comment from #19397 (comment).

avatar csthomas
csthomas - comment - 27 Jan 2018

I did a simple test.

At 3.8.3:
Go to archive view (index.php/en/archived-articles) and set Month to Jan and limit to 5.
The filter, next go to the second page (index.php/en/archived-articles?month=1&year=0&start=5)
After that go to third page (index.php/en/archived-articles?month=1&year=0&start=10)
Success. You are on 3rd page.

At staging:
Go to archive view (index.php/en/archived-articles) and set Month to Jan and limit to 5.
The filter, next go to the second page (index.php/en/archived-articles/0/1?view=archive&start=5)
After that go to third page, it is broken (index.php/en/archived-articles?id=1&start=10)
Failed. You can not get to page 3.

At this PR:
Go to archive view (index.php/en/archived-articles) and set Month to Jan and limit to 5.
The filter, next go to the second page (index.php/en/archived-articles?month=1&year=0&start=5)
After that go to third page (index.php/en/archived-articles?month=1&year=0&start=10)
Success. You are on 3rd page.

avatar mbabker mbabker - change - 27 Jan 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-01-27 16:27:20
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 27 Jan 2018
avatar mbabker mbabker - merge - 27 Jan 2018

Add a Comment

Login with GitHub to post a comment