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

tag alias #38576

Merged
merged 4 commits into from
Sep 7, 2022
Merged

tag alias #38576

merged 4 commits into from
Sep 7, 2022

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Aug 23, 2022

You should be able to create tags with parents and children such as

  • Sport
    -- adults
    -- children
  • games
    -- adults
    -- children

Before this PR you could not as you would get a duplicate alias message error even though the tags have different parents and there is no reason that I can see to prevent this I may be very wrong as I dont use tags that often

Pull Request for Issue #21326 .

You should be able to create tags with parents and children such as

- Sport
-- adults
-- children
- games
-- adults
-- children

Before this PR you could not as you would get a duplicate alias message error even though the tags have different parents and there is no reason that I can see to prevent this *_I may be very wrong as I dont use tags that often_*
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.2-dev labels Aug 23, 2022
simbus82 added a commit to simbus82/joomla-cms that referenced this pull request Aug 24, 2022
simbus82 added a commit to simbus82/joomla-cms that referenced this pull request Aug 24, 2022
simbus82 added a commit to simbus82/joomla-cms that referenced this pull request Aug 24, 2022
@ghost
Copy link

ghost commented Aug 27, 2022

I have tested this item ✅ successfully on 46f47fb


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

1 similar comment
@jwaisner
Copy link
Member

I have tested this item ✅ successfully on 46f47fb


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

@jwaisner
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 27, 2022
@fancyFranci
Copy link
Contributor

To be sure that there are no side effects we do not see, please move it to 4.3. Especially with your new tag feature this fix will have the chance to receive deeper tests.

@fancyFranci fancyFranci changed the base branch from 4.2-dev to 4.3-dev September 6, 2022 20:45
@brianteeman
Copy link
Contributor Author

please move it to 4.3.

You already did ;(

@obuisard obuisard added this to the Joomla! 4.3.0 milestone Sep 7, 2022
@obuisard obuisard merged commit 3563ef9 into joomla:4.3-dev Sep 7, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 7, 2022
@obuisard
Copy link
Contributor

obuisard commented Sep 7, 2022

Thank you Brian @brianteeman

@Hackwar
Copy link
Member

Hackwar commented Nov 3, 2022

Please be aware that this change means that routing is going to be broken for the duplicate aliases. The routing currently (and with my rewrite in #39114) simply takes the alias of the tag to identify the tag to load. I would propose to add a switch to either allow single level tags or to use nestable tags and change the routing in that case.

Comment on lines -84 to +87
if ($table->load(array('type_alias' => $this->type_alias)) && ($table->type_id != $this->type_id || $this->type_id == 0)) {
if (
$table->load(array('alias' => $this->alias, 'parent_id' => (int) $this->parent_id))
&& ($table->id != $this->id || $this->id == 0)
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@brianteeman Are you sure this change is correct?. The alias and parent_id fields do not exist in #__content_types table and I think using the lang key COM_TAGS_ERROR_UNIQUE_ALIAS does not belong here and was wrong even before this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm currently testing the Joomla 4.3 Beta 4 with HikaShop. During the installation process, we use this store function to add the tags capability on the products from HikaShop. We get the error:
Missing field in database: Joomla\CMS\Table\ContentType   alias.
https://i.imgur.com/yRGq5wK.png
Reverting this change does fix the problem. But I'm not familiar enough with the tag system to know if the code change is ok and we should change something in HikaShop (but what ?) or if another kind of modification should be necessary in Joomla 4.3 to fix the tag alias issue while preventing this error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes this PR is wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants