Skip to content

Conversation

@Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Nov 20, 2025

Jira URL

https://jira.xwiki.org/browse/XWIKI-20437

Changes

Description

  • Split the code of the findCustomSectionsToConfigure macro to make it a bit more readable/reusable.
  • Made a second lookup to get all the categories in all the cases, so that we always get all the order information.
  • The structures used are complexe for velocity, so deep copying was needed at some point.
  • The $query initialization was unnecessary on modern XS.
  • Added comments to explain a bit more the process.

Clarifications

  • TLDR: To get the right order, we need to retrieve the ordering information scattered across multiple objects that are all throughout the wiki.
  • Despite the amount of changes in this PR, there is barely any DOM change.
  • It was a pain to debug. AFAIK we don't have ways to make deep copies in Velocity so I had to have a few lines of hard-coded deep copy.
  • It's a patch on this legacy system. Ideally we'd replace and deprecate it whole, but I hope the comments and structure added in here help make this less of a pain to replace without a hitch in the future. E.g. use a value property on categories (the standard way, the one already used for sections in categories) instead of a linkedlist simili-structure to compute a score.
  • I created a few utility macros (non API) to avoid getting a 200 lines macro ^^'
  • Technically this makes local administration pages slower. I did not measure the impact on performance though.

Screenshots & Video

After the PR was applied on my local instance, it looked like:
Screenshot From 2025-11-19 18-41-24
Screenshot From 2025-11-19 18-41-18

Executed Tests

Manual tests (see screenshots above).
Docker tests for admin are really slow, so far though they did not fail (WIP).

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • None. This is too dangerous for LTS IMO.

* Split the code of the findCustomSectionsToConfigure macro to make it a bit more readable/reusable.
* Made a second lookup to get all the categories in all the cases, so that we always get all the order information.
* The structures used are complexe for velocity, so deep copying was needed at some point.
* The $query initialization was unnecessary on modern XS.
* Added comments to explain a bit more the process.
Comment on lines 181 to 188
#foreach ($entry in $adminMenu)
#set ($entryCopy = {})
#set ($discard = $entryCopy.putAll($entry))
#set ($entryChildrenCopy = [])
#set ($discard = $entryChildrenCopy.addAll($entry.children))
#set ($discard = $entryCopy.put('children',$entryChildrenCopy))
#set ($discard = $outputList.add($entryCopy))
#end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#foreach ($entry in $adminMenu)
#set ($entryCopy = {})
#set ($discard = $entryCopy.putAll($entry))
#set ($entryChildrenCopy = [])
#set ($discard = $entryChildrenCopy.addAll($entry.children))
#set ($discard = $entryCopy.put('children',$entryChildrenCopy))
#set ($discard = $outputList.add($entryCopy))
#end
#set ($discard = $outputList.addAll($jsontool.fromString($jsontool.serialize($adminMenu))))

This should work because we're using only Map, List and simple types (string, number, boolean) to build the administration menu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! That looks way better than what I had :)
Addressed in 24ee46e 👍

@Sereza7 Sereza7 requested a review from mflorea November 21, 2025 11:03
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