Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Multilingual : Correcting info-block category and parent category urls #21916

Merged
merged 8 commits into from
Sep 6, 2018
+61 −50
Merged

[4.0] Multilingual : Correcting info-block category and parent category urls #21916

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
299689a
[4.0] Correcting info-block category and parent category urls
infograf768 Aug 30, 2018
e50668a
Getting rid of category slugs thanks Hannes
infograf768 Aug 31, 2018
c1d382f
missing one cat language variable
infograf768 Aug 31, 2018
c21eb52
Correcting routing for archived
infograf768 Aug 31, 2018
2141933
Getting the active language when item is tagged to ALL
infograf768 Aug 31, 2018
70f455e
Load current language when item is set to ALL
infograf768 Sep 1, 2018
f3dc6e9
factory not used
infograf768 Sep 1, 2018
e06a014
better code for menurules thanks @csthomas
infograf768 Sep 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions components/com_content/Model/ArticleModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ public function getItem($pk = null)
->where($db->quoteName('wa.stage_id') . ' = ' . $db->quoteName('ws.id'));

// Join on category table.
$query->select('c.title AS category_title, c.alias AS category_alias, c.access AS category_access')
$query->select('c.title AS category_title, c.alias AS category_alias, c.access AS category_access,' .
'c.language AS category_language')
->innerJoin('#__categories AS c on c.id = a.catid')
->where('c.published > 0');

Expand All @@ -130,7 +131,8 @@ public function getItem($pk = null)
}

// Join over the categories to get parent category titles
$query->select('parent.title as parent_title, parent.id as parent_id, parent.path as parent_route, parent.alias as parent_alias')
$query->select('parent.title as parent_title, parent.id as parent_id, parent.path as parent_route,' .
'parent.alias as parent_alias, parent.language as parent_language')
->join('LEFT', '#__categories as parent ON parent.id = c.parent_id');

// Join on voting table
Expand Down
6 changes: 4 additions & 2 deletions components/com_content/Model/ArticlesModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ protected function getListQuery()
->join('LEFT', '#__workflow_stages AS ws ON ws.id = wa.stage_id');

// Join over the categories.
$query->select('c.title AS category_title, c.path AS category_route, c.access AS category_access, c.alias AS category_alias')
$query->select('c.title AS category_title, c.path AS category_route, c.access AS category_access, c.alias AS category_alias,' .
'c.language AS category_language')
->select('c.published, c.published AS parents_published, c.lft')
->join('LEFT', '#__categories AS c ON c.id = a.catid');

Expand All @@ -262,7 +263,8 @@ protected function getListQuery()
->join('LEFT', '#__users AS uam ON uam.id = a.modified_by');

// Join over the categories to get parent category titles
$query->select('parent.title as parent_title, parent.id as parent_id, parent.path as parent_route, parent.alias as parent_alias')
$query->select('parent.title as parent_title, parent.id as parent_id, parent.path as parent_route, parent.alias as parent_alias,' .
'parent.language as parent_language')
->join('LEFT', '#__categories as parent ON parent.id = c.parent_id');

if (PluginHelper::isEnabled('content', 'vote'))
Expand Down
9 changes: 6 additions & 3 deletions components/com_content/View/Archive/HtmlView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,16 @@ public function display($tpl = null)

