Skip to content

[com_fields] - don't traverse the tree#24515

Closed
alikon wants to merge 1 commit intojoomla:stagingfrom
alikon:patch-116
Closed

[com_fields] - don't traverse the tree#24515
alikon wants to merge 1 commit intojoomla:stagingfrom
alikon:patch-116

Conversation

@alikon
Copy link
Copy Markdown
Contributor

@alikon alikon commented Apr 6, 2019

Pull Request for Issue #24361 .

Summary of Changes

we don't need to traverse the tree

Testing Instructions

make a category tree like this
A
-> B
-> -> C
-> -> -> D

create an article field and assign to previous categories (a,b,c,d)
create an article in the D category

visit that artilce you'll see 4 times the same field

see #24361 for more details

Expected result

to see the field once

Actual result

Note

same happens on administrator side if you filter for category before patch
Screenshot from 2019-04-06 09-22-08

@ghost ghost added the J3 Issue label Apr 6, 2019
@ghost
Copy link
Copy Markdown

ghost commented Apr 6, 2019

@artur-stepien please test.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Apr 6, 2019

But what happens if you only assign the field to category A? It should imho be available in all subcategories as well (at least code indicates that is intended behavior).
If you change that, it is a change in expected behavior which would be unacceptable in J3.

The thing is, you shouldn't need to assign the field to category B, C and D as they inherit the field from A. So it's kind of a user failure. Neverthlesss it shouldn't show them multiple times, thus a DISTINCT in the query would be the correct solution.

@alikon
Copy link
Copy Markdown
Contributor Author

alikon commented Apr 6, 2019

a field should be available in all subcategories .. at least code indicates that is intended behavior

ummm, not so much expected behaviour for me , at least
but i can be wrong

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Apr 6, 2019

Afaik it was like this since the feature was introduced.
@laoneo Correct me if i'm wrong.

@alikon
Copy link
Copy Markdown
Contributor Author

alikon commented Apr 6, 2019

so let's go with DISTINCT #24516 😄

@alikon alikon closed this Apr 6, 2019
@alikon alikon deleted the patch-116 branch April 6, 2019 07:52
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Apr 8, 2019

@Bakual yes you are right. Both sides have their advantages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants