From 176e373d6c7ce6e78ebf7dea58b9a04a96faafa0 Mon Sep 17 00:00:00 2001
From: Molkobain <lajarige.guillaume@free.fr>
Date: Tue, 7 Mar 2023 22:03:20 +0100
Subject: [PATCH] =?UTF-8?q?N=C2=B05530=20-=20Fix=20list=20of=20impacted=20?=
 =?UTF-8?q?elements=20(Impact=20Analysis)=20not=20display=20correctly=20du?=
 =?UTF-8?q?e=20to=20mixup=20in=20async=20JS=20files=20loading?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 js/dataTables.settings.js                     |   4 +-
 .../backoffice/ajaxpage/layout.html.twig      | 134 ++++++++++++------
 .../pages/backoffice/webpage/layout.html.twig |  32 ++++-
 3 files changed, 116 insertions(+), 54 deletions(-)

diff --git a/js/dataTables.settings.js b/js/dataTables.settings.js
index 52754a972..d2e7f5253 100644
--- a/js/dataTables.settings.js
+++ b/js/dataTables.settings.js
@@ -89,11 +89,11 @@ $(function () {
 					aOptions = $.extend(aOptions, JSON.parse(data));
 					if (aOptions.js_files) {
 						$.each(aOptions.js_files, function (i, item) {
-							if ($.inArray(item, aListJsFiles) === -1)
+							if ($.inArray(item, aLoadedJsFilesRegister) === -1)
 							{
 								sFileUrl = CombodoGlobalToolbox.AddParameterToUrl(item, aOptions.js_files_param, aOptions.js_files_value);
 								$.ajax({url:sFileUrl, dataType: 'script', cache: true });
-								aListJsFiles.push(item);
+								aLoadedJsFilesRegister.push(item);
 							}
 						});
 					}
diff --git a/templates/pages/backoffice/ajaxpage/layout.html.twig b/templates/pages/backoffice/ajaxpage/layout.html.twig
index ecd001a2a..74dbc7ba1 100644
--- a/templates/pages/backoffice/ajaxpage/layout.html.twig
+++ b/templates/pages/backoffice/ajaxpage/layout.html.twig
@@ -35,55 +35,99 @@
         {% set sId = oLayout.GetId() | sanitize(constant('utils::ENUM_SANITIZATION_FILTER_VARIABLE_NAME')) %}
         {% block iboPageJsFiles %}
             <script type="text/javascript">
-            window['{{ sPromiseId }}'] = new Promise(function (resolve, reject) {
-                let fInlineOnDomReadyScript{{ sId }} = function () {
-                    {% for sJsInlineOnDomReady in aPage.aJsInlineOnDomReady %}
-                    {{ sJsInlineOnDomReady|raw }}
+                window['{{ sPromiseId }}'] = new Promise(function (resolve, reject) {
+                    let fInlineOnDomReadyScript{{ sId }} = function () {
+                        {% for sJsInlineOnDomReady in aPage.aJsInlineOnDomReady %}
+                            {{ sJsInlineOnDomReady|raw }}
+                        {% endfor %}
+                        resolve();
+                    }
+
+                    /**
+                     * @type {Array} aJsFilesToLoad Files required by the current \AjaxPage
+                     *
+                     * For each file:
+                     * - "id": Used as an identifier to check if file is already being handled
+                     * - "url" is the URL that will be use for loading. It should include any relevant query args, including the cache buster
+                     *
+                     * ```
+                     * [
+                     *  {"id": "https://itop/js/foo.js", "url": "https://itop/js/foo.js?cache_buster=123},
+                     *  {"id": "https://itop/js/bar.js", "url": "https://itop/js/bar.js?a=b&cache_buster=123"},
+                     *  ...
+                     * ]
+                     * ```
+                     */
+                    let aJsFilesToLoad = [];
+                    /**
+                     * @type {Array} aJsFilesToLoadByOtherRequests Files required by the current \AjaxPage but that are already being handled by another request (done or ongoing)
+                     */
+                    let aJsFilesToLoadByOtherRequests = [];
+
+                    {% for sJsFile in aPage.aJsFiles %}
+                        aJsFilesToLoad.push({
+                            "id": "{{ sJsFile|raw }}",
+                            "url": "{{ sJsFile|add_itop_version|raw }}"
+                        });
+
+                        // If file is already present in the register (see it declaration in \WebPage TWIG template), let its original requester load it
+                        if (aLoadedJsFilesRegister.has("{{ sJsFile|raw }}") === true) {
+                            aJsFilesToLoadByOtherRequests.push("{{ sJsFile|raw }}");
+                        }
+                        // Otherwise add it to register and initialize corresponding promise
+                        else {
+                            aLoadedJsFilesRegister.set("{{ sJsFile|raw }}", new Promise(function(fJsFileResolve) {
+                                aLoadedJsFilesResolveCallbacks.set("{{ sJsFile|raw }}", fJsFileResolve);
+                            }));
+                        }
                     {% endfor %}
-                    resolve();
-                }
 
-                let aFilesToLoad{{ sId }} = [];
+                    let iCurrentIdx = 0;
+                    let iFilesToLoadCount = aJsFilesToLoad.length;
+                    if (iFilesToLoadCount > 0)
+                    {
+                        let fLoadScript{{ sId }} = function () {
+                            let sCurrentFileId = aJsFilesToLoad[iCurrentIdx]["id"];
+                            let sCurrentFileUrl = aJsFilesToLoad[iCurrentIdx]["url"];
 
-                {% for sJsFile in aPage.aJsFiles %}
-                if ($.inArray('{{ sJsFile|raw }}', aListJsFiles) === -1)
-                {
-                    aFilesToLoad{{ sId }}.push('{{ sJsFile|add_itop_version|raw }}');
-                    aListJsFiles.push("{{ sJsFile|raw }}");
-                }
-                {% endfor %}
+                            /** @type {Promise} oPromise Promise to use once file is loaded */
+                            let oPromise = null;
+                            // If file is handled by another request, retrieve the existing promise
+                            if ($.inArray(sCurrentFileId, aJsFilesToLoadByOtherRequests) !== -1) {
+                                oPromise = aLoadedJsFilesRegister.get(sCurrentFileId)
+                            }
+                            // Otherwise create its own promise to load it
+                            else {
+                                oPromise = $.when(
+                                        $.ajax({
+                                            url: sCurrentFileUrl,
+                                            dataType: 'script',
+                                            cache: true
+                                        }),
+                                        aLoadedJsFilesResolveCallbacks.get(sCurrentFileId)()
+                                );
+                            }
 
-                let iCurrentIdx{{ sId }} = 0;
-                let iFilesToLoadCount{{ sId }} = aFilesToLoad{{ sId }}.length;
-                if (iFilesToLoadCount{{ sId }} > 0)
-                {
-                    let fLoadScript{{ sId }} = function () {
-                        $.when(
-                                $.ajax({
-                                    url: aFilesToLoad{{ sId }}[iCurrentIdx{{ sId }}],
-                                    dataType: 'script',
-                                    cache: true
-                                })
-                                )
-                                .then(function () {
-                                    iCurrentIdx{{ sId }}++;
-                                    if (iCurrentIdx{{ sId }} !== iFilesToLoadCount{{ sId }})
-                                    {
-                                        fLoadScript{{ sId }}();
-                                    }
-                                    else
-                                    {
-                                        fInlineOnDomReadyScript{{ sId }}();
-                                    }
-                                });
-                    };
-                    fLoadScript{{ sId }}();
-                }
-                else
-                {
-                    fInlineOnDomReadyScript{{ sId }}();
-                }
-            });
+                            // Only once file is loaded (by the request or another), proceed to next step
+                            oPromise.then(function () {
+                                iCurrentIdx++;
+                                if (iCurrentIdx !== iFilesToLoadCount)
+                                {
+                                    fLoadScript{{ sId }}();
+                                }
+                                else
+                                {
+                                    fInlineOnDomReadyScript{{ sId }}();
+                                }
+                            });
+                        };
+                        fLoadScript{{ sId }}();
+                    }
+                    else
+                    {
+                        fInlineOnDomReadyScript{{ sId }}();
+                    }
+                });
             </script>
         {% endblock %}
     {% else %}
diff --git a/templates/pages/backoffice/webpage/layout.html.twig b/templates/pages/backoffice/webpage/layout.html.twig
index 457075bf1..2dd8483b7 100644
--- a/templates/pages/backoffice/webpage/layout.html.twig
+++ b/templates/pages/backoffice/webpage/layout.html.twig
@@ -64,14 +64,32 @@
         {{ render_block(oBlock, {aPage: aPage}) }}
     {% endfor %}
 {% endblock %}
-{% if aPage.aJsFiles is not empty %}
-    <script type="text/javascript">
-    var aListJsFiles = [];
+
+<script type="text/javascript">
+    {# JS files can either be loaded initially or requested by an XHR response                                          #}
+    {# - For the initial page, all files are loaded before running inline snippets                                      #}
+    {# - For XHR responses, we need to ensure that all required files are fully loaded before running inline snippets   #}
+
+    /**
+     * @var {Map} aLoadedJsFilesRegister
+     * Register of all JS files loaded in this page, including the Promise corresponding to the loading state of each file.
+     * A JS file MUST NOT be loaded more than once as it could compromise settings / plugins loaded after the first time.
+     */
+    const aLoadedJsFilesRegister = new Map();
+    /**
+     * @var {Map} aLoadedJsFilesResolveCallbacks
+     * Resolve callbacks of JS files registered in aLoadedJsFilesRegister. Used -mostly in \AjaxPage- to know when a file is done loading so the depending snippets can run
+     */
+    const aLoadedJsFilesResolveCallbacks = new Map();
     {% for sJsFile in aPage.aJsFiles %}
-            aListJsFiles.push("{{ sJsFile|raw }}");
-        {% endfor %}
-    </script>
-{% endif %}
+        aLoadedJsFilesRegister.set("{{ sJsFile|raw }}", new Promise(function(resolve) {
+            aLoadedJsFilesResolveCallbacks.set("{{ sJsFile|raw }}", resolve);
+            // Resolve promise right away as these files are loaded immediately before any inline JS is executed
+            aLoadedJsFilesResolveCallbacks.get("{{ sJsFile|raw }}")();
+        }));
+    {% endfor %}
+</script>
+
 {% block iboPageJsFiles %}
     {% for sJsFile in aPage.aJsFiles %}
         <script type="text/javascript" src="{{ sJsFile|add_itop_version|raw }}"></script>