User tests: Successful: Unsuccessful:
Pull Request for Issue #38846.
Check if taxonomy is "Type" and translate its data.
Before PR, Type: Category for all languages (Category not translated):
After PR, Category is translated:
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Status | New | ⇒ | Pending |
Category | ⇒ | Front End com_finder |
Labels |
Added:
?
|
I don't really want to have a special case just for the type, but translate everything.
What do you mean by translating everything?
other values are from item data linked to their associated language.
I confirm that it works and fixes the error.
@cyrezdev Before I submit a successful test, perhaps you can quickly fix an identical problem in the admin panel on the Smart Search: Index page. If it can't enter this PR - let me know and I'll post a successful test.
I will see to fix it in another PR (and others things i've seen to be improved or fixed in com_finder...).
So better to test this PR alone ;-)
Please keep pull requests to single tasks.
I have tested this item
I have tested this item
I don't really want to have a special case just for the type, but translate everything.
What do you mean by translating everything?
other values are from item data linked to their associated language.
@Hackwar Could you check and report back more detailed if something is wrong or missing? Thanks in advance.
Labels |
Added:
?
|
Labels |
Removed:
?
|
@Kostelano @viocassel Could you test again with the latest changes? Thanks in advance.
Thanks @richard67 for review!
@Kostelano @viocassel Sorry, it needs test again, as i've changed the code with error fix, and removal of language hasKey control (as already processed in branchSingular).
Could you test new version?
Thank you!
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
I have tested this item
Hmm, I just tested and it doesn't work for me.
@Kostelano @viocassel Are you sure you have tested according to the description in issue #38846 with the latest changes in this PR?
@cyrezdev I think you need to do it in the way like you had it at the beginning, i.e. replace the
<?php echo Text::_($text); ?>
by
<?php $key = LanguageHelper::branchSingular($text); ?>
<?php echo $lang->hasKey($key) ? Text::_($key) : $text; ?>
in the <?php if ($type === 'Type') : ?>
.
I will suggest a code change on GitHub.
Status | Ready to Commit | ⇒ | Pending |
Back to pending due to unsuccessful test.
@richard67 Yes, you're right, it's broken now. I tested late at night and somehow the latest changes to the assembly did not transfer. Sorry.
I can also confirm that your suggested change works.
Oh i'm sorry... Maybe too much tired due to covid... :-D
So, @richard67 not needed to control hasKey as already processed in LanguageHelper::branchSingular
But, this is what i missed in my last edit...
I've updated this PR, accordingly to my local code...
So, @richard67 not needed to control hasKey as already processed in LanguageHelper::branchSingular
Correct, I just saw that.
@Kostelano @viocassel the change was maybe not applied on your last test, so could you test again, and control code change is the same as the PR ?
Sorry for inconvenience, and thanks for your time testing! ;-)
I have tested this item
Now it definitely works!
I have tested this item
I don't really want to have a special case just for the type, but translate everything.
What do you mean by translating everything?
other values are from item data linked to their associated language.
@Hackwar Is your comment clarified with that response? If not, could you make any suggestions on what to change?
Did you not see #39353 (comment)? This fixes the issue for a specific hardcoded taxonomy but does not work for other taxonomies that may be translatable in the same manner.
Did you not see #39353 (comment)? This fixes the issue for a specific hardcoded taxonomy but does not work for other taxonomies that may be translatable in the same manner.
What you linked to is in another PR (for admin side): #39359
Not related to this PR (frontend search results)
What you linked to is in another PR (for admin side): #39359
It's probably a third party content type. For example, in this case, with such PR logic, smart search will work for the Contacts type (it really works - I checked it), but it may NOT work with a third-party component. The type may not be translated. Not sure.
What you linked to is in another PR (for admin side): #39359
It's probably a third party content type. For example, in this case, with such PR logic, smart search will work for the Contacts type (it really works - I checked it), but it may NOT work with a third-party component. The type may not be translated. Not sure.
I did this PR, as i'm working on a smart search plugin for one of my extension ;-)
So, works the same (core and third-party)
@cyrezdev What Sharky and Hackwar mean is that they don't like this $type === 'Type'
special treatment. Maybe a solution could be to just leave that if away and do the echo Text::_(LanguageHelper::branchSingular($text));
always. Not sure thought if that would be right. @Hackwar Was it that what you had in mind?
@cyrezdev What Sharky and Hackwar mean is that they don't like this
$type === 'Type'
special treatment. Maybe a solution could be to just leave that if away and do theecho Text::_(LanguageHelper::branchSingular($text));
always. Not sure thought if that would be right. @Hackwar Was it that what you had in mind?
Something like this:
<?php $text = LanguageHelper::branchSingular(implode(',', $taxonomy_text)); ?>
<?php echo Text::_($text); ?>
To explain why i did apply it only on Type
:
In don't have problems to apply it for all, but should we allow data translation ?
For example, if you have a category with title stored in database "Contact", this title will be translated.
Because of Language key "PLG_FINDER_QUERY_FILTER_BRANCH_S_CONTACT" in plg_finder_contacts.ini
Another example, you have an article with title "March".
If translation apply to all, you will have article title translated even if you don't want it (eg. in French, it will be translated in "Mars".
Because of Language key "MARCH" in joomla.ini
So, should we translate $item data?
Maybe another way without checking that branch name is "Type" is to check for branch parent_id.
What needs to be translated is all branches with parent_id = 1 (with ROOT as parent).
This could work:
<?php $text = implode(',', $taxonomy_text); ?>
<?php if ($branch->parent_id == 1) : ?>
<?php echo Text::_(LanguageHelper::branchSingular($text)); ?>
<?php else : ?>
<?php echo $text; ?>
<?php endif; ?>
What's your advice? @richard67 @Hackwar @SharkyKZ
Maybe another way without checking that branch name is "Type" is to check for branch parent_id.
What needs to be translated is all branches with parent_id = 1 (with ROOT as parent).
This could work:
<?php $text = implode(',', $taxonomy_text); ?> <?php if ($branch->parent_id == 1) : ?> <?php echo Text::_(LanguageHelper::branchSingular($text)); ?> <?php else : ?> <?php echo $text; ?> <?php endif; ?>
What's your advice? @richard67 @Hackwar @SharkyKZ
That could work. 1 is the root item in the taxonomy table, so that should not change.
Or you could check the „level“ column, that should be 2 when parent_id is 1 because that’s the first child level of the root item, and there should not be any other items with level 1, so all level 2 are children of the root item. Maybe that’s better, I don’t really know how safely we can rely on the root item always having id = 1. if someone deletes that with SQL and rebuilds the table, it might have another id than 1.
Or you could check the „level“ column, that should be 2 when parent_id is 1 because that’s the first child level of the root item, and there should not be any other items with level 1, so all level 2 are children of the root item. Maybe that’s better, I don’t really know how safely we can rely on the root item always having id = 1. if someone deletes that with SQL and rebuilds the table, it might have another id than 1.
You mean level 1 ?
For me, ROOT has level 0 (zero).
Or you could check the „level“ column, that should be 2 when parent_id is 1 because that’s the first child level of the root item, and there should not be any other items with level 1, so all level 2 are children of the root item. Maybe that’s better, I don’t really know how safely we can rely on the root item always having id = 1. if someone deletes that with SQL and rebuilds the table, it might have another id than 1.
You mean level 1 ?
For me, ROOT has level 0 (zero).
Sorry, you are right. Root item has level 0, so its direkt children have level 1.
Sorry, you are right. Root item has level 0, so its direkt children have level 1.
In fact, it won't work... I've tested. All taxonomy types have level 1. (Article, Category, Contact...)
I have 2 proposals:
1. Using a different way to add taxonomy when we expect its value to be translated.
For example, instead of this currently in plg_finder_content:
$item->addTaxonomy('Type', 'Article');
We can set it like this:
$item->addTaxonomy('Type', '**Article**');
And then, in search/default_results.php, code like this:
<?php
$text = implode(',', $taxonomy_text);
$branchName = trim($text, '**');
$key = LanguageHelper::branchSingular($branchName);
?>
<?php if ($text !== $branchName) : ?>
<?php echo Text::_($key); ?>
<?php else : ?>
<?php echo $text; ?>
<?php endif; ?>
It requires too an update for filter (select type) to trim **
:
$key = LanguageHelper::branchPlural(trim($node->title, '**'));
And for content map filter in admin:
$key = LanguageHelper::branchSingular(trim($item->text, '**'));
This way, it is backward compatible (same behavior as before with no translation, if addTaxonomy
not updated with **
to be translated).
2. No change with addTaxonomy
and plugins required, but won't be 100% in control.
(example case of a category with title stored in database "Contact": #39353 (comment)).
WARNING !
A concrete exemple about potential issue of the solution 2:
So code in search/default_results.php:
<?php
$text = implode(',', $taxonomy_text);
$key = LanguageHelper::branchSingular($text);
?>
<?php if ($text !== $key) : ?>
<?php echo Text::_($key); ?>
<?php else : ?>
<?php echo $text; ?>
<?php endif; ?>
SUM-UP:
Of course, if @richard67 @Hackwar @SharkyKZ and others agree to solution 1, i will update this PR accordingly. ;-)
@richard67 No news from other, but after some reflexions, i think the PR as it is, and was successfully tested, is valid.
As checking for type === 'Type'
is not an issue, because changing "Type" value would be a B/C break for third-party integration, so it will stay "Type".
So, this PR to fix this issue with translation, works.
And for else (better handling of translatable or not value), it would need a refactory of com_finder, which could be set for J5.0
(and then, a $item->addTaxonomyType('article')
would be better than $item->addTaxonomy('Type', 'article')
i think.)
What's your opinion?
@cyrezdev I'm not an expert on the finder. Let's see what @Hackwar says.
Ok, thank you!
But i don't think he has seen the possible issue of unwanted translation, if translating everything (not only Type value).
@Hackwar this PR fixes issue for com_finder in frontend as com_finder works currently, but it would need some refactory to extend its possibilities.
Possible issue in applying translation for all:
@richard67 @Kostelano @viocassel
As the previous PR got no news, and some requested translation for all values (#39353 (comment)), with no type check, i've redo a PR to simplify it, as i would expect this simple issue to be fixed asap! ;-)
So there the new PR: #39636
Thank you in advance for your testing!
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-01-15 14:30:15 |
Closed_By | ⇒ | cyrezdev |
I don't really want to have a special case just for the type, but translate everything.