[#19012] - [Modern Routing] make a simpler loop in StandardRules::build
- Closed
- 3 Jan 2018
- Medium
- Build: staging
- # 19012
- Diff
- csthomas:make_simpler_standard_rule_build
User tests: Successful: Unsuccessful:
Summary of Changes
Replace the old complex loop with a simpler one.
Remove PHP notice.
Everything should work as before.
This PR adds a way to build correct URL for below structure:
$featured = new JComponentRouterViewconfiguration('featured');
$this->registerView($featured);
//$categories = new JComponentRouterViewconfiguration('categories');
//$categories->setKey('id')->setParent($featured);
//$this->registerView($categories);
$category = new JComponentRouterViewconfiguration('category');
$category->setKey('id')->setParent($featured)->setNestable()->addLayout('blog');
$this->registerView($category);
$article = new JComponentRouterViewconfiguration('article');
$article->setKey('id')->setParent($category, 'catid');
$this->registerView($article);
$this->registerView(new JComponentRouterViewconfiguration('archive'));
//$this->registerView(new JComponentRouterViewconfiguration('featured'));
$form = new JComponentRouterViewconfiguration('form');
$form->setKey('a_id');
$this->registerView($form);Testing Instructions
Install Joomla with sample data testing.
After you replace php code in com_content/router.php as describer above:
- disable all menu items for categories and category view.
- add a featured view to menu item if missing
- Go to
index.php/component/content/categories/and compare link to category view before and after patch.
Expected result
No PHP notices.
Link to the top category view should be like
/featured-articles/14-sample-data-articles
Actual result
There is a PHP Notice: Undefined index id on line 265.
Documentation Changes Required
No
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Title |
|
||||||
| Labels |
Added:
?
|
||
Manually,
to test this PR you need a custom routing configuration as above.
StandardRules from modern routing should support a various configurations, above too.
To create a test example (in order to not install a separate component) I created a custom configuration for com_content in order to show a bug in StandardRules build method.
For such custom configuration, the link to the top category should look like /featured-articles/14-sample-data-articles.
You do not need to click on that link because it has not work yet (parse method has not support such custom configuration yet) and returns error 404. I would like to leave it for another PR.
For this PR I would like to change the loop to simpler and remove PHP notice.
@joomdonation As you are familiar with StandardRules class, would you like to to add your opinion?
But thanks for interest:)
I have tested this item
After following the instructions the top link has the url "/featured-articles/sample-data-articles" instead of the php notice about an undefined id index.
Like expected the link leads to error 404. With IDs in url its workting too.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19012.
@csthomas I think for this PR you should focus in make a simpler loop in StandardRules::build task only. That part is good and easier to understand compare to the original code. It will help testers easier to review the change and also easier for maintainer to review and merge.
For adding support for new router configuration, I would leave it for the next PR as I am unsure if it needed for now (and as you mentioned, the generated link causes 404 error anyway). The routing code is quite complex, so I prefer small step at a time.
Ok. When I have more time to think about it I will fix it.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-01-03 11:55:52 |
| Closed_By | ⇒ | csthomas |
this is be done by applying PR or manually?