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

XWIKI-15776: The groups displayer store values with a coma at the end #1800

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

surli
Copy link
Member

@surli surli commented Mar 18, 2022

  • Refactor ListClass#fromList to take into account multiselect
  • Provide a new ListClass#fromList API with a parameter to filter out
    empty values
  • Provide a new ListClass#getListFromString API with a parameter to
    filter out empty values
  • Use those new APIS in UsersClass/GroupsClass/LevelsClass
  • Add some new tests

  * Refactor ListClass#fromList to take into account multiselect
  * Provide a new ListClass#fromList API with a parameter to filter out
    empty values
  * Provide a new ListClass#getListFromString API with a parameter to
    filter out empty values
  * Use those new APIS in UsersClass/GroupsClass/LevelsClass
  * Add some new tests
@surli surli requested review from mflorea and tmortagne March 18, 2022 07:23
  * Makes explicit the usage of a comma as separator for
    GroupsClass/UsersCLass/LevelsClass
  * Add a fixme comment on ListClass#newProperty since this one looks
    buggy
  * Ensure that UsersClass/GroupsClass/LevelsClass does not allow to
    override the separator
@surli surli changed the title XWIKI-15576: The groups displayer store values with a coma at the end XWIKI-15776: The groups displayer store values with a coma at the end Mar 18, 2022
* @param filterEmptyValues if {@code true} filter out the empty values from the list.
* @since 14.2RC1
*/
protected void fromList(BaseProperty<?> property, List<String> list, boolean filterEmptyValues)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this new filterEmptyValues concept should be an actual configuration of the property class (that happen to be always true for group/user classes) instead of just a parameter in a protected method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be a global improvment to make indeed. Feels a bit too much for this PR though.

  * Override #fromList in GroupsClass/UsersClass/LevelsClass to be sure
    to filter out the empty values there
  * Provide unit tests for the changes
@surli surli merged commit 2dc9f52 into master Mar 18, 2022
@surli surli deleted the listclass-fromlist-multiselect branch March 18, 2022 13:42
surli added a commit that referenced this pull request Mar 18, 2022
…#1800)

  * Refactor ListClass#fromList to take into account multiselect
  * Provide a new ListClass#fromList API with a parameter to filter out
    empty values
  * Provide a new ListClass#getListFromString API with a parameter to
    filter out empty values
  * Use those new APIS in UsersClass/GroupsClass/LevelsClass
  * Add some new tests
  * Makes explicit the usage of a comma as separator for
    GroupsClass/UsersCLass/LevelsClass
  * Add a fixme comment on ListClass#newProperty since this one looks
    buggy
  * Ensure that UsersClass/GroupsClass/LevelsClass does not allow to
    override the separator
  * Override #fromList in GroupsClass/UsersClass/LevelsClass to be sure
    to filter out the empty values there
  * Provide unit tests for the changes

(cherry picked from commit 2dc9f52)
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.

2 participants