Can you supply some ample code to make it easier for people to test this
please

On 17 April 2015 at 09:11, Oleksandr Panasenko notifications@github.com
wrote:

In my project I needed to access attribs field from #__content which is
stored as core_params in #__ucm_content. This commit will allow to access
this field when outputting tags related articles. It's especially useful if

you have custom parameters associated with your article.

You can view, comment on, or merge this pull request online at:

#6792
Commit Summary

• Including core_params field in SELECT

File Changes

• M libraries/cms/helper/tags.php https://github.com/joomla/joomla-cms/pull/6792/files#diff-0 (1)

Patch Links:

• https://github.com/joomla/joomla-cms/pull/6792.patch
• https://github.com/joomla/joomla-cms/pull/6792.diff

—
Reply to this email directly or view it on GitHub
#6792.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

So, basically there's only one line added, it has effect on gettagitemsquery() method defined at /libraries/cms/helper/tags.php -> line 467 and referenced only in one place: Tag model /components/com_tags/models/tag.php -> line 147. With this change the contents of the object returned in response to call getItems() of this model will be added with one more field core_params. Thus in /components/com_tags/views/tag/tmpl/default.php elements under $this->items would have this field which isn't available now.

Ouch, that query looks awful already...

Looking at the other selects I don't think that will work. The whole query is a group one, the #__ucm_content is a joined table and may or may not return multiple matching rows. The other selects all use MAX() to make sure only one row is returned.

Yes, you're right on that query, but $query->select() will just add one more parameter to SELECT, so I don't see what bad could happen in this case. It works for me: one more field as expected.

I don't know if it's needed or not. But all other cases where something is fetched from the same table (c.) in that query has it wrapped in MAX().
Most likely one of the joins in that query may return more than one row in some cases, and then you get duplicate results if you haven't aggregated the fields.

Just checked, it works on multiple elements too. I have 3 articles matching a tag: all have core_params field.

I think it's needed because we should have access to all the parameters of an article whether it's returned. In other places: content modules or com_content views we have access to attribs field. I think in case of tags this shouldn't be different.

I've tried to brace core_params with MAX, it still works. Do you think it'll work in this case?

I'd say either all of the selects from c. need to be wrapped in MAX() or none of them. But having one not wrapped and the other are wrapped sure is wrong.
I honestly don't know if it's needed or not, but it should be consistent for sure.

Ok, I'll add another commit with the MAX, and you'll see merge it or not. Btw, is it possible to edit this pull request to reflect new changes with our discussion or I need to create a new one?

The PR updates automatically with your new commit

If you edit your first post and add detailed testing instructions, then it will be tested faster and could be merged.

@test SUCCESS
Tested with staging package of today.

Some test instructions because @oldrpan didn't provide them:

• Create menu item of type Tagged Items and select tag(s). Save menu item.
• Open several Articles, Categories, Newsfeeds, Contacts, Contact Categories, whatever and add this tag to these items.
• Set some individual options/parameters if you want to. Add image in categories and so on.
• Save item(s).
• Now a small core hack in

/components/com_tags/views/tag/tmpl/default_items.php

Directly after line 63

<?php foreach ($items as $i => $item) : ?>

add a new line

<?php echo 'Core_Params:<br /><pre>' . $item->core_params . '</pre><br />'; ?>

• Open your new menu item in frontend (Protostar).
• Properties core_params are all empty.

• Apply patch, reload page:

• These params are also important e.g. to be able to display category images that are saved in field core_params.

Tested like described and successfull :)

Thank you @oldrpan! Merged.