diff --git a/pyproject.toml b/pyproject.toml index 3a44169..a9b2d3a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,10 +39,10 @@ dynamic = ["version"] [project.optional-dependencies] dev = [ - "tutor[dev]>=20.0.0,<21.0.0", + "tutor[dev]>=20.0.0,<21.0.0", "types-aiofiles", "types-Markdown", - "pylint", + "pylint", "black", ] diff --git a/tutordeck/server/app.py b/tutordeck/server/app.py index 86758d7..f7d23af 100644 --- a/tutordeck/server/app.py +++ b/tutordeck/server/app.py @@ -152,14 +152,16 @@ async def configuration_update() -> BaseResponse: """ await process_config_update_request() - # Handle non-ajax call - next_url = request.args.get("next", "") - if next_url: - return redirect(next_url) + response: BaseResponse + if next_url := request.args.get("next", ""): + # Handle non-ajax call + response = redirect(next_url) + else: + # Handle ajax call + response = Response("", status=200, content_type="text/html") + response.headers["HX-Redirect"] = url_for("configuration") - # Handle ajax call - response = Response("", status=200, content_type="text/html") - response.headers["HX-Redirect"] = url_for("configuration") + notify_run_sequential(response) return response @@ -260,9 +262,6 @@ async def plugin(name: str) -> Response: if not index_entry and name not in g.installed_plugins: return Response("Plugin not found", status=404) - # TODO this seq_command_executed argument is confusing and causing issues, for - # instance with the "unset" button. We need to get rid of it. - seq_command_executed = request.args.get("seq_command_executed") description = markdown(index_entry.description) if index_entry else "" rendered_template = await render_template( "plugin.html", @@ -273,18 +272,15 @@ async def plugin(name: str) -> Response: tutorclient.Client.get_plugin_author(index_entry) if index_entry else "" ), plugin_description=description, - seq_command_executed=seq_command_executed, plugin_config_unique=tutorclient.Client.plugin_config_unique(name), plugin_config_defaults=tutorclient.Client.plugin_config_defaults(name), user_config=tutorclient.Project.get_user_config(), ) # Redirect to plugin page - # TODO this is useful only after a POST to plugin//update. I don't think these two things should be handled in the same place. response = Response(rendered_template, status=200, content_type="text/html") - response.headers["HX-Redirect"] = url_for( - "plugin", name=name, seq_command_executed=seq_command_executed - ) + + response.headers["HX-Redirect"] = url_for("plugin", name=name) return response @@ -305,16 +301,9 @@ async def plugin_toggle(name: str) -> Response: response = t.cast( Response, - await make_response( - redirect( - url_for( - "plugin", - name=name, - seq_command_executed=True, - ) - ) - ), + await make_response(redirect(url_for("plugin", name=name))), ) + notify_run_sequential(response) if enable_plugin: update_plugins_requiring_launch(response, add=name) else: @@ -362,17 +351,10 @@ async def plugin_config_update(name: str) -> Response: await process_config_update_request() response = t.cast( Response, - await make_response( - redirect( - url_for( - "plugin", - name=name, - seq_command_executed=True, - ) - ) - ), + await make_response(redirect(url_for("plugin", name=name))), ) update_plugins_requiring_launch(response, add=name) + notify_run_sequential(response) return response @@ -496,6 +478,13 @@ async def command() -> BaseResponse: return redirect(url_for("advanced")) +def notify_run_sequential(response: BaseResponse) -> None: + """ + Notify the frontend that a sequential command was run. + """ + set_cookie(response, constants.COMMAND_EXECUTED_COOKIE_NAME, "1") + + def update_plugins_requiring_launch( response: Response, add: t.Optional[str] = None, remove: t.Optional[str] = None ) -> None: @@ -527,8 +516,15 @@ def update_plugins_requiring_launch( names.discard(remove) # Update the response - response.set_cookie( + set_cookie( + response, constants.PLUGINS_REQUIRE_LAUNCH_COOKIE_NAME, separator.join(sorted(names)), - max_age=60 * 60 * 24 * 30, # 1 month ) + + +def set_cookie(response: BaseResponse, name: str, value: str) -> None: + """ + Set a cookie with a consistent expiry time. + """ + response.set_cookie(name, value, max_age=60 * 60 * 24 * 30) # 1 month diff --git a/tutordeck/server/constants.py b/tutordeck/server/constants.py index 3960906..dde7af4 100644 --- a/tutordeck/server/constants.py +++ b/tutordeck/server/constants.py @@ -1,3 +1,4 @@ SHORT_SLEEP_SECONDS = 0.1 PLUGINS_REQUIRE_LAUNCH_COOKIE_NAME = "plugins-require-launch" +COMMAND_EXECUTED_COOKIE_NAME = "command-executed" ITEMS_PER_PAGE = 100 diff --git a/tutordeck/server/static/js/deck.js b/tutordeck/server/static/js/deck.js index 8b0dd17..515018e 100644 --- a/tutordeck/server/static/js/deck.js +++ b/tutordeck/server/static/js/deck.js @@ -1,9 +1,27 @@ +// Cookie utilities +// We can't use the cookieStore because we might want to access tutor deck in http mode, +// where it is not available. +function getCookie(name) { + let nameEQ = name + "="; + return ( + document.cookie + .split(";") + .map((cookie) => cookie.trim()) + .find((cookie) => cookie.startsWith(nameEQ)) + ?.slice(nameEQ.length) || null + ); +} +function eraseCookie(name) { + document.cookie = + name + "=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;"; +} + // Handle plugins requiring launch based on the values in the corresponding cookie const pluginsRequireLaunchCookieName = "plugins-require-launch"; -async function displayPluginsRequireLaunchWarning() { - const cookie = await cookieStore.get(pluginsRequireLaunchCookieName); - if (cookie && cookie.value) { - const cookieValue = cookie.value.slice(1, -1); // remove quotes +function displayPluginsRequireLaunchWarning() { + const cookie = getCookie(pluginsRequireLaunchCookieName); + if (cookie) { + const cookieValue = cookie.slice(1, -1); // remove quotes cookieValue.split('+').map(s => s.trim()).forEach(plugin => { document.querySelectorAll(`[data-plugin="${plugin}"] .warning-launch-required`).forEach(element => { element.classList.add("visible"); @@ -41,7 +59,7 @@ function showLaunchSuccessfulToast() { // TODO this is very brittle because it relies on static variables and string values. if (toast) { if (toastTitle === "Launch platform was successfully executed") { - cookieStore.delete(pluginsRequireLaunchCookieName); + eraseCookie(pluginsRequireLaunchCookieName); } toast.style.display = "flex"; setTimeout(() => { @@ -59,35 +77,33 @@ function hideToast() { } } +const launchDescription = "To apply the changes, run Launch Platform. This will update your platform and may take a few minutes to complete."; const TOAST_CONFIGS = { - "tutor plugins enable": { - title: "Your plugin was successfully enabled", - description: - "To apply the changes, run Launch Platform. This will update your platform and may take a few minutes to complete.", - showFooter: true, - }, - "tutor plugins upgrade": { - title: "Your plugin was successfully updated", - description: - "To apply the changes, run Launch Platform. This will update your platform and may take a few minutes to complete.", - showFooter: true, - }, - "tutor plugins install": { - title: "Plugin Installed Successfully", - description: "Enable it now to start using its features", - showFooter: false, - }, "tutor config save": { - title: "You have successfully modified parameters", - description: - "To apply the changes, run Launch Platform. This will update your platform and may take a few minutes to complete.", + title: "Configuration parameters were updated", + description: launchDescription, showFooter: true, }, "tutor local launch": { - title: "Launch platform was successfully executed", + title: "Platform was launched", description: "", showFooter: false, }, + "tutor plugins install": { + title: "Plugin was installed", + description: "Enable it now to start using its features", + showFooter: false, + }, + "tutor plugins enable": { + title: "Plugin was enabled", + description: launchDescription, + showFooter: true, + }, + "tutor plugins upgrade": { + title: "Plugin was updated", + description: launchDescription, + showFooter: true, + }, }; let toastTitle = document.getElementById("toast-title"); let toastDescription = document.getElementById("toast-description"); @@ -105,38 +121,7 @@ function setToastContent(cmd) { } // Each page defines its own relevant commands, we use them to check -// if the currently running commands belong the currently opened page or not -let relevantCommands = []; -let onDeveloperPage = false; -function onRelevantPage(command) { - if (onDeveloperPage) { - // Developer page is relevant to all commands - return true; - } - return relevantCommands.some((prefix) => command.startsWith(prefix)); -} +// if the currently running commands belong the currently opened page or not. +// A "*" relevant command matches all possible commands. +let tutorCommandsToWatch = []; -function activateInputs() { - document.querySelectorAll("button").forEach((button) => { - button.disabled = false; - }); - document.querySelectorAll("input").forEach((input) => { - input.disabled = false; - }); - document.querySelectorAll(".form-switch").forEach((formSwitch) => { - formSwitch.style.opacity = 1; - }); - document.getElementById("warning-command-running").style.display = "none"; -} -function deactivateInputs() { - document.querySelectorAll("button").forEach((button) => { - button.disabled = true; - }); - document.querySelectorAll("input").forEach((input) => { - input.disabled = true; - }); - document.querySelectorAll(".form-switch").forEach((formSwitch) => { - formSwitch.style.opacity = 0.5; - }); - document.getElementById("warning-command-running").style.display = "flex"; -} diff --git a/tutordeck/server/static/js/logs.js b/tutordeck/server/static/js/logs.js index bd1a5b4..9ed1d18 100644 --- a/tutordeck/server/static/js/logs.js +++ b/tutordeck/server/static/js/logs.js @@ -7,53 +7,81 @@ // Each page that uses logs defines its own command execution/cancellation toggle functions with the same signature // We can safely call these functions and their functionality will be handeled by the page specific js +// Scrolling management let shouldAutoScroll = true; let isScrollingProgrammatically = false; // When user manually scrolls, update behaviour -logsElement.addEventListener("scroll", function () { - if (!isScrollingProgrammatically) { - shouldAutoScroll = false; +logsElement.addEventListener( + "scroll", + function () { + if (!isScrollingProgrammatically) { + shouldAutoScroll = false; + } } -}); -let executedNewCommand = false; +); +logsElement.addEventListener( + "wheel", + function () { + shouldAutoScroll = false; + }, + { passive: true } +); +logsElement.addEventListener( + "touchstart", + function () { + shouldAutoScroll = false; + }, + { passive: true } +); + +let commandExecuted = false; +function checkAndClearCommandExecuted() { + const commandExecutedCookieName = "command-executed"; + if (getCookie(commandExecutedCookieName)) { + eraseCookie(commandExecutedCookieName); + commandExecuted = true; + } +}; +checkAndClearCommandExecuted(); + +let threadWasAlive = false; htmx.on("htmx:sseBeforeMessage", function (evt) { // Don't swap content, we want to append evt.preventDefault(); - const data = JSON.parse(evt.detail.data); - const command = data.command; + evt.detail.elt.appendChild(document.createTextNode(data.stdout)); // This means a parallel command is executing if (data.thread_alive) { + threadWasAlive = true; // Check if we are on the same page on which the actual command was executed - // Each page defines its relevant commands which are sent to `onRelevantPage` function to check if we are on the relevant page - if (onRelevantPage(command)) { + // Each page defines its relevant commands that are monitored and that will trigger a display of the log window. + let shouldDisplayLogs = tutorCommandsToWatch.some( + (prefix) => prefix === "*" || data.command.startsWith(prefix) + ); + if(shouldDisplayLogs) { ShowCancelCommandButton(); logsElement.style.display = "block"; } else { // If we are not on relevant page we don't show the cancel button and disable all inputs - deactivateInputs(); + onCommandRunning(); } - executedNewCommand = true; } - const parallelCommandCompleted = executedNewCommand && !data.thread_alive; - - // TODO this is a very brittle way of checking that we are on a plugin page... Let's not use static variables. Same for sequentialCommandExecuted. + // A parallel command was running, and now it's completed + const parallelCommandCompleted = threadWasAlive && !data.thread_alive; + // TODO this is a very brittle way of checking that we are on a plugin page... Let's not use static variables. const onPluginPage = typeof pluginName !== "undefined"; // Note that sequential commands are only executed on the plugins page // Refreshing the page will run this block again // Because there is no way to determine if its a newly executed sequential command or an old one - if ( - parallelCommandCompleted || - (onPluginPage && sequentialCommandExecuted) - ) { - activateInputs(); + if (parallelCommandCompleted || commandExecuted) { + onCommandComplete(); // There are certain commands for which we do not show the toast message // Only show the toast if it was set in the `setToastContent` function and if the command ran successfully // TODO this is brittle because it relies on a hard-coded "Success!" string that is sent from the backend. if (data.stdout.includes("Success!")) { - setToastContent(command); + setToastContent(data.command); if (toastTitle.textContent.trim()) { showLaunchSuccessfulToast(); } @@ -70,9 +98,10 @@ htmx.on("htmx:sseBeforeMessage", function (evt) { ShowRunCommandButton(); } } - evt.detail.elt.appendChild(document.createTextNode(data.stdout)); + + // Scrolling management if (shouldAutoScroll) { - // Set flag so event listner knows we are scrolling programatically + // Set flag so event listener knows we are scrolling programmatically isScrollingProgrammatically = true; evt.detail.elt.scrollTop = evt.detail.elt.scrollHeight; @@ -83,19 +112,32 @@ htmx.on("htmx:sseBeforeMessage", function (evt) { } }); -// Additional handlers for scroll inputs -logsElement.addEventListener( - "wheel", - function () { - shouldAutoScroll = false; - }, - { passive: true } -); - -logsElement.addEventListener( - "touchstart", - function () { - shouldAutoScroll = false; - }, - { passive: true } -); +// TODO we removed all code in these functions, which was too extensive. We should now clean this up. +function onCommandComplete() { + // // This is to enable "cancel" buttons. But as a side effect, it's causing all disable inputs to be + // // enabled, which is an error in some cases (e.g: "unset buttons") + // document.querySelectorAll("button").forEach((button) => { + // button.disabled = false; + // }); + // document.querySelectorAll("input").forEach((input) => { + // input.disabled = false; + // }); + // document.querySelectorAll(".form-switch").forEach((formSwitch) => { + // formSwitch.style.opacity = 1; + // }); + document.body.classList.remove("command-running"); +} +function onCommandRunning() { + // // This is to prevent running additional commands at the same time as a long-running + // // command. But it's way too extensive. We shouldn't disable ALL inputs. + // document.querySelectorAll("button").forEach((button) => { + // button.disabled = true; + // }); + // document.querySelectorAll("input").forEach((input) => { + // input.disabled = true; + // }); + // document.querySelectorAll(".form-switch").forEach((formSwitch) => { + // formSwitch.style.opacity = 0.5; + // }); + document.body.classList.add("command-running"); +} diff --git a/tutordeck/server/static/scss/deck.scss b/tutordeck/server/static/scss/deck.scss index cdef3e5..c6c8a59 100644 --- a/tutordeck/server/static/scss/deck.scss +++ b/tutordeck/server/static/scss/deck.scss @@ -364,10 +364,8 @@ main { } } #warning-command-running { - display: flex; justify-content: center; align-items: center; - display: none; border: 1px solid $gray-2; border-radius: 0.5em; align-items: center; @@ -1009,3 +1007,13 @@ main { } } } + +// Show/Hide/Disable elements when commands are running or are complete +.show-on-command-running { + display: none; +} +.command-running { + .show-on-command-running { + display: flex; + } +} diff --git a/tutordeck/server/templates/advanced.html b/tutordeck/server/templates/advanced.html index 306c729..dcf7f54 100644 --- a/tutordeck/server/templates/advanced.html +++ b/tutordeck/server/templates/advanced.html @@ -30,7 +30,7 @@ Search for any tutor command and execute it with a single click. {% block scripts %} {% endblock %} + +{% block scripts %} + +{% endblock %} diff --git a/tutordeck/server/templates/index.html b/tutordeck/server/templates/index.html index c42b68e..18eac4c 100644 --- a/tutordeck/server/templates/index.html +++ b/tutordeck/server/templates/index.html @@ -54,7 +54,7 @@
-
+
Command execution in progress. Click here to view details.
diff --git a/tutordeck/server/templates/local_launch.html b/tutordeck/server/templates/local_launch.html index 3caa7cc..bf7c89a 100644 --- a/tutordeck/server/templates/local_launch.html +++ b/tutordeck/server/templates/local_launch.html @@ -24,7 +24,7 @@ This will run Launch Platform to apply all plugin changes. This may take a few m {% block scripts %}