User tests: Successful: Unsuccessful:
Pull Request for Issue #20667.
Adds a check function if associations is available for categories of the current component.
The Associations column should display.
No Associations column is displayed.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_categories Libraries |
Labels |
Added:
?
|
@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?
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.
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.
BTW, patch works fine in admin everywhere where we display/edit categories.
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.
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.
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?
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.
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.
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.
@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
Category | Administration com_categories Libraries | ⇒ | Administration com_categories com_contact com_content com_menus com_newsfeeds Libraries Modules Front End Plugins |
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?
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;
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)
Can you test again, changed to $cassociations
.
It solves what it did in my test. The remaining issues are the urls produced when no menu items.
For example with associated articles which categories are also associated I get
French
http://localhost:8888/newfolder/joomla40/fr/component/content/article/3-article-fr-fr-2?catid=10
English
http://localhost:8888/newfolder/joomla40/en/component/content/article/4-article-en-gb-2?catid=11
@Hackwar @csthomas
@wilsonge For me this PR is a good step and can be merged, except if it demonstrated that it has an impact on the routing.
Note: wanted to test when associating contacts and contacts categories (as they use the old way) but it is broken. Will create a new issue.
PHP Fatal error: Cannot declare class Joomla\Component\Contact\Site\Helper\Route, because the name is already in use in /Applications/MAMP/htdocs/newfolder/joomla40/components/com_contact/helper/route.php on line 23
In fact, it has an impact on routing, which means that much more has to be done.
When using this patch, switching between home pages is also incorrect.
Let's say I am on a fr home page
url is http://localhost:8888/testnew/joomla40/fr
The language switcher to en gives the url http://localhost:8888/testnew/joomla40/en/component/content/category/11-category-en-gb
instead of http://localhost:8888/testnew/joomla40/en
same for other languages.
This breaks multilang login on these pages.
There is an issues with router and lang parameter in URL.
The link generated from association looks like:
index.php?option=com_content&view=category&id=8&lang=pl-PL&lang=pl
Associations use $language->lang_code
but router requires $language->sef
.
A quick fix (for associations with menu items) is to replace if
andelseif
places in below code.
joomla-cms/modules/mod_languages/Helper/LanguagesHelper.php
Lines 121 to 129 in 5b04eea
IMO the menu association takes precedence over category associations.
@infograf768 does the routing work correct without the patch? Can you give me step by step instructions how I can reproduce the routing issue?
@csthomas
category or item associations have always taken precedence over menu associations as you can see in the code order in this helper as well as in the languagefilter plugin.
I.e. $cassociations (categories and items) over $associations (menu items)
we may indeed reconsider this. Not sure.
But I do not understand your comment about $language->sef as it correctly set in both files concerned.
also not sure what you mean by dropping if and elseif
Here, on my 4.0 multilingual test site, SEF off, I get for associated articles
index.php?option=com_content&view=article&id=7&catid=15&lang=fr
which is perfectly OK.
and for category
index.php?option=com_content&view=category&id=15&lang=fr
which is perfectly fine too.
EDIT: results obtained with this patch!
Therefore the issue we have is when we use SEF.
@laoneo
Just an example when SEF on: the home pages.
Each is a blog menu item (using the new multilingual sample data helps a lot to set up fastly such a test site)
Without your patch, the url obtained via the language switcher gives the correct ones (In 3.x there is also a slash at the end):
http://localhost:8888/testnew/joomla40/el
for Greek
http://localhost:8888/testnew/joomla40/en
for English
http://localhost:8888/testnew/joomla40/fr
for French
See above what I get:
#20692 (comment)
But, still without your patch, clicking on the article title in this home page is also wrong when switching
For example (I did not remove default language code). Both the categories of these articles and these articles are associated.
original page
http://localhost:8888/testnew/joomla40/fr/7-article-fr-fr
gives, which indeed displays the article concerned but only because they are displayed by their home page.
http://localhost:8888/testnew/joomla40/el
for Greek...
http://localhost:8888/testnew/joomla40/en
for English
With your patch, it gives
original: http://localhost:8888/testnew/joomla40/fr/7-article-fr-fr
as above
association
Greek: http://localhost:8888/testnew/joomla40/el/component/content/article/8-%CE%AC%CF%81%CE%B8%CF%81%CE%BF-el-gr?catid=16
( I am using unicode alias for that Greek article)
English: http://localhost:8888/testnew/joomla40/en/component/content/article/6-article-en-gb?catid=14
Once switched, when I switch back to French
http://localhost:8888/testnew/joomla40/fr/component/content/article/7-article-fr-fr?catid=15
Also login works fine from the module. Depending on the language, I may get a 404 with your patch.
EDIT: with your patch, the 404 are obtained when the url of the page (or the associated page if a redirect is done to the user preferred language) is of the type
http://localhost:8888/testnew/joomla40/fr/component/content/article/7-article-fr-fr?catid=15
To get an info for debug I did:
$language->link = Route::_($cassociations[$language->lang_code] . '&lang=' . $language->sef);
$language->link = $cassociations[$language->lang_code] . '&lang=' . $language->sef;
Then I get weird link like .../index.php?option=com_content&view=category&id=9&lang=en-GB&lang=en
where the "lang" parameter is repeated.
To get a correct link you can remove the lang parameter as below:
$language->link = Route::_($cassociations[$language->lang_code]);
then all seems to be OK.
SO I get it right, with and without the patch it is broken? If yes, can we do the routing problem in another pr then? Otherwise this one turns into a "fix all language problem" pr.
@csthomas
This indeed works for com_content but maybe because $cassociations
is now coming from
if ($component instanceof AssociationServiceInterface)
{
$cassociations = $component->getAssociationsExtension()->getAssociationsForItem();
}
@laoneo
Not sure at all that it would work for other components that have not been touched by your patch.
Before changing, we must test with other comps.
hmm, dumping $cassociations here and from 3.x gives the same type of array.
What has the hell changed in 4.0 with route vs 3.x ?
Tested with Newsfeeds and, alas, I get a similar error as contacts
[15-Jun-2018 10:15:05 Europe/Berlin] PHP Fatal error: Cannot declare class Joomla\Component\Newsfeeds\Site\Helper\Route, because the name is already in use in /Applications/MAMP/htdocs/testnew/joomla40/components/com_newsfeeds/helper/route.php on line 20
Multilingual links use ...&lang=en
or ...&lang=en-GB
. There are no consequence.
The class variable Joomla\CMS\Component\Router\Rules\MenuRules::lookup
has different keys ('en', 'en-GB') for the same language.
Menu items with selected language appear only in the longer key (for English in 'en-GB').
For com_content the link (after this PR) looks like
index.php?option=com_content&view=category&id=9&lang=en-GB&lang=en
.
after my trick it will be:
index.php?option=com_content&view=category&id=9&lang=en-GB
(5 chars).
but in other places joomla still uses ...&lang=en
. (2 chars)
The main question is, which one is correct?
$language->sef is normally ok, i.e. the content language url code i.e. en
in 3.x joomla also understands the language tag i.e en-GB
not at my desk but the editors xtd do use I think the tag and no issue
I prepared a patch which should fix the problem, the above trick is not needed.
diff --git a/libraries/src/Component/Router/Rules/MenuRules.php b/libraries/src/Component/Router/Rules/MenuRules.php
index ab14b509a2..1ea8ed866f 100644
--- a/libraries/src/Component/Router/Rules/MenuRules.php
+++ b/libraries/src/Component/Router/Rules/MenuRules.php
@@ -75,6 +75,17 @@ class MenuRules implements RulesInterface
// Get query language
$language = isset($query['lang']) ? $query['lang'] : '*';
+ if ($language !== '*')
+ {
+ // Translate the language sef to lang_code
+ $languages = \Joomla\CMS\Language\LanguageHelper::getLanguages('sef');
+
+ if (isset($languages[$language]))
+ {
+ $language = $languages[$language]->lang_code;
+ }
+ }
+
if (!isset($this->lookup[$language]))
{
$this->buildLookup($language);
Does not solve the single Newsfeeds associations.
For the blog home pages, still same issue: missing the slash after the url language code.
Otherwise, no login error any more for article without specific menu item WHEN THEY are associated as I get "normal" urls.
http://localhost:8888/testnew/joomla40/fr/7-article-fr-fr
http://localhost:8888/testnew/joomla40/en/6-article-en-gb
http://localhost:8888/testnew/joomla40/el/8-%CE%AC%CF%81%CE%B8%CF%81%CE%BF-el-gr
But, if articles are not associated AND their categories are associated, the switcher sends me to the home pages instead of using the allcategories menu item as a base for their urls.
BTW, I checked the xtd code when entering an article link in an article
<p><a href="index.php?option=com_content&view=article&id=6&catid=14&lang=en-GB" hreflang="en">Article (en-gb)</a></p>
so, I was right that it does use the tag and not the sef. URL produced is fine ( http://localhost:8888/testnew/joomla40/en/6-article-en-gb
)
I will convert newsfeed later to services, then it is very likely that the issue will get fixed too. It was the case as I converted com_config which solved a lot of front end editing stuff for module editing and template/sysconfig changes in front. So please lets not mix things here.
I'm pretty sure that the issue is because the components do get namespaces and not yet migrated to services. An old legavy component should work as beford without a BC break.
https://github.com/Bakual/SermonSpeaker/releases/download/6.0.0-alpha1/com_sermonspeaker.zip should work. It's the one I started adapting to 4.0.
I have issues with sermonspeaker
It has many errors, including a Fatal one
[16-Jun-2018 10:42:06 Europe/Berlin] PHP Fatal error: Class SermonSpeakerAssociationsHelper contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Joomla\CMS\Association\AssociationExtensionInterface::getAssociationsForItem) in /Applications/MAMP/htdocs/testnew/joomla40/administrator/components/com_sermonspeaker/helpers/associations.php on line 228
Also many Notices and Undefined Property and com_associations does not display at all.
As for urls obtained, no good, which I guess is expected when we consider the errors:
A menu item like http://localhost:8888/testnew/joomla40/en/sermonspeaker-cat-list-en
displays one category
Clicking on the title of this category gives a 404 therefore I can't test if the associated category works or not.
Frontend Notice:
[16-Jun-2018 10:52:57 Europe/Berlin] PHP Notice: Use of undefined constant JPATH_COMPONENT_SITE - assumed 'JPATH_COMPONENT_SITE' in /Applications/MAMP/htdocs/testnew/joomla40/modules/mod_languages/Helper/LanguagesHelper.php on line 82
Are these due to the component or 4.0 with services and this patch? No idea.
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 :)
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.
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
Merged upstream and fixed conflicts.
I have tested this item
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.
I have tested this item
May be incomplete, but without it in core we can't test associations at least for com_content.
Please, again, @wilsonge , merge.
I have tested this item
May be incomplete, but without it in core we can't test associations at least for com_content.
Please, again, @wilsonge , merge.
Status | Pending | ⇒ | Ready to Commit |
RTC
Labels |
Added:
?
|
Updated branch to allow merging.
Restarting drone.
@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
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-07-13 12:57:48 |
Closed_By | ⇒ | laoneo |
Going to merge this one to move on with associations testing. Thanks to the testers. Please provide the routing patch in a new pr.
For legacy everything should work as before. We just extended it with services.
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.
I suggest to solve this one before modifying these components to use services.
Please open a new issue as I don't think it is related to services at all.
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.