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

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

alikon
Copy link
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

ghost commented Apr 6, 2019

@artur-stepien please test.

@Bakual
Copy link
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
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
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
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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants