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

Feat(user): add default entity value = Full structure #17923

Open
wants to merge 5 commits into
base: 10.0/bugfixes
Choose a base branch
from

Conversation

Rom1-B
Copy link
Contributor

@Rom1-B Rom1-B commented Sep 24, 2024

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

  • It fixes -
  • Here is a brief description of what this PR does
    Add Full structure value in user settings:

image

@Rom1-B Rom1-B marked this pull request as draft September 30, 2024 09:32
@Rom1-B Rom1-B marked this pull request as ready for review October 1, 2024 07:41
'entity' => $entities
'entity' => $entities,
'toadd' => [
0 => __('Full structure'),
Copy link
Contributor

Choose a reason for hiding this comment

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

The 0 id is already used by the root entity.

This mean with this change I can't pick the root entity as my default anymore.
Is it on purpose ?

Copy link
Contributor

Choose a reason for hiding this comment

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

It always has been this way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have access to the root entity, the behavior is the same between "full structure" and "tree structure." The difference only occurs when you do not have access to the root entity.

We cannot use -1 without modifying the database schema, and I'm not sure it's worth it?

Copy link
Contributor

@AdrienClairembault AdrienClairembault Oct 2, 2024

Choose a reason for hiding this comment

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

It always has been this way

What do you mean ?
Before this change, I can pick the root entity.
After this change, I can't.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have access to the root entity, the behavior is the same between "full structure" and "tree structure." The difference only occurs when you do not have access to the root entity.

Maybe the root entity option should be removed then ?

Because right now if I click on the root entity it selects full structure instead.
It is likely that we will get support requests from people that will think it is a bug :/

@AdrienClairembault
Copy link
Contributor

Also, it seems a bit risky to add a new option like this directly in the bugfixes branch, no ?

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