Misc: panelWizard.js Quality

[Sereza7]

Reference URL

Instead of the usual Jira issue, here is a link to the list of quality problems found on SonarQube:
https://sonarcloud.io/project/issues?fileUuids=AXnpASDXDDFOvAKXAP6t&languages=js&issueStatuses=OPEN%2CCONFIRMED&id=org.xwiki.platform%3Axwiki-platform

Changes

Description

• Removed the 22 quality "blocker" issues by declaring the javascript variables properly.
• Removed easy to remove high->low quality issues. 8 issues left (XWiki.widgets.Notification unused instantiations and Cognitive Complexity)

Clarifications

• It's far from being perfect but it reduces the number of quality issues found from 94 to 8, without introducing any new one.

Screenshots & Video

Test issue only.

Executed Tests

SonarQube validation (see screenshot above)
Built the change successfully with mvn clean install -f xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war -Pquality
Then passed the test suite successfully: mvn clean install -f xwiki-platform-core/xwiki-platform-panels/xwiki-platform-panels-test/xwiki-platform-panels-test-docker/

Tested manually on a local instance to make sure that changes did not influence functionality. At some point I tried changing the scope of variables and this broke toolTip.js (which relies on values of globally defined variables...) but the current solution works well.

Expected merging strategy

• Prefers squash: Yes
• Backport on branches:
  • None

[mflorea]

Looks good. I left a few suggestions, but can be merged as is.

[mflorea]

This may silence SonarQube, but it doesn't improve the code much. Sure, it makes it more explicit that those are global variables, but using global variables is not nice. So this needs to be further improved, later I guess.

[Sereza7]

Yeah I don't like this solution much, but removing all of those is pretty much just rewriting the whole script from zero and a good part of the other panelwizard scripts...

E.g. enabletip is set at a global level in toolTip.js and used without a warning right in the middle of panelWizard.js.

I don't remember all the issues I had but I tried limiting the scope of those variables and this created bugs that weren't solvable easily without large changes in the code architecture.

---

Side note: Even before changing the architecture, we probably want to remove/factorize some of the functions in here. Things like

xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/panelwizard/panelWizard.js

Lines 366 to 381 in 23b8f71

	function getBlocNameList(el) {
	var list = "";
	var nb = el.childNodes.length;
	for (var i = 0; i < nb; ++i) {
	var el2 = el.childNodes[i];
	if (isPanel(el2)) {
	if (!el2.isDragging) {
	if (list != "") {
	list += ",";
	}
	list += el2.fullname;
	}
	}
	}
	return list;
	}

and

xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/panelwizard/panelWizard.js

Lines 70 to 82 in 23b8f71

	function getBlocList(el) {
	var list = [];
	var nb = el.childNodes.length;
	for (var i = 0; i < nb; ++i) {
	var el2 = el.childNodes[i];
	if (isPanel(el2)) {
	if (!el2.isDragging) {
	list.push(el2);
	}
	}
	}
	return list;
	}

feel like they could almost be a one liner with today's javascript...

[Sereza7]

Thanks for the help @mflorea !
I reran the sonarQube tests, found a small problem added with your proposed changes and updated the PR.
SonarQube doesn't find any new issue with the file.
I tested it on my local instance and it still works as expected.

This PR is ready for further review or a merge :)