foreach ($items as $item)
{
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->catlanguage = $item->category_language;
$item->parentlanguage = $item->parent_language;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you adding these attributes again here? We don't need to duplicate these attributes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also don't need the slugs for the categories. For the categories we only need the ID.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you adding these attributes again here? We don't need to duplicate these attributes.

Looks like they are needed for the info-block JLayout

We also don't need the slugs for the categories. For the categories we only need the ID.

I was adapting to the info-block layout which uses slugs.


// No link for ROOT category
if ($item->parent_alias === 'root')
{
$item->parent_slug = null;
$item->parent_slug = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you revert this change here? Don't want to have unnecessary changes...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

}

$item->event = new \stdClass;
Expand Down
10 changes: 6 additions & 4 deletions components/com_content/View/Article/HtmlView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,11 @@ public function display($tpl = null)
$item->tagLayout = new FileLayout('joomla.content.tags');

// Add router helpers.
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->catlanguage = $item->category_language;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->parentlanguage = $item->parent_language;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, don't duplicate the attributes and we don't need the slugs.


// No link for ROOT category
if ($item->parent_alias === 'root')
Expand Down Expand Up @@ -302,7 +304,7 @@ protected function _prepareDocument()

while ($category && ($menu->query['option'] !== 'com_content' || $menu->query['view'] === 'article' || $id != $category->id) && $category->id > 1)
{
$path[] = array('title' => $category->title, 'link' => \ContentHelperRoute::getCategoryRoute($category->id));
$path[] = array('title' => $category->title, 'link' => \ContentHelperRoute::getCategoryRoute($category->id, $category->language));
$category = $category->getParent();
}

Expand Down
9 changes: 5 additions & 4 deletions components/com_content/View/Category/HtmlView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,18 @@ public function display($tpl = null)
// Compute the article slugs and prepare introtext (runs content plugins).
foreach ($this->items as $item)
{
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;

$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->catlanguage = $item->category_language;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->parentlanguage = $item->parent_language;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, don't duplicate the attributes and we don't need the slug.


// No link for ROOT category
if ($item->parent_alias === 'root')
{
$item->parent_slug = null;
}

$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->event = new \stdClass;

// Old plugins: Ensure that text property is available
Expand Down
8 changes: 5 additions & 3 deletions components/com_content/View/Featured/HtmlView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,11 @@ public function display($tpl = null)
// Compute the article slugs and prepare introtext (runs content plugins).
foreach ($items as &$item)
{
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$item->catslug = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
$item->parent_slug = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
$item->catlanguage = $item->category_language;
$item->parentlanguage = $item->parent_language;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above...


// No link for ROOT category
if ($item->parent_alias === 'root')
Expand Down
21 changes: 11 additions & 10 deletions components/com_content/helpers/route.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
defined('_JEXEC') or die;

use Joomla\CMS\Categories\CategoryNode;
use Joomla\CMS\Factory;
use Joomla\CMS\Language\Multilanguage;

/**
Expand Down Expand Up @@ -77,17 +78,17 @@ public static function getCategoryRoute($catid, $language = 0)
{
$link = 'index.php?option=com_content&view=category&id=' . $id;

if ($language && $language !== '*' && Multilanguage::isEnabled())
if ($language && Multilanguage::isEnabled())
{
$link .= '&lang=' . $language;
}

$jinput = JFactory::getApplication()->input;
$layout = $jinput->get('layout');

if ($layout !== '')
{
$link .= '&layout=' . $layout;
if ($language !== '*')
{
$link .= '&lang=' . $language;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you undo changes that do not do anything in this file, I mean just delete the part with the layout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

else
{
$lang = Factory::getLanguage()->getTag();
$link .= '&lang=' . $lang;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $lang changes should not be necessary. It defaults to the current language if no language is given. Did you check if this is necessary? The $layout is indeed wrong here. In any case, you should change the fingerprint of these methods to not have the $language (and also the $catid) be optional anymore. These are always required.

Copy link
Member Author

@infograf768 infograf768 Aug 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $lang changes should not be necessary. It defaults to the current language if no language is given. Did you check if this is necessary?

Alas it is. Tested that. Specially for the info-block at least.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, you should change the fingerprint of these methods to not have the $language (and also the $catid) be optional anymore. These are always required.

Any hint how to do that? And would it be B/C ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply remove the optional parameter stuff here. So from

 	public static function getCategoryRoute($catid, $language = 0)

to

 	public static function getCategoryRoute($catid, $language)

This is not B/C, but we are talking about Joomla 4.0 here, where we can break B/C and this is a case where we have to break it, to clear up this longlasting bug.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please remove the $lang changes. We should never have to add something if the language is *. If this currently is necessary, we have to fix this in another place, but not here. Not only because it is wrong, but also because this method is just an amenity, not a necessity. These methods only concatenate a string, nothing more, they don't look up anything. People can also just hand in their own handcrafted URL into the router, which could not contain the language parameter. So we have to see what the issue is here in the router and fix that and not hand in fake parameters. This code has to go.

Copy link
Member Author

@infograf768 infograf768 Aug 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please remove the $lang changes.

My problem here is that if I remove it, full tests can't be done on this PR as soon as we have a category set to ALL languages.

This is not B/C, but we are talking about Joomla 4.0 here, where we can break B/C and this is a case where we have to break it, to clear up this longlasting bug.

I would also do for getArticleRoute() I guess
I am not opposed to it. Need OK from
@wilsonge @mbabker

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a solution for the $lang stuff. Will add later on.

}
}

Expand Down
13 changes: 8 additions & 5 deletions components/com_content/tmpl/archive/default_items.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@
<div class="parent-category-name">
<?php $title = $this->escape($item->parent_title); ?>
<?php if ($params->get('link_parent_category') && !empty($item->parent_slug)) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->parent_slug)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->parent_slug),
$item->parentlanguage ) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php echo JText::sprintf('COM_CONTENT_PARENT', $url); ?>
<?php else : ?>
<?php echo JText::sprintf('COM_CONTENT_PARENT', '<span itemprop="genre">' . $title . '</span>'); ?>
Expand All @@ -73,7 +74,8 @@
<div class="category-name">
<?php $title = $this->escape($item->category_title); ?>
<?php if ($params->get('link_category') && $item->catslug) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->catslug)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->catslug,
$item->catlanguage)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php echo JText::sprintf('COM_CONTENT_CATEGORY', $url); ?>
<?php else : ?>
<?php echo JText::sprintf('COM_CONTENT_CATEGORY', '<span itemprop="genre">' . $title . '</span>'); ?>
Expand Down Expand Up @@ -146,7 +148,8 @@
<div class="parent-category-name">
<?php $title = $this->escape($item->parent_title); ?>
<?php if ($params->get('link_parent_category') && $item->parent_slug) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->parent_slug)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->parent_slug,
$item->parentlanguage)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php echo JText::sprintf('COM_CONTENT_PARENT', $url); ?>
<?php else : ?>
<?php echo JText::sprintf('COM_CONTENT_PARENT', '<span itemprop="genre">' . $title . '</span>'); ?>
Expand All @@ -159,7 +162,7 @@
<div class="category-name">
<?php $title = $this->escape($item->category_title); ?>
<?php if ($params->get('link_category') && $item->catslug) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->catslug)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($item->catslug, $item->catlanguage)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php echo JText::sprintf('COM_CONTENT_CATEGORY', $url); ?>
<?php else : ?>
<?php echo JText::sprintf('COM_CONTENT_CATEGORY', '<span itemprop="genre">' . $title . '</span>'); ?>
Expand Down Expand Up @@ -223,6 +226,6 @@
</p>
<?php endif; ?>
<div class="com-content-archive__pagination">
<?php echo $this->pagination->getPagesLinks(); ?>
<?php echo $this->pagination->getPagesLinks(); ?>
</div>
</div>
4 changes: 2 additions & 2 deletions components/com_content/tmpl/category/blog_children.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@
<?php echo $child->getNumItems(true); ?>
</span>
<?php endif; ?>
<a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id)); ?>">
<a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id, $child->language)); ?>">
<?php echo $this->escape($child->title); ?></a>

<?php if ($this->maxLevel > 1 && count($child->getChildren()) > 0) : ?>
<a href="#category-<?php echo $child->id; ?>" data-toggle="collapse" data-toggle="button" class="btn btn-xs float-right" aria-label="<?php echo Text::_('JGLOBAL_EXPAND_CATEGORIES'); ?>"><span class="icon-plus" aria-hidden="true"></span></a>
<?php endif; ?>
</h3>
<?php else : ?>
<h3 class="page-header item-title"><a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id)); ?>">
<h3 class="page-header item-title"><a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id, $child->language)); ?>">
<?php echo $this->escape($child->title); ?></a>
<?php if ( $this->params->get('show_cat_num_articles', 1)) : ?>
<span class="badge badge-info tip hasTooltip" title="<?php echo HTMLHelper::_('tooltipText', 'COM_CONTENT_NUM_ITEMS_TIP'); ?>">
Expand Down
4 changes: 2 additions & 2 deletions components/com_content/tmpl/category/default_children.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@
<?php echo $child->getNumItems(true); ?>
</span>
<?php endif; ?>
<a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id)); ?>">
<a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id, $child->language)); ?>">
<?php echo $this->escape($child->title); ?></a>

<?php if (count($child->getChildren()) > 0 && $this->maxLevel > 1) : ?>
<a href="#category-<?php echo $child->id; ?>" data-toggle="collapse" data-toggle="button" class="btn btn-xs float-right" aria-label="<?php echo Text::_('JGLOBAL_EXPAND_CATEGORIES'); ?>"><span class="icon-plus" aria-hidden="true"></span></a>
<?php endif; ?>
</h3>
<?php else : ?>
<h3 class="page-header item-title"><a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id)); ?>">
<h3 class="page-header item-title"><a href="<?php echo Route::_(ContentHelperRoute::getCategoryRoute($child->id, $child->language)); ?>">
<?php echo $this->escape($child->title); ?></a>
<?php if ( $this->params->get('show_cat_num_articles', 1)) : ?>
<span class="badge badge-info tip hasTooltip" title="<?php echo HTMLHelper::_('tooltipText', 'COM_CONTENT_NUM_ITEMS_TIP'); ?>">
Expand Down
5 changes: 3 additions & 2 deletions layouts/joomla/content/info_block/category.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
?>
<dd class="category-name">
<?php $title = $this->escape($displayData['item']->category_title); ?>
<?php if ($displayData['params']->get('link_category') && $displayData['item']->catslug) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($displayData['item']->catslug)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php if ($displayData['params']->get('link_category') && !empty($displayData['item']->catslug)) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($displayData['item']->catslug,
$displayData['item']->catlanguage)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php echo Text::sprintf('COM_CONTENT_CATEGORY', $url); ?>
<?php else : ?>
<?php echo Text::sprintf('COM_CONTENT_CATEGORY', '<span itemprop="genre">' . $title . '</span>'); ?>
Expand Down
3 changes: 2 additions & 1 deletion layouts/joomla/content/info_block/parent_category.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
<dd class="parent-category-name">
<?php $title = $this->escape($displayData['item']->parent_title); ?>
<?php if ($displayData['params']->get('link_parent_category') && !empty($displayData['item']->parent_slug)) : ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($displayData['item']->parent_slug)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php $url = '<a href="' . Route::_(ContentHelperRoute::getCategoryRoute($displayData['item']->parent_slug,
$displayData['item']->parent_language)) . '" itemprop="genre">' . $title . '</a>'; ?>
<?php echo Text::sprintf('COM_CONTENT_PARENT', $url); ?>
<?php else : ?>
<?php echo Text::sprintf('COM_CONTENT_PARENT', '<span itemprop="genre">' . $title . '</span>'); ?>
Expand Down