[4.0] Check on the component if the categories support is available #20692

laoneo · 2018-06-08T16:26:02Z

Pull Request for Issue #20667.

Summary of Changes

Adds a check function if associations is available for categories of the current component.

Testing Instructions

Enable the Language filter plugin and make sure Associations is set to Yes
Display Articles Categories Manager

Expected result

The Associations column should display.

Actual result

No Associations column is displayed.

carlitorweb · 2018-06-08T17:55:39Z

administrator/components/com_categories/Model/CategoriesModel.php

+		if ($component instanceof AssociationServiceInterface)
+		{
+			$assoc = $component->hasAssociationsCategorySupport();
+			return $assoc;


[CS] Add an empty line before the return

carlitorweb · 2018-06-08T17:55:53Z

administrator/components/com_categories/Model/CategoriesModel.php

@@ -326,15 +327,22 @@ public function getAssoc()
 		if (!$assoc || !$component || !$cname)
 		{
 			$assoc = false;
+			return $assoc;


[CS] Add an empty line before the return

carlitorweb · 2018-06-08T17:57:04Z

administrator/components/com_categories/Model/CategoryModel.php

@@ -1281,15 +1284,22 @@ public function getAssoc()
 		if (!$assoc || !$component || !$cname)
 		{
 			$assoc = false;
+			return $assoc;


[CS] Add an empty line before the return

carlitorweb · 2018-06-08T17:57:17Z

administrator/components/com_categories/Model/CategoryModel.php

+		if ($component instanceof AssociationServiceInterface)
+		{
+			$assoc = $component->hasAssociationsCategorySupport();
+			return $assoc;


[CS] Add an empty line before the return

carlitorweb · 2018-06-08T18:00:19Z

I have tested this item ✅ successfully on b2f878b

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20692.}

infograf768 · 2018-06-08T18:22:55Z

@laoneo
Will test but I do not understand what is going on here. Can you explain how
\JLoader::register($hname, JPATH_SITE . '/components/' . $component . '/helpers/association.php');
may work when there is no such file for com_content as you moved and changed the file, its name and its class?

mbabker · 2018-06-08T19:04:57Z

libraries/src/Association/AssociationServiceTrait.php

+	 *
+	 * @since  __DEPLOY_VERSION__
+	 */
+	public function hasAssociationsCategorySupport()


Do you really need this method in the interface/trait? You can't just have code do the instanceof check without relying on an interface to be able to say you can do this instanceof check?

If every component which has associations and categories integrated, does also have associations for the category automatically, then yes. Wasn't sure about it?

I just don't get what the difference is in using $component instanceof CategoriesServiceInterface versus $component->hasAssociationsCategorySupport() is all. Like why does this has method need to be part of an interface? Unless you've got a plan for being able to dynamically toggle this support it just feels like adding an interface method for the sake of having it.

Actually the component has control to decide if categories associations is supported, what the trait does is just to add default behavior. As I said if we can guarantee that when a component implements the AssociationsServiceInterface and Categories that associations for categories is then also working for the component. Then I'm more that happy to remove it. I just don't know that detail. Perhaps @infograf768 can shed some light here.

mbabker · 2018-06-08T19:06:22Z

may work when there is no such file for com_content as you moved and changed the file, its name and its class?

Simple. That code path won't execute in the case of com_content because of the checks above it. So if a component follows the service architecture, it'll get what it needs from that and return. If it doesn't, then it'll try to locate the class and file the "old" way.

infograf768 · 2018-06-09T05:43:34Z

@laoneo

I just don't know that detail. Perhaps @infograf768 can shed some light here.

What was done when was implemented com_associations was to make sure to define for each component its possible associations.

For example for com_contacts in the administrator/components/com_contact/helpers/associations.php the getType()` method (and also in getAssociations() and getItem() methods btw). Same for com_newsfeeds and also in its specific repo for weblinks.

If you look at com_menus similar file, you will see that category is not part of the code.

For com_content you moved the file and changed the class: similar methods are present in administrator/components/com_content/Helper/AssociationsHelper.php

My question above would have been more clear is I had asked why you decided to change the former structure for com_content ONLY as it was working perfectly OK and is still doing so for the other components. The Service Architecture is rather Tibetan for me ( https://en.wikipedia.org/wiki/Service-oriented_architecture ) as you may guess 😃 .
Or, if I understand well, it means you will also change the way it is done for the other components in core in the near future as it would look unclean to me.

infograf768 · 2018-06-09T05:44:57Z

BTW, patch works fine in admin everywhere where we display/edit categories.

laoneo · 2018-06-09T06:10:25Z

Yes, the strategy is to bring com_content into a state where we can say it is the way to go for all extensions and then I will convert the rest. Don't worry about different set ups between core components for now.
What my question is, does have a component associations support for categories automatically when it implements associations for its entities and uses categories? Because then we can remove the mentioned check by @mbabker. Otherwise not.

infograf768 · 2018-06-09T06:13:25Z

does have a component associations support for categories automatically when it implements associations for its entities and uses categories?

As I said, if it is coded such in the component association helper, it will. Nothing is automatic. Not sure I understand what you exactly mean otherwise.

laoneo · 2018-06-09T06:25:07Z

I'm not talking about how it is implemented but more about the set up. Lats try it differently. Can a component has associations and categories but not associations support for categories?

infograf768 · 2018-06-09T06:27:28Z

Can a component has associations and categories but not associations support for categories?

If it has, it means that it is wrongly coded. There is no reason to do so and I don't know of any.
When a component has categories and support associations generally speaking, then associations SHOULD work for categories.

laoneo · 2018-06-09T06:31:56Z

Because when I remove the check function now, it is not possible (or only with an effort which we can avoid now) to add it later when J4 is released. That's why I want to have confirmation on this.

infograf768 · 2018-06-09T06:37:46Z

TBH: if someone coding a multingual aware component which uses categories does not add the necessary code to also associate categories, this coder should just be eliminated from the surface of the Earth by any means, even illegal ones. 😃

infograf768 · 2018-06-09T07:51:02Z

@laoneo
Looks like we have some places where we need also some changes
The language filter plugin is an example
https://github.com/joomla/joomla-cms/blob/4.0-dev/plugins/system/languagefilter/languagefilter.php#L786-L794

…/associations

laoneo · 2018-06-09T11:31:08Z

Moved the helper function getAssociations to the extension helper class which is accessible through the service. This comes with the cost of a BC break. An alternative would be to introduce a new helper interface and a function in AssociationServiceInterface like:
public function getAssociationsHelper(): AssociationHelperInterface;.

Additionally all of this must be set from the service provider down to the component class. Not sure if we want to go that way. What do you guys think?

infograf768 · 2018-06-09T15:07:09Z

This still does not work for multilingual associations between content categories for example (I guess also articles).
$cassociations is always NULL in this code

			if ($component instanceof AssociationServiceInterface)
			{
				$associations = $component->getAssociationsExtension()->getAssociationsForItem();
			}
			else
			{
				$cName = StringHelper::ucfirst(StringHelper::str_ireplace('com_', '', $option)) . 'HelperAssociation';
				JLoader::register($cName, JPath::clean(JPATH_COMPONENT_SITE . '/helpers/association.php'));

				if (class_exists($cName) && is_callable(array($cName, 'getAssociations')))
				{
					$cassociations = call_user_func(array($cName, 'getAssociations'));
				}
			}

We need $cassociations further down

					// Component association
					case (isset($cassociations[$i])):
						$language->link = Route::_($cassociations[$i] . '&lang=' . $language->sef);
						break;

infograf768 · 2018-06-09T16:51:41Z

I tested changing in mod_languages as well as languagefilter
$associations = $component->getAssociationsExtension()->getAssociationsForItem();
to
$cassociations = $component->getAssociationsExtension()->getAssociationsForItem();
and it looks like it solves my issues., i.e. when no specific menu item to display an associated article or an associated category, we can switch OK (not so nice URLS but it works):

For example the display of a sub category
http://localhost:8888/newfolder/joomla40/fr/component/content/category/10-catfrenchsub
will give when using the lang switcher to en-GB
http://localhost:8888/newfolder/joomla40/en/component/content/category/11-catenglishsub

I am rather expecting (as I have a hidden List All Categories menu item for each language with alias categories) something like
http://localhost:8888/newfolder/joomla40/en/categories/11-catenglishsub in en-GB
and
http://localhost:8888/newfolder/joomla40/fr/categories/10-catfrenchsub in fr-FR

But I guess this is due to the "modern router" in 4.0 (as it does work fine in staging)

Bakual · 2018-06-16T09:59:02Z

Hard to say. I've only rewritten it for Joomla 4.0 alpha from last year and the release is from january. There were quite a lot of major changes to the repo since then. I'm not constantely adjusting it to the latest changes in architecture :)

laoneo · 2018-06-16T18:50:41Z

Try it with the j3 version as we should support them and we need to make sure these versions do work. Not one which is made for J4 alpha 1.

Bakual · 2018-06-16T19:26:21Z

The J3 version likely doesn't work as it uses various deprecated functions.
My plan was to get it working in J4 and then "backport" whatever is needed to get a "transition" version that works both in 3.10 and 4.0.
But that obviously failed as I started to early 😄

wilsonge · 2018-06-20T16:38:16Z

@laoneo can you fix the conflicts with this one so I can merge it please?

laoneo · 2018-06-26T13:32:27Z

Merged upstream and fixed conflicts.

richard67 · 2018-06-30T07:31:41Z

I have tested this item ✅ successfully on 720f944

Tested on current 4.0-dev + this PR applied.
Associations column in categories manager is shown.
Associations tab in category edit view is shown.
Category associations can be added/changed/edited/used as it should be.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20692.}

infograf768 · 2018-07-06T07:46:27Z

@wilsonge Can this be merged?

infograf768 · 2018-07-13T07:51:20Z

I have tested this item ✅ successfully on 720f944

May be incomplete, but without it in core we can't test associations at least for com_content.

Please, again, @wilsonge , merge.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20692.}

infograf768 · 2018-07-13T07:51:39Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20692.}

infograf768 · 2018-07-13T07:53:18Z

Updated branch to allow merging.

infograf768 · 2018-07-13T08:12:13Z

Restarting drone.

brianteeman · 2018-07-13T08:14:33Z

@infograf768 i have no idea what that error is but its been there for a while - i did ask what it was but got no response

laoneo · 2018-07-13T12:59:10Z

Going to merge this one to move on with associations testing. Thanks to the testers. Please provide the routing patch in a new pr.

infograf768 · 2018-07-13T13:00:09Z

Thanks @laoneo
We certainly will need quite a few modifications/improvements specially for legacy but at least people working on multilang will have one possible test.

laoneo · 2018-07-13T13:23:46Z

For legacy everything should work as before. We just extended it with services.

infograf768 · 2018-07-13T14:23:44Z

@laoneo

I still get issues in back-end com_associations for com_contact for example (who has its associations class in a different file/location and with a different type of class name).

Here I still get for single contact

Notice: Trying to get property of non-object in /Applications/MAMP/htdocs/installmulti/joomla40/administrator/components/com_associations/Helper/AssociationsHelper.php on line 578
and
Undefined index: language in /Applications/MAMP/htdocs/installmulti/joomla40/administrator/components/com_associations/Field/ItemlanguageField.php on line 55
and
Call to a member function checkin() on null

Did not test but I guess it will be the same for newsfeeds.

infograf768 · 2018-07-13T14:54:12Z

I suggest to solve this one before modifying these components to use services.

laoneo · 2018-07-13T16:54:53Z

Please open a new issue as I don't think it is related to services at all.

Check on the component if the categories support is available

5ce4b9f

joomla-cms-bot added the PR-4.0-dev label Jun 8, 2018

Merge branch '4.0-dev' into j4/fix/associations

b2f878b

laoneo mentioned this pull request Jun 8, 2018

[4.0] Multililanguage: Associations do not display any more for articles categories #20667

Closed

carlitorweb reviewed Jun 8, 2018

View reviewed changes

mbabker reviewed Jun 8, 2018

View reviewed changes

Merge branch '4.0-dev' into j4/fix/associations

1b47871

laoneo added 3 commits June 9, 2018 13:16

add check

b80397d

Move getAssociations to interface

a451a2a

Merge remote-tracking branch 'origin/j4/fix/associations' into j4/fix…

da40e5d

…/associations

Revert

7218d80

Change to $cassociations

dbc586a

laoneo added 2 commits June 26, 2018 15:30

Merge branch '4.0-dev' into j4/fix/associations

fb82795

Merge branch '4.0-dev' into j4/fix/associations

720f944

infograf768 mentioned this pull request Jul 6, 2018

Automatically create associations of selected languages joomla-projects/gsoc18_automatic_multilingual_association#4

Open

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 13, 2018

Merge branch '4.0-dev' into j4/fix/associations

f26ebdd

Merge branch '4.0-dev' into j4/fix/associations

50aea58

laoneo merged commit 77c731f into joomla:4.0-dev Jul 13, 2018

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 13, 2018

laoneo deleted the j4/fix/associations branch July 13, 2018 12:57

zero-24 added this to the Joomla 4.0 milestone Jul 13, 2018

This was referenced Jul 14, 2018

[4.0] Multilang: com_associations broken for contacts and newsfeeds #21110

Closed

[4.0] Multilanguage wrong url given by language switcher for homes of type category #21132

Closed

[4.0] Check on the component if the categories support is available #20692

[4.0] Check on the component if the categories support is available #20692

Conversation

laoneo commented Jun 8, 2018 • edited by infograf768 Loading

Summary of Changes

Testing Instructions

Expected result

Actual result

carlitorweb Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

carlitorweb Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

carlitorweb Jun 8, 2018

Choose a reason for hiding this comment

carlitorweb Jun 8, 2018

Choose a reason for hiding this comment

carlitorweb commented Jun 8, 2018

infograf768 commented Jun 8, 2018

mbabker Jun 8, 2018

Choose a reason for hiding this comment

laoneo Jun 8, 2018

Choose a reason for hiding this comment

mbabker Jun 8, 2018

Choose a reason for hiding this comment

laoneo Jun 9, 2018 • edited Loading

Choose a reason for hiding this comment

mbabker commented Jun 8, 2018

infograf768 commented Jun 9, 2018

infograf768 commented Jun 9, 2018

laoneo commented Jun 9, 2018

infograf768 commented Jun 9, 2018

laoneo commented Jun 9, 2018

infograf768 commented Jun 9, 2018 • edited Loading

laoneo commented Jun 9, 2018 • edited Loading

infograf768 commented Jun 9, 2018

infograf768 commented Jun 9, 2018

laoneo commented Jun 9, 2018

infograf768 commented Jun 9, 2018 • edited Loading

infograf768 commented Jun 9, 2018 • edited Loading

Bakual commented Jun 16, 2018

laoneo commented Jun 16, 2018

Bakual commented Jun 16, 2018

wilsonge commented Jun 20, 2018

laoneo commented Jun 26, 2018

richard67 commented Jun 30, 2018 • edited Loading

infograf768 commented Jul 6, 2018

infograf768 commented Jul 13, 2018

infograf768 commented Jul 13, 2018

infograf768 commented Jul 13, 2018

infograf768 commented Jul 13, 2018

brianteeman commented Jul 13, 2018

laoneo commented Jul 13, 2018

infograf768 commented Jul 13, 2018

laoneo commented Jul 13, 2018

infograf768 commented Jul 13, 2018

infograf768 commented Jul 13, 2018

laoneo commented Jul 13, 2018

laoneo commented Jun 8, 2018 •

edited by infograf768

Loading

carlitorweb Jun 8, 2018 •

edited

Loading

carlitorweb Jun 8, 2018 •

edited

Loading

laoneo Jun 9, 2018 •

edited

Loading

infograf768 commented Jun 9, 2018 •

edited

Loading

laoneo commented Jun 9, 2018 •

edited

Loading

infograf768 commented Jun 9, 2018 •

edited

Loading

infograf768 commented Jun 9, 2018 •

edited

Loading

richard67 commented Jun 30, 2018 •

edited

Loading