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

EZP-20027: Provide possibility to filter children locations based on visibility #140

Closed
wants to merge 1 commit into from

Conversation

andrerom
Copy link
Contributor

No description provided.

@dpobel
Copy link
Contributor

dpobel commented Nov 19, 2012

Shouldn't the default value depend on the setting of the siteaccess ie on a setting passed to the repository like it is done for the prioritized languages list ?

@lolautruche
Copy link
Contributor

Agreed with @dpobel. There should be an injected setting. Moreover, why is the filter deactivated by default ?

@andrerom
Copy link
Contributor Author

Shouldn't the default value depend on the settings of the siteaccess ie on a setting passed to the repository like it is done for the prioritized languages list ?

If we inject it, then that means API is in charge of dealing with hidden content, always. This would mean a lot of changes to the API as this was not intended from the inception.

So this PR is about making it possible for user code to deal with visibility, and to simplify things, provide a optional parameter to filter them out in this function since it does not provide access to specify query.

@dpobel
Copy link
Contributor

dpobel commented Nov 19, 2012

that's a big loss for developers :-( that also means we have to update the code of the DemoBundle to force the fact we only want to load visible content otherwise the "hide/unhide" does not work out of the box...

@lolautruche
Copy link
Contributor

@dpobel Same feeling here.

I think we can inject it and still be able to override by passing the value explicitly.

@andrerom
Copy link
Contributor Author

I think we can inject it and still be able to override by passing the value explicitly.

If we pass setting to PAPI, it implies the logic is inside the API, while now it is outside the API, big difference.
Making PAPI fully responsible for dealing with hidden locations is not realistic for 5.0, which is why it is kept outside for now.

We can definitely discuss how we can handle this differently for 5.1 later, but right now this is about what we can / should do for 5.0.

@andrerom
Copy link
Contributor Author

Given the DemoBundle issue is mostly fixed now*, we can maybe skip this PR and instead aim to find a better solution in 5.1 if needed.
Close or pull?

* reaming issue there is that it will show hidden menu locations if menu item has more locations and some of them are accessible.

@andrerom
Copy link
Contributor Author

Closing PR, no feedback.

@andrerom andrerom closed this Nov 20, 2012
ViniTou pushed a commit that referenced this pull request Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants