feat: display launch warning on config change

To achieve that, we had to get rid of the seq_command_executed request
parameter. We replaced it by a cookie, which is cleared after the
warning is displayed.

Note that we had to remove a feature, which was to disable/enable all
inputs while commands were running. This was causing very weird
behaviour, where some disabled buttons were being re-enabled again.

We also had to get rid of the cookieStore, which is not compatible with
non-https access. Http access is required in the self-serve AMI, for
instance.

Author: regisb

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",
 ]
 

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)
-
-    # Handle ajax call
-    response = Response("", status=200, content_type="text/html")
-    response.headers["HX-Redirect"] = url_for("configuration")
+    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")
+
+    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/<name>/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

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

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,34 +77,32 @@ 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.",
+	"tutor config save": {
+		title: "Configuration parameters were updated",
+		description: launchDescription,
 		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 local launch": {
+		title: "Platform was launched",
+		description: "",
+		showFooter: false,
 	},
 	"tutor plugins install": {
-		title: "Plugin Installed Successfully",
+		title: "Plugin was installed",
 		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.",
+	"tutor plugins enable": {
+		title: "Plugin was enabled",
+		description: launchDescription,
 		showFooter: true,
 	},
-	"tutor local launch": {
-		title: "Launch platform was successfully executed",
-		description: "",
-		showFooter: false,
+	"tutor plugins upgrade": {
+		title: "Plugin was updated",
+		description: launchDescription,
+		showFooter: true,
 	},
 };
 let toastTitle = document.getElementById("toast-title");
@@ -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";
-}