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.

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

Bug: Correcting category associations #15664

Merged
merged 2 commits into from Apr 30, 2017

Conversation

infograf768
Copy link
Member

@infograf768 infograf768 commented Apr 28, 2017

Pull Request for Issue #15612

Summary of Changes

Correcting faulty code in #12042

Testing Instructions

Create a multilingual site (2 languages is enough)

Create a new article category for each language. Associate these new categories.
Create an article in each of these new categories. Tagged to the same language.

In the mainmenu for one of the languages, create a new menu item of type category list displaying the new category tagged to the same language as the Home page of that menu.

In frontend, display this menu item. Using the language switcher, click on the flag of the other language

Before patch

Clicking on that flag will display the home page of that language.

After patch

The associated category will display correctly.

@oliver-74
Please confirm on issues.joomla.org

@Bakual
This patch makes us lose the possible associated flag next to associated articles when displaying a blog or a category list. We do keep it OK when the view is article or featured.
Any way you see to not lose the feature for category views?

@infograf768
Copy link
Member Author

Grace to Bakual, we do not lose anymore the association in category view.
Example:
screen shot 2017-04-28 at 16 37 03

@@ -36,7 +36,7 @@ public static function getAssociations($id = 0, $view = null)
$view = $view === null ? $jinput->get('view') : $view;
$id = empty($id) ? $jinput->getInt('id') : $id;

if ($view === 'article' || $view === 'category' || $view === 'featured')
if ($view === 'article')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you should remove the || $view === 'featured' here. Only the category case needs to be gone.

Copy link
Member Author

Choose a reason for hiding this comment

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

it works though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Judging from code it could break if the current view is "featured". But I wasn't able to find a way to break it, so you may be right that it is not needed 👍

@infograf768
Copy link
Member Author

@alikon @AlexRed
Please test.

@AlexRed
Copy link
Contributor

AlexRed commented Apr 29, 2017

I have tested this item ✅ successfully on effd40c

Patch ok for me


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

@rjcf18
Copy link
Contributor

rjcf18 commented Apr 29, 2017

I have tested this item ✅ successfully on effd40c

Patch working ok for me as well. The associated category is correctly displayed.


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.1 milestone Apr 29, 2017
@ghost
Copy link

ghost commented Apr 29, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 29, 2017
@rdeutz rdeutz added this to the Joomla 3.7.1 milestone Apr 30, 2017
@rdeutz rdeutz merged commit d4c1c21 into joomla:staging Apr 30, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 30, 2017
@infograf768 infograf768 deleted the content_cat_assoc branch April 30, 2017 08:15
rdeutz pushed a commit to joomlajenkins/joomla-cms that referenced this pull request May 1, 2017
* Bug: Correcting category associations

* Thanks, Thomas
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants