From 35752e611273f3c587af9f2210776419607a8396 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 26 Jun 2025 01:11:34 +0200 Subject: [PATCH 01/19] Migrate Turbolinks to Turbo This is the first part of using Turbo for this code base. Some solutions, like the temporary events triggered, should be temporary and removed once we fully migrated from Sprockets to another JavaScript distribution (such as Shakapacker, importmaps, etc.) --- Gemfile | 2 +- Gemfile.lock | 7 +--- app/assets/javascripts/application.js | 6 +++- app/assets/javascripts/base.js | 8 ++--- .../javascripts/bootstrap-dropdown-submenu.js | 2 +- .../javascripts/channels/la_exercises.js | 4 +-- .../channels/pg_matching_channel.js | 2 +- .../channels/synchronized_editor_channel.js | 2 +- app/assets/javascripts/codeharbor_link.js | 3 +- app/assets/javascripts/community_solution.js | 6 ++-- app/assets/javascripts/dashboard.js | 2 +- app/assets/javascripts/editor.js | 4 +-- app/assets/javascripts/editor/editor.js | 4 +-- app/assets/javascripts/error_templates.js | 2 +- .../javascripts/exercise_collections.js | 4 +-- app/assets/javascripts/exercise_graphs.js | 2 +- app/assets/javascripts/exercises.js | 2 +- app/assets/javascripts/external_users.js | 2 +- app/assets/javascripts/file_types.js | 2 +- app/assets/javascripts/forms.js | 4 +-- app/assets/javascripts/markdown_editor.js | 2 +- app/assets/javascripts/programming_groups.js | 2 +- .../javascripts/request_for_comments.js | 2 +- app/assets/javascripts/shell.js | 2 +- .../statistics_activity_history.js | 2 +- app/assets/javascripts/statistics_graphs.js | 2 +- .../javascripts/submission_statistics.js | 4 +-- app/assets/javascripts/working_time_graphs.js | 2 +- app/javascript/application.js | 4 +++ app/javascript/turbo-migration.js | 35 +++++++++++++++++++ app/javascript/webauthn.js | 2 +- app/views/admin/dashboard/show.html.slim | 4 +-- .../application/_locale_selector.html.slim | 2 +- app/views/application/_navigation.html.slim | 4 +-- app/views/community_solutions/edit.html.slim | 4 +-- .../execution_environments/_form.html.slim | 4 +-- .../execution_environments/show.html.slim | 2 +- .../statistics.html.slim | 2 +- .../exercise_collections/_form.html.slim | 2 +- app/views/exercise_collections/show.html.slim | 2 +- .../exercise_collections/statistics.html.slim | 2 +- app/views/exercises/_form.html.slim | 4 +-- app/views/exercises/_tips_content.html.slim | 4 +-- app/views/exercises/implement.html.slim | 4 +-- app/views/exercises/index.html.slim | 6 ++-- app/views/exercises/show.html.slim | 8 ++--- app/views/exercises/statistics.html.slim | 4 +-- .../exercises/study_group_dashboard.html.slim | 4 +-- app/views/layouts/application.html.slim | 9 ++--- app/views/programming_groups/index.html.slim | 2 +- app/views/proxy_exercises/_form.html.slim | 4 +-- app/views/proxy_exercises/index.html.slim | 2 +- .../statistics/activity_history.html.slim | 4 +-- app/views/statistics/graphs.html.slim | 4 +-- app/views/submissions/show.html.slim | 4 +-- app/views/tips/_form.html.slim | 4 +-- app/views/tips/show.html.slim | 4 +-- .../new.html.slim | 4 +-- app/views/webauthn_credentials/new.html.slim | 4 +-- .../initializers/content_security_policy.rb | 2 +- config/locales/de/programming_group.yml | 2 +- config/locales/en/programming_group.yml | 2 +- lib/assets/javascripts/color_mode_picker.js | 2 +- lib/assets/javascripts/flash.js | 2 +- package.json | 1 + spec/support/wait_for_ajax.rb | 9 ++++- yarn.lock | 25 +++++++++++++ 67 files changed, 176 insertions(+), 105 deletions(-) create mode 100644 app/javascript/turbo-migration.js diff --git a/Gemfile b/Gemfile index 776600a62..664cc9848 100644 --- a/Gemfile +++ b/Gemfile @@ -53,7 +53,7 @@ gem 'sprockets-rails' gem 'telegraf' gem 'terser', require: false gem 'tubesock', github: 'openhpi/tubesock' -gem 'turbolinks' +gem 'turbo-rails' gem 'webauthn' gem 'zxcvbn-ruby', require: 'zxcvbn' diff --git a/Gemfile.lock b/Gemfile.lock index 9e8608f29..9f5d0bf4d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -618,9 +618,6 @@ GEM turbo-rails (2.0.16) actionpack (>= 7.1.0) railties (>= 7.1.0) - turbolinks (5.2.1) - turbolinks-source (~> 5.2) - turbolinks-source (5.2.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (3.1.4) @@ -750,7 +747,7 @@ DEPENDENCIES telegraf terser tubesock! - turbolinks + turbo-rails web-console webauthn webmock @@ -993,8 +990,6 @@ CHECKSUMS tpm-key_attestation (0.14.1) sha256=7fd4e4653a7afd0a386632ddfb05d10ecfdd47678299c5e69165bc9ae111193f tubesock (0.2.9) turbo-rails (2.0.16) sha256=d24e1b60f0c575b3549ecda967e5391027143f8220d837ed792c8d48ea0ea38d - turbolinks (5.2.1) sha256=5fea5889c4e2a78a5bd9abda3860c565342b50c6e2593697d5558a08e15cce9c - turbolinks-source (5.2.0) sha256=362a41fa851a22b0f15cf8f944b6c7c5788f645dc1f61ae25478bb25c3bc85d4 tzinfo (2.0.6) sha256=8daf828cc77bcf7d63b0e3bdb6caa47e2272dcfaf4fbfe46f8c3a9df087a829b unicode-display_width (3.1.4) sha256=8caf2af1c0f2f07ec89ef9e18c7d88c2790e217c482bfc78aaa65eadd5415ac1 unicode-emoji (4.0.4) sha256=2c2c4ef7f353e5809497126285a50b23056cc6e61b64433764a35eff6c36532a diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index ba76b2e15..d58279b98 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -10,7 +10,6 @@ // Read Sprockets README (https://github.com/rails/sprockets#sprockets-directives) for details // about supported directives. // -//= require turbolinks //= require rails-timeago //= require locales/jquery.timeago.de.js // @@ -35,3 +34,8 @@ // // All remaining assets are loaded in alphabetical order //= require_tree . +// +// Finally, we dispatch a custom event to signal that all assets are loaded. +// This is used by our custom migration for Turbo to trigger the `turbo-migration:load` event +const sprocketsLoad = new Event('sprockets:load'); +document.dispatchEvent(sprocketsLoad); diff --git a/app/assets/javascripts/base.js b/app/assets/javascripts/base.js index 2a512a07a..5418bac26 100644 --- a/app/assets/javascripts/base.js +++ b/app/assets/javascripts/base.js @@ -4,7 +4,7 @@ Array.prototype.includes = function(element) { window.CodeOcean = { refresh: function() { - Turbolinks.visit(window.location.pathname); + Turbo.visit(window.location.pathname); } }; @@ -24,7 +24,7 @@ $.fn.scrollTo = function(selector) { }, ANIMATION_DURATION); }; -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { // Update all CSRF tokens on the page to reduce InvalidAuthenticityToken errors // See https://github.com/rails/jquery-ujs/issues/456 for details $.rails.refreshCSRFTokens(); @@ -45,7 +45,7 @@ $(document).on('turbolinks:load', function() { // Initialize Sentry const sentrySettings = $('meta[name="sentry"]') - // Workaround for Turbolinks: We must not re-initialize the Relay object when visiting another page + // Workaround for Turbo: We must not re-initialize the Relay object when visiting another page if (sentrySettings && sentrySettings.data()['enabled'] && Sentry.getReplay() === undefined) { Sentry.init({ dsn: sentrySettings.data('dsn'), @@ -69,7 +69,7 @@ $(document).on('turbolinks:load', function() { // Enable all tooltips $('[data-bs-toggle="tooltip"]').tooltip(); - // Enable sorttable again, as it is disabled otherwise by Turbolinks + // Enable sorttable again, as it is disabled otherwise by Turbo if (sorttable) { sorttable.init.done = false; sorttable.init(); diff --git a/app/assets/javascripts/bootstrap-dropdown-submenu.js b/app/assets/javascripts/bootstrap-dropdown-submenu.js index 38ad38540..65122d4c4 100644 --- a/app/assets/javascripts/bootstrap-dropdown-submenu.js +++ b/app/assets/javascripts/bootstrap-dropdown-submenu.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { var subMenusSelector = 'ul.dropdown-menu [data-bs-toggle=dropdown]'; diff --git a/app/assets/javascripts/channels/la_exercises.js b/app/assets/javascripts/channels/la_exercises.js index bfb7e0e20..ceef0135e 100644 --- a/app/assets/javascripts/channels/la_exercises.js +++ b/app/assets/javascripts/channels/la_exercises.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('exercises') && $('.teacher_dashboard').isPresent()) { const exercise_id = $('.teacher_dashboard').data().exerciseId; @@ -11,7 +11,7 @@ $(document).on('turbolinks:load', function() { function addClickEventToRfCEntry($row) { $row.click(function () { - Turbolinks.visit($(this).data("href")); + Turbo.visit($(this).data("href")); }); } diff --git a/app/assets/javascripts/channels/pg_matching_channel.js b/app/assets/javascripts/channels/pg_matching_channel.js index f07fe4fde..445cc5b59 100644 --- a/app/assets/javascripts/channels/pg_matching_channel.js +++ b/app/assets/javascripts/channels/pg_matching_channel.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function () { +$(document).on('turbo-migration:load', function () { if ($.isController('programming_groups') && window.location.pathname.includes('programming_groups/new')) { const matching_page = $('#matching'); diff --git a/app/assets/javascripts/channels/synchronized_editor_channel.js b/app/assets/javascripts/channels/synchronized_editor_channel.js index 6f9093280..0a54c4fcc 100644 --- a/app/assets/javascripts/channels/synchronized_editor_channel.js +++ b/app/assets/javascripts/channels/synchronized_editor_channel.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function () { +$(document).on('turbo-migration:load', function () { if (window.location.pathname.includes('/implement')) { var editor = $('#editor'); diff --git a/app/assets/javascripts/codeharbor_link.js b/app/assets/javascripts/codeharbor_link.js index c63e8b60a..95ed2f729 100644 --- a/app/assets/javascripts/codeharbor_link.js +++ b/app/assets/javascripts/codeharbor_link.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if($.isController('codeharbor_links')) { if ($('.edit_codeharbor_link, .new_codeharbor_link').isPresent()) { @@ -33,4 +33,3 @@ $(document).on('turbolinks:load', function() { } } }); - diff --git a/app/assets/javascripts/community_solution.js b/app/assets/javascripts/community_solution.js index dd3280bb6..1ce46f610 100644 --- a/app/assets/javascripts/community_solution.js +++ b/app/assets/javascripts/community_solution.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('community_solutions') && $('#community-solution-editor').isPresent()) { CodeOceanEditor.sendEvents = false; @@ -38,7 +38,7 @@ function submitCode(event) { this.autosaveIfChanged(); await this.stopCode(event); this.editors = []; - Turbolinks.clearCache(); - Turbolinks.visit(submission.redirect); + Turbo.cache.clear(); + Turbo.visit(submission.redirect); }); } diff --git a/app/assets/javascripts/dashboard.js b/app/assets/javascripts/dashboard.js index e8c434442..93247a12b 100644 --- a/app/assets/javascripts/dashboard.js +++ b/app/assets/javascripts/dashboard.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { var CHART_START = window.vis ? vis.moment().add(-1, 'minute') : undefined; var DEFAULT_REFRESH_INTERVAL = 5000; diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js index 9ca1e8263..3a8930e59 100644 --- a/app/assets/javascripts/editor.js +++ b/app/assets/javascripts/editor.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function(event) { +$(document).on('turbo-migration:load', function(event) { //Merge all editor components. $.extend( @@ -13,7 +13,7 @@ $(document).on('turbolinks:load', function(event) { CodeOceanEditorRequestForComments ); - if ($('#editor').isPresent() && CodeOceanEditor && event.originalEvent.data.url.includes("/implement")) { + if ($('#editor').isPresent() && CodeOceanEditor && event.detail.url.includes("/implement")) { CodeOceanEditor.initializeEverything(); } diff --git a/app/assets/javascripts/editor/editor.js b/app/assets/javascripts/editor/editor.js index 16e9f6412..8b9f9e4f4 100644 --- a/app/assets/javascripts/editor/editor.js +++ b/app/assets/javascripts/editor/editor.js @@ -1106,10 +1106,10 @@ var CodeOceanEditor = { this.initializeDeadlines(); CodeOceanEditorTips.initializeEventHandlers(); - window.addEventListener("turbolinks:before-render", App.synchronized_editor?.disconnect.bind(App.synchronized_editor)); + window.addEventListener("turbo:visit", App.synchronized_editor?.disconnect.bind(App.synchronized_editor)); window.addEventListener("beforeunload", App.synchronized_editor?.disconnect.bind(App.synchronized_editor)); - window.addEventListener("turbolinks:before-render", this.autosaveIfChanged.bind(this)); + window.addEventListener("turbo:visit", this.autosaveIfChanged.bind(this)); window.addEventListener("beforeunload", this.autosaveIfChanged.bind(this)); // create autosave when the editor is opened the first time this.autosave(); diff --git a/app/assets/javascripts/error_templates.js b/app/assets/javascripts/error_templates.js index d98840a77..72beff356 100644 --- a/app/assets/javascripts/error_templates.js +++ b/app/assets/javascripts/error_templates.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('error_templates')) { const button = $('#add-attribute').find('button') button.on('click', function () { diff --git a/app/assets/javascripts/exercise_collections.js b/app/assets/javascripts/exercise_collections.js index d89bcd271..7fb3f5ad1 100644 --- a/app/assets/javascripts/exercise_collections.js +++ b/app/assets/javascripts/exercise_collections.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('exercise_collections')) { var dataElement = $('#data'); var exerciseList = $('#exercise-list'); @@ -100,7 +100,7 @@ $(document).on('turbolinks:load', function() { tooltip.style("display", "none"); }) .on("click", function (_event, d) { - Turbolinks.visit(Routes.statistics_exercise_path(d.exercise_id)); + Turbo.visit(Routes.statistics_exercise_path(d.exercise_id)); }) .attr("x", function (d) { return x(d.index); diff --git a/app/assets/javascripts/exercise_graphs.js b/app/assets/javascripts/exercise_graphs.js index 18709969a..bb1a31c37 100644 --- a/app/assets/javascripts/exercise_graphs.js +++ b/app/assets/javascripts/exercise_graphs.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { // /exercises/38/statistics good for testing if ($.isController('exercises') && $('.graph-functions-2').isPresent()) { diff --git a/app/assets/javascripts/exercises.js b/app/assets/javascripts/exercises.js index 824e3d486..21d6959b1 100644 --- a/app/assets/javascripts/exercises.js +++ b/app/assets/javascripts/exercises.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function () { +$(document).on('turbo-migration:load', function () { const TAB_KEY_CODE = 9; let execution_environments; diff --git a/app/assets/javascripts/external_users.js b/app/assets/javascripts/external_users.js index f97436e31..6067584cb 100644 --- a/app/assets/javascripts/external_users.js +++ b/app/assets/javascripts/external_users.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { const grid = $('#tag-grid'); if ($.isController('external_users') && grid.isPresent()) { diff --git a/app/assets/javascripts/file_types.js b/app/assets/javascripts/file_types.js index 2133be370..1522e8193 100644 --- a/app/assets/javascripts/file_types.js +++ b/app/assets/javascripts/file_types.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('file_types')) { const select_tag = $('#file_type_editor_mode'); diff --git a/app/assets/javascripts/forms.js b/app/assets/javascripts/forms.js index 86b1b211c..08c02b910 100644 --- a/app/assets/javascripts/forms.js +++ b/app/assets/javascripts/forms.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { var CHOSEN_OPTIONS = { allow_single_deselect: true, disable_search_threshold: 5, @@ -50,6 +50,6 @@ $(document).on('turbolinks:load', function() { }); // Remove some elements before going back to an older site. Otherwise, they might not work. -$(document).on('turbolinks:before-cache', function() { +$(document).on('turbo:visit', function() { $('.chosen-container').remove(); }); diff --git a/app/assets/javascripts/markdown_editor.js b/app/assets/javascripts/markdown_editor.js index 7575aae41..f3f011873 100644 --- a/app/assets/javascripts/markdown_editor.js +++ b/app/assets/javascripts/markdown_editor.js @@ -192,7 +192,7 @@ const setResizeBtn = (formInput, editor) => { }); }; -$(document).on("turbolinks:load", function () { +$(document).on("turbo-migration:load", function () { initializeMarkdownEditors(); disableImageUpload(); }); diff --git a/app/assets/javascripts/programming_groups.js b/app/assets/javascripts/programming_groups.js index e49ce5055..fa74658d2 100644 --- a/app/assets/javascripts/programming_groups.js +++ b/app/assets/javascripts/programming_groups.js @@ -26,7 +26,7 @@ var ProgrammingGroups = { } }; -$(document).on('turbolinks:load', function () { +$(document).on('turbo-migration:load', function (event) { const modal = $('#modal-info-pair-programming'); if (modal.isPresent()) { ProgrammingGroups.initializeEventHandler(); diff --git a/app/assets/javascripts/request_for_comments.js b/app/assets/javascripts/request_for_comments.js index d0db56bc0..00fa73074 100644 --- a/app/assets/javascripts/request_for_comments.js +++ b/app/assets/javascripts/request_for_comments.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function () { +$(document).on('turbo-migration:load', function () { const exerciseCaption = $('#exercise_caption'); if (!$.isController('request_for_comments') || !exerciseCaption.isPresent()) { diff --git a/app/assets/javascripts/shell.js b/app/assets/javascripts/shell.js index d58426e34..e819d2535 100644 --- a/app/assets/javascripts/shell.js +++ b/app/assets/javascripts/shell.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function () { +$(document).on('turbo-migration:load', function () { const ENTER_KEY_CODE = 13; const clearOutput = function () { diff --git a/app/assets/javascripts/statistics_activity_history.js b/app/assets/javascripts/statistics_activity_history.js index 7126d428a..c4a488405 100644 --- a/app/assets/javascripts/statistics_activity_history.js +++ b/app/assets/javascripts/statistics_activity_history.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { function manageActivityHistory(prefix) { var containerId = prefix + '-activity-history'; diff --git a/app/assets/javascripts/statistics_graphs.js b/app/assets/javascripts/statistics_graphs.js index bda79fa6d..fba8c7b65 100644 --- a/app/assets/javascripts/statistics_graphs.js +++ b/app/assets/javascripts/statistics_graphs.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('statistics') && $('.graph#user-activity').isPresent()) { function manageGraph(containerId, url, refreshAfter) { diff --git a/app/assets/javascripts/submission_statistics.js b/app/assets/javascripts/submission_statistics.js index 8010af4ac..539f58cec 100644 --- a/app/assets/javascripts/submission_statistics.js +++ b/app/assets/javascripts/submission_statistics.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function(event) { +$(document).on('turbo-migration:load', function(event) { var currentSubmission = 0; var active_file = undefined; var fileTrees = []; @@ -60,7 +60,7 @@ $(document).on('turbolinks:load', function(event) { $(fileTrees[index]).show(); }; - if ($.isController('exercises') && $('#timeline').isPresent() && event.originalEvent.data.url.includes("/statistics")) { + if ($.isController('exercises') && $('#timeline').isPresent() && event.detail.url.includes("/statistics")) { var slider = $('#submissions-slider>input'); var submissions = $('#data').data('submissions'); diff --git a/app/assets/javascripts/working_time_graphs.js b/app/assets/javascripts/working_time_graphs.js index 7d09169df..f73e6d932 100644 --- a/app/assets/javascripts/working_time_graphs.js +++ b/app/assets/javascripts/working_time_graphs.js @@ -1,4 +1,4 @@ -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { // /38/statistics good for testing if ($.isController('exercises') && $('.working-time-graphs').isPresent()) { diff --git a/app/javascript/application.js b/app/javascript/application.js index 03bda112e..daaf6b697 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -84,3 +84,7 @@ import "ace-builds/src-noconflict/ext-language_tools"; // Enable autocompletion import "ace-builds/src-noconflict/ext-modelist"; // Enable language mode detection ace.config.set("useStrictCSP", true); // Enable strict CSP mode window.ace = ace; // Publish ace in global namespace + +// Turbo +import '@hotwired/turbo-rails'; +import './turbo-migration'; diff --git a/app/javascript/turbo-migration.js b/app/javascript/turbo-migration.js new file mode 100644 index 000000000..abe5b2f89 --- /dev/null +++ b/app/javascript/turbo-migration.js @@ -0,0 +1,35 @@ +// `turbo:load` is dispatched earlier than the previous `turbolinks:load` event. +// This is causing issues for our migration, since some assets are not fully loaded +// when the event is dispatched. To ensure that the DOM content is fully rendered, +// we use `requestAnimationFrame` to ensures that the DOM content is completely painted +// before dispatching a custom event `turbo-migration:load`. +// +// Further, we need to ensure that the `turbo-migration:load` event is only processed after +// Sprockets has loaded, since it would miss the event otherwise. +// +// We should remove this workaround once we fully migrated to Turbo and dropped Sprockets. + +let sprocketsLoaded = false; +const sprocketsLoadQueue = []; + +document.addEventListener('turbo:load', (event) => { + sprocketsLoaded ? forwardTurboLoad(event) : sprocketsLoadQueue.push(event); +}); + +// Wait for Sprockets to load before processing queued Turbo events +document.addEventListener('sprockets:load', () => { + sprocketsLoaded = true; + flushQueue(sprocketsLoadQueue); +}); + +function forwardTurboLoad(event) { + requestAnimationFrame(() => { + const delayedEvent = new CustomEvent('turbo-migration:load', { detail: { ...event.detail } }); + document.dispatchEvent(delayedEvent); + }); +} + +const flushQueue = (queue) => { + queue.forEach(forwardTurboLoad); + queue.length = 0; +}; diff --git a/app/javascript/webauthn.js b/app/javascript/webauthn.js index ea9400082..93da9b251 100644 --- a/app/javascript/webauthn.js +++ b/app/javascript/webauthn.js @@ -23,7 +23,7 @@ async function getCredential(publicKey) { return await get(options); } -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { if ($.isController('webauthn_credentials')) { form = $('form#new_webauthn_credential'); credentialMethod = createCredential; diff --git a/app/views/admin/dashboard/show.html.slim b/app/views/admin/dashboard/show.html.slim index a1ac431ee..a9b1daf8c 100644 --- a/app/views/admin/dashboard/show.html.slim +++ b/app/views/admin/dashboard/show.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('vis') - append_stylesheet_pack_tag('vis') diff --git a/app/views/application/_locale_selector.html.slim b/app/views/application/_locale_selector.html.slim index 3f429b616..1b595c7ba 100644 --- a/app/views/application/_locale_selector.html.slim +++ b/app/views/application/_locale_selector.html.slim @@ -4,4 +4,4 @@ li.nav-item.dropdown span.caret ul.dropdown-menu.p-0.mt-1 role='menu' - I18n.available_locales.sort_by {|locale| t("locales.#{locale}") }.each do |locale| - li = link_to(t("locales.#{locale}"), AuthenticatedUrlHelper.add_query_parameters(request.url, locale:), 'data-turbolinks': 'false', class: 'dropdown-item') + li = link_to(t("locales.#{locale}"), AuthenticatedUrlHelper.add_query_parameters(request.url, locale:), 'data-turbo': 'false', class: 'dropdown-item') diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index 337b64ede..b517e3f58 100644 --- a/app/views/application/_navigation.html.slim +++ b/app/views/application/_navigation.html.slim @@ -6,8 +6,8 @@ span.caret ul.dropdown-menu.p-0.mt-1 role='menu' - if current_user.admin? - li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path, class: 'dropdown-item', 'data-turbolinks': 'false') if policy(%i[admin dashboard]).show? - li = link_to(t('breadcrumbs.rails_admin.show'), rails_admin.dashboard_path, class: 'dropdown-item', 'data-turbolinks': 'false') if policy(%i[admin dashboard]).show? + li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path, class: 'dropdown-item', 'data-turbo': 'false') if policy(%i[admin dashboard]).show? + li = link_to(t('breadcrumbs.rails_admin.show'), rails_admin.dashboard_path, class: 'dropdown-item', 'data-turbo': 'false') if policy(%i[admin dashboard]).show? li = link_to(t('breadcrumbs.statistics.show'), statistics_path, class: 'dropdown-item') if policy(:statistics).show? li.dropdown-divider role='separator' = render('navigation_submenu', title: Exercise.model_name.human(count: :other), diff --git a/app/views/community_solutions/edit.html.slim b/app/views/community_solutions/edit.html.slim index a8f43e2c8..703bba84f 100644 --- a/app/views/community_solutions/edit.html.slim +++ b/app/views/community_solutions/edit.html.slim @@ -1,6 +1,6 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' == render 'form' diff --git a/app/views/execution_environments/_form.html.slim b/app/views/execution_environments/_form.html.slim index 081dd113a..24267a4a1 100644 --- a/app/views/execution_environments/_form.html.slim +++ b/app/views/execution_environments/_form.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/execution_environments/show.html.slim b/app/views/execution_environments/show.html.slim index d70f13abb..f85b01f17 100644 --- a/app/views/execution_environments/show.html.slim +++ b/app/views/execution_environments/show.html.slim @@ -5,7 +5,7 @@ h1.d-inline-block = @execution_environment ul.dropdown-menu.dropdown-menu-end role='menu' li = link_to(t('execution_environments.index.synchronize.button'), sync_to_runner_management_execution_environment_path(@execution_environment), method: :post, class: 'dropdown-item') if policy(@execution_environment).sync_to_runner_management? li = link_to(t('execution_environments.index.shell'), shell_execution_environment_path(@execution_environment), class: 'dropdown-item') if policy(@execution_environment).shell? - li = link_to(t('shared.statistics'), statistics_execution_environment_path(@execution_environment), 'data-turbolinks': 'false', class: 'dropdown-item') if policy(@execution_environment).statistics? + li = link_to(t('shared.statistics'), statistics_execution_environment_path(@execution_environment), 'data-turbo': 'false', class: 'dropdown-item') if policy(@execution_environment).statistics? li = link_to(t('shared.destroy'), @execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(@execution_environment).destroy? = row(label: 'execution_environment.name', value: @execution_environment.name) diff --git a/app/views/execution_environments/statistics.html.slim b/app/views/execution_environments/statistics.html.slim index e7053356e..fad2c4cd8 100644 --- a/app/views/execution_environments/statistics.html.slim +++ b/app/views/execution_environments/statistics.html.slim @@ -21,7 +21,7 @@ h1 = @execution_environment - if wts then average_time = wts['average_time'] else 0 # rubocop:disable Lint/ElseLayout - if wts then stddev_time = wts['stddev_time'] else 0 # rubocop:disable Lint/ElseLayout tr - td = link_to_if policy(exercise).statistics?, exercise.title, controller: 'exercises', action: 'statistics', id: exercise.id, 'data-turbolinks': 'false' + td = link_to_if policy(exercise).statistics?, exercise.title, controller: 'exercises', action: 'statistics', id: exercise.id, 'data-turbo': 'false' td = us['contributors'] td = us['average_score'].to_f.round(4) td = us['maximum_score'].to_f.round(2) diff --git a/app/views/exercise_collections/_form.html.slim b/app/views/exercise_collections/_form.html.slim index 99a55f914..351437580 100644 --- a/app/views/exercise_collections/_form.html.slim +++ b/app/views/exercise_collections/_form.html.slim @@ -24,7 +24,7 @@ td span.fa-solid.fa-bars td = item.exercise.title - td = link_to(t('shared.show'), item.exercise, 'data-turbolinks': 'false') + td = link_to(t('shared.show'), item.exercise, 'data-turbo': 'false') td a.remove-exercise href='#' = t('shared.destroy') .d-none diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim index 9ddf17bfe..b7a80dac0 100644 --- a/app/views/exercise_collections/show.html.slim +++ b/app/views/exercise_collections/show.html.slim @@ -25,4 +25,4 @@ h4.mt-4 = ExerciseCollection.human_attribute_name('exercises') td = link_to_if(policy(exercise).show?, exercise.title, exercise) td = link_to_if(exercise.execution_environment && policy(exercise.execution_environment).show?, exercise.execution_environment, exercise.execution_environment) td = link_to_if(exercise.user && policy(exercise.user).show?, exercise.user.displayname, exercise.user) - td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbolinks': 'false') if policy(exercise).statistics? + td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbo': 'false') if policy(exercise).statistics? diff --git a/app/views/exercise_collections/statistics.html.slim b/app/views/exercise_collections/statistics.html.slim index 66e8a960a..f76db18f8 100644 --- a/app/views/exercise_collections/statistics.html.slim +++ b/app/views/exercise_collections/statistics.html.slim @@ -48,4 +48,4 @@ h4.mt-4 = ExerciseCollection.human_attribute_name('exercises') td = exercise.submissions.send(:final).distinct.count(:contributor_id) td = exercise.finishers_percentage td = exercise.average_percentage - td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbolinks': 'false') if policy(exercise).statistics? + td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbo': 'false') if policy(exercise).statistics? diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index e1365556e..7cce42f29 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - append_javascript_pack_tag('sortable') - append_javascript_pack_tag('toast-ui') diff --git a/app/views/exercises/_tips_content.html.slim b/app/views/exercises/_tips_content.html.slim index 4491473fa..2d8b38eca 100644 --- a/app/views/exercises/_tips_content.html.slim +++ b/app/views/exercises/_tips_content.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') - append_stylesheet_pack_tag("multilang_#{I18n.locale}") diff --git a/app/views/exercises/implement.html.slim b/app/views/exercises/implement.html.slim index 355c0e1bc..32a54e5e4 100644 --- a/app/views/exercises/implement.html.slim +++ b/app/views/exercises/implement.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, lti_parameters might be nil - meta name='turbolinks-cache-control' content='no-cache' + meta name='turbo-cache-control' content='no-cache' - append_stylesheet_pack_tag("multilang_#{I18n.locale}") #editor-column diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index dac1b5f63..cf140e160 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -28,7 +28,7 @@ h1 = Exercise.model_name.human(count: :other) - @exercises.each do |exercise| tr data-id=exercise.id td.p-1.pt-2 - = link_to_if(policy(exercise).show?, exercise.title, exercise, 'data-turbolinks': 'false') + = link_to_if(policy(exercise).show?, exercise.title, exercise, 'data-turbo': 'false') - if exercise.internal_title.present? p.mb-0.text-muted i.fa-solid.fa-arrow-turn-up.fa-rotate-90 @@ -41,13 +41,13 @@ h1 = Exercise.model_name.human(count: :other) td.p-1.pt-2.public data-value=exercise.public? = symbol_for(exercise.public?) td.p-1.pt-2 = link_to(t('shared.edit'), edit_exercise_path(exercise)) if policy(exercise).edit? td.p-1.pt-2 = link_to(t('.implement'), implement_exercise_path(exercise)) if policy(exercise).implement? - td.p-1.pt-2 = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbolinks': 'false') if policy(exercise).statistics? + td.p-1.pt-2 = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbo': 'false') if policy(exercise).statistics? td.p-1 .btn-group button.btn.btn-outline-primary.btn-sm.dropdown-toggle data-bs-toggle='dropdown' type='button' = t('shared.actions_button') ul.dropdown-menu.float-end role='menu' - li = link_to(t('shared.show'), exercise, 'data-turbolinks': 'false', class: 'dropdown-item') if policy(exercise).show? + li = link_to(t('shared.show'), exercise, 'data-turbo': 'false', class: 'dropdown-item') if policy(exercise).show? li = link_to(UserExerciseFeedback.model_name.human(count: :other), feedback_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).feedback? li = link_to(RequestForComment.model_name.human(count: :other), exercise_request_for_comments_path(exercise), class: 'dropdown-item') if policy(exercise).rfcs_for_exercise? li = link_to(ProgrammingGroup.model_name.human(count: :other), exercise_programming_groups_path(exercise), class: 'dropdown-item') if policy(exercise).programming_groups_for_exercise? diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index eb2c0c6c8..1f322688b 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') @@ -15,8 +15,8 @@ h1.d-inline-block = render('shared/edit_button', object: @exercise) button.btn.btn-secondary.float-end.dropdown-toggle data-bs-toggle='dropdown' type='button' ul.dropdown-menu.dropdown-menu-end role='menu' - li = link_to(t('exercises.index.implement'), implement_exercise_path(@exercise), 'data-turbolinks': 'false', class: 'dropdown-item') if policy(@exercise).implement? - li = link_to(t('shared.statistics'), statistics_exercise_path(@exercise), 'data-turbolinks': 'false', class: 'dropdown-item') if policy(@exercise).statistics? + li = link_to(t('exercises.index.implement'), implement_exercise_path(@exercise), 'data-turbo': 'false', class: 'dropdown-item') if policy(@exercise).implement? + li = link_to(t('shared.statistics'), statistics_exercise_path(@exercise), 'data-turbo': 'false', class: 'dropdown-item') if policy(@exercise).statistics? li = link_to(UserExerciseFeedback.model_name.human(count: :other), feedback_exercise_path(@exercise), class: 'dropdown-item') if policy(@exercise).feedback? li = link_to(RequestForComment.model_name.human(count: :other), exercise_request_for_comments_path(@exercise), class: 'dropdown-item') if policy(@exercise).rfcs_for_exercise? li = link_to(ProgrammingGroup.model_name.human(count: :other), exercise_programming_groups_path(@exercise), class: 'dropdown-item') if policy(@exercise).programming_groups_for_exercise? diff --git a/app/views/exercises/statistics.html.slim b/app/views/exercises/statistics.html.slim index ac4d50847..aae517fbe 100644 --- a/app/views/exercises/statistics.html.slim +++ b/app/views/exercises/statistics.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('d3-tip') h1 = @exercise diff --git a/app/views/exercises/study_group_dashboard.html.slim b/app/views/exercises/study_group_dashboard.html.slim index d41a79486..832442b16 100644 --- a/app/views/exercises/study_group_dashboard.html.slim +++ b/app/views/exercises/study_group_dashboard.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('d3-tip') h1 diff --git a/app/views/layouts/application.html.slim b/app/views/layouts/application.html.slim index 94e33a523..177eaa091 100644 --- a/app/views/layouts/application.html.slim +++ b/app/views/layouts/application.html.slim @@ -12,10 +12,11 @@ html lang=I18n.locale data-default-locale=I18n.default_locale = favicon_link_tag('/icon.png', rel: 'apple-touch-icon', type: 'image/png') = tag.link rel: 'manifest', href: pwa_manifest_path = action_cable_meta_tag - = stylesheet_pack_tag('application', 'stylesheets', media: 'all', 'data-turbolinks-track': 'reload', integrity: true, crossorigin: 'anonymous') - = stylesheet_link_tag('application', media: 'all', 'data-turbolinks-track': 'reload', integrity: true, crossorigin: 'anonymous') - = javascript_pack_tag('application', 'data-turbolinks-track': 'reload', defer: false, integrity: true, crossorigin: 'anonymous') - = javascript_include_tag('application', 'data-turbolinks-track': 'reload', integrity: true, crossorigin: 'anonymous') + = stylesheet_pack_tag('application', 'stylesheets', media: 'all', 'data-turbo-track': 'reload', integrity: true, crossorigin: 'anonymous') + = stylesheet_link_tag('application', media: 'all', 'data-turbo-track': 'reload', integrity: true, crossorigin: 'anonymous') + // Since d3-tip is loaded via a separate pack and requires the application pack to be loaded first, we cannot use `defer` here. + = javascript_pack_tag('application', 'data-turbo-track': 'reload', defer: false, integrity: true, crossorigin: 'anonymous') + = javascript_include_tag('application', 'data-turbo-track': 'reload', defer: true, integrity: true, crossorigin: 'anonymous') = yield(:head) = csrf_meta_tags /= csp_meta_tag diff --git a/app/views/programming_groups/index.html.slim b/app/views/programming_groups/index.html.slim index 4b5dabde1..7a0ca76f7 100644 --- a/app/views/programming_groups/index.html.slim +++ b/app/views/programming_groups/index.html.slim @@ -30,7 +30,7 @@ tr td = link_to_if(policy(programming_group).show?, programming_group.displayname, programming_group) - if @exercise.nil? - td = link_to_if(policy(programming_group.exercise).show?, programming_group.exercise.title, programming_group.exercise, 'data-turbolinks': 'false') + td = link_to_if(policy(programming_group.exercise).show?, programming_group.exercise.title, programming_group.exercise, 'data-turbo': 'false') td == programming_group.users.map {|user| link_to_if(policy(user).show?, user.name, user) }.join(', ') td = programming_group.users.size td = l(programming_group.created_at, format: :short) diff --git a/app/views/proxy_exercises/_form.html.slim b/app/views/proxy_exercises/_form.html.slim index 472bcb8f6..269fb2fb5 100644 --- a/app/views/proxy_exercises/_form.html.slim +++ b/app/views/proxy_exercises/_form.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/proxy_exercises/index.html.slim b/app/views/proxy_exercises/index.html.slim index e596262cf..9d317daa8 100644 --- a/app/views/proxy_exercises/index.html.slim +++ b/app/views/proxy_exercises/index.html.slim @@ -31,7 +31,7 @@ h1 = ProxyExercise.model_name.human(count: :other) span.caret span.visually-hidden Toggle Dropdown ul.dropdown-menu.float-end role='menu' - li = link_to(t('shared.show'), proxy_exercise, 'data-turbolinks': 'false', class: 'dropdown-item') if policy(proxy_exercise).show? + li = link_to(t('shared.show'), proxy_exercise, 'data-turbo': 'false', class: 'dropdown-item') if policy(proxy_exercise).show? li = link_to(t('shared.destroy'), proxy_exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(proxy_exercise).destroy? li = link_to(t('.clone'), clone_proxy_exercise_path(proxy_exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(proxy_exercise).clone? diff --git a/app/views/statistics/activity_history.html.slim b/app/views/statistics/activity_history.html.slim index 631a9d38c..03e5ec03a 100644 --- a/app/views/statistics/activity_history.html.slim +++ b/app/views/statistics/activity_history.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('vis') - append_stylesheet_pack_tag('vis') diff --git a/app/views/statistics/graphs.html.slim b/app/views/statistics/graphs.html.slim index dbefd5ad0..97f7946d0 100644 --- a/app/views/statistics/graphs.html.slim +++ b/app/views/statistics/graphs.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('vis') - append_stylesheet_pack_tag('vis') diff --git a/app/views/submissions/show.html.slim b/app/views/submissions/show.html.slim index 2bd983e0b..e2da80929 100644 --- a/app/views/submissions/show.html.slim +++ b/app/views/submissions/show.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') diff --git a/app/views/tips/_form.html.slim b/app/views/tips/_form.html.slim index 933366cf0..04238c1d4 100644 --- a/app/views/tips/_form.html.slim +++ b/app/views/tips/_form.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/tips/show.html.slim b/app/views/tips/show.html.slim index d45524b71..5bce95473 100644 --- a/app/views/tips/show.html.slim +++ b/app/views/tips/show.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, code might not be highlighted correctly (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') - append_stylesheet_pack_tag("multilang_#{I18n.locale}") diff --git a/app/views/webauthn_credential_authentication/new.html.slim b/app/views/webauthn_credential_authentication/new.html.slim index df206b7b0..bcf78d6a7 100644 --- a/app/views/webauthn_credential_authentication/new.html.slim +++ b/app/views/webauthn_credential_authentication/new.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('webauthn') h1 = t('.headline') diff --git a/app/views/webauthn_credentials/new.html.slim b/app/views/webauthn_credentials/new.html.slim index 5bbfb5f03..15a5a8115 100644 --- a/app/views/webauthn_credentials/new.html.slim +++ b/app/views/webauthn_credentials/new.html.slim @@ -1,7 +1,7 @@ - content_for :head do - // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326. + // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbolinks-visit-control' content='reload' + meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('webauthn') h1 = t('shared.new_model', model: WebauthnCredential.model_name.human) diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index a74bfa6e9..581f739cb 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -67,7 +67,7 @@ def self.get_host_source(url) # The `script_src` directive is only a fallback for browsers not supporting `script_src_elem` and `script_src_attr`. policy.script_src :self, :report_sample # Some dependencies add new styles to the DOM dynamically, requiring :unsafe-inline. - # Currently, these include turbolinks, and vis.js. + # Currently, these include Turbo, and vis.js. policy.style_src_elem :self, :unsafe_inline, :report_sample # We still use some inline styles within the application, and indirectly through d3.js. # Further, the ToastUi markdown editor currently requires inline styles, too. diff --git a/config/locales/de/programming_group.yml b/config/locales/de/programming_group.yml index 2908e0599..d55deae2a 100644 --- a/config/locales/de/programming_group.yml +++ b/config/locales/de/programming_group.yml @@ -35,5 +35,5 @@ de: own_user_id: 'Ihre Personen-ID:' pair_programming_info: Pair Programming Info work_alone: Alleine arbeiten - work_alone_description: Sie können sich einmalig dafür entscheiden, die Aufgabe alleine zu bearbeiten. Anschließend können Sie jedoch nicht mehr in die Partnerarbeit für diese Aufgabe wechseln.
Klicken Sie hier, um die Aufgabe im Einzelmodus zu starten. + work_alone_description: Sie können sich einmalig dafür entscheiden, die Aufgabe alleine zu bearbeiten. Anschließend können Sie jedoch nicht mehr in die Partnerarbeit für diese Aufgabe wechseln.
Klicken Sie hier, um die Aufgabe im Einzelmodus zu starten. work_with_a_friend: Mit einem/einer Freund:in zusammenarbeiten diff --git a/config/locales/en/programming_group.yml b/config/locales/en/programming_group.yml index e96be6b40..8db57d807 100644 --- a/config/locales/en/programming_group.yml +++ b/config/locales/en/programming_group.yml @@ -35,5 +35,5 @@ en: own_user_id: 'Your user ID:' pair_programming_info: Pair Programming Info work_alone: Work Alone - work_alone_description: You can choose once to work on the exercise alone. Afterward, however, you will not be able to switch to work in a pair for this exercise.
Click here to get to the exercise in single mode. + work_alone_description: You can choose once to work on the exercise alone. Afterward, however, you will not be able to switch to work in a pair for this exercise.
Click here to get to the exercise in single mode. work_with_a_friend: Work with a friend diff --git a/lib/assets/javascripts/color_mode_picker.js b/lib/assets/javascripts/color_mode_picker.js index b3ce96cef..59b9edc81 100644 --- a/lib/assets/javascripts/color_mode_picker.js +++ b/lib/assets/javascripts/color_mode_picker.js @@ -77,7 +77,7 @@ function showActiveTheme(theme, focus = false) { } } -$(document).on('turbolinks:load', function() { +$(document).on('turbo-migration:load', function() { setTheme(getPreferredTheme()) window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', () => { diff --git a/lib/assets/javascripts/flash.js b/lib/assets/javascripts/flash.js index f1bfa6100..fd519b67f 100644 --- a/lib/assets/javascripts/flash.js +++ b/lib/assets/javascripts/flash.js @@ -1,4 +1,4 @@ -$( document ).on('turbolinks:load', function() { +$( document ).on('turbo-migration:load', function() { var DURATION = 10000; var SEVERITIES = ['danger', 'info', 'success', 'warning']; diff --git a/package.json b/package.json index f3b9a4544..90b80017f 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "@egjs/hammerjs": "^2.0.17", "@fortawesome/fontawesome-free": "^6.7.2", "@github/webauthn-json": "^2.1.1", + "@hotwired/turbo-rails": "^8.0.16", "@popperjs/core": "^2.11.8", "@sentry/browser": "^9.35.0", "@toast-ui/editor": "^3.2.2", diff --git a/spec/support/wait_for_ajax.rb b/spec/support/wait_for_ajax.rb index 9baf7bcff..293d45ced 100644 --- a/spec/support/wait_for_ajax.rb +++ b/spec/support/wait_for_ajax.rb @@ -7,7 +7,7 @@ def wait_for_ajax loop do sleep 0.1 # Short sleep time to prevent busy waiting - break if ajax_requests_finished? || (Time.current - start_time) > timeout + break if (ajax_requests_finished? && turbo_finished?) || (Time.current - start_time) > timeout end end @@ -16,6 +16,13 @@ def ajax_requests_finished? # Otherwise, Selenium and the browser driver might crash, preventing further tests from running. page.evaluate_script('jQuery.active').zero? end + + def turbo_finished? + # Check if Turbo is finished by looking for the absence of the progress bar. + if has_css?('.turbo-progress-bar', visible: true, wait: 0.1.seconds) + has_no_css?('.turbo-progress-bar') + end + end end RSpec.configure do |config| diff --git a/yarn.lock b/yarn.lock index 72cc05236..f945edd27 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1322,6 +1322,16 @@ __metadata: languageName: node linkType: hard +"@hotwired/turbo-rails@npm:^8.0.16": + version: 8.0.16 + resolution: "@hotwired/turbo-rails@npm:8.0.16" + dependencies: + "@hotwired/turbo": "npm:^8.0.13" + "@rails/actioncable": "npm:>=7.0" + checksum: 10c0/a3781bf5e7c798307754a726b5b66f63e4ec71a74e508549f257b3acf834ffc5c28baaf03b8dcdeeb7b35caa361769e417bbca629dcf873729398773585d35ca + languageName: node + linkType: hard + "@hotwired/turbo@npm:^7.3.0": version: 7.3.0 resolution: "@hotwired/turbo@npm:7.3.0" @@ -1329,6 +1339,13 @@ __metadata: languageName: node linkType: hard +"@hotwired/turbo@npm:^8.0.13": + version: 8.0.13 + resolution: "@hotwired/turbo@npm:8.0.13" + checksum: 10c0/fc9fd58ce2e006ad2f9e3948cf1ec71f47187ce8115f03e531bab849d0e13abc94cd0067f0888f7064d730b4c1a8212101bfa6d55f6166c6ad2db275e280149a + languageName: node + linkType: hard + "@isaacs/cliui@npm:^8.0.2": version: 8.0.2 resolution: "@isaacs/cliui@npm:8.0.2" @@ -1673,6 +1690,13 @@ __metadata: languageName: node linkType: hard +"@rails/actioncable@npm:>=7.0": + version: 8.0.200 + resolution: "@rails/actioncable@npm:8.0.200" + checksum: 10c0/42c861f3b43131ab523d37125e30e802bd5bab98bf8150a780531f82f05177cd0071a75a7f1341eac9de772732625194ac16205a40b82dc567cc05c21c56e412 + languageName: node + linkType: hard + "@rails/actioncable@npm:^7.0": version: 7.2.201 resolution: "@rails/actioncable@npm:7.2.201" @@ -2786,6 +2810,7 @@ __metadata: "@egjs/hammerjs": "npm:^2.0.17" "@fortawesome/fontawesome-free": "npm:^6.7.2" "@github/webauthn-json": "npm:^2.1.1" + "@hotwired/turbo-rails": "npm:^8.0.16" "@popperjs/core": "npm:^2.11.8" "@sentry/browser": "npm:^9.35.0" "@toast-ui/editor": "npm:^3.2.2" From 88e584d359de09e1027a951176b7e597ea4603e7 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 29 Jun 2025 19:51:45 +0200 Subject: [PATCH 02/19] Remove unnecessary disable of Turbo These links work correctly with Turbo, so that we don't need to opt out here. Also, we can cache the /implement route normally, since the LTI handling has changed in 2024. --- app/views/application/_locale_selector.html.slim | 2 +- app/views/exercises/show.html.slim | 2 +- app/views/proxy_exercises/index.html.slim | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/application/_locale_selector.html.slim b/app/views/application/_locale_selector.html.slim index 1b595c7ba..c7e0e673a 100644 --- a/app/views/application/_locale_selector.html.slim +++ b/app/views/application/_locale_selector.html.slim @@ -4,4 +4,4 @@ li.nav-item.dropdown span.caret ul.dropdown-menu.p-0.mt-1 role='menu' - I18n.available_locales.sort_by {|locale| t("locales.#{locale}") }.each do |locale| - li = link_to(t("locales.#{locale}"), AuthenticatedUrlHelper.add_query_parameters(request.url, locale:), 'data-turbo': 'false', class: 'dropdown-item') + li = link_to(t("locales.#{locale}"), AuthenticatedUrlHelper.add_query_parameters(request.url, locale:), class: 'dropdown-item') diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index 1f322688b..05e58f84c 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -15,7 +15,7 @@ h1.d-inline-block = render('shared/edit_button', object: @exercise) button.btn.btn-secondary.float-end.dropdown-toggle data-bs-toggle='dropdown' type='button' ul.dropdown-menu.dropdown-menu-end role='menu' - li = link_to(t('exercises.index.implement'), implement_exercise_path(@exercise), 'data-turbo': 'false', class: 'dropdown-item') if policy(@exercise).implement? + li = link_to(t('exercises.index.implement'), implement_exercise_path(@exercise), class: 'dropdown-item') if policy(@exercise).implement? li = link_to(t('shared.statistics'), statistics_exercise_path(@exercise), 'data-turbo': 'false', class: 'dropdown-item') if policy(@exercise).statistics? li = link_to(UserExerciseFeedback.model_name.human(count: :other), feedback_exercise_path(@exercise), class: 'dropdown-item') if policy(@exercise).feedback? li = link_to(RequestForComment.model_name.human(count: :other), exercise_request_for_comments_path(@exercise), class: 'dropdown-item') if policy(@exercise).rfcs_for_exercise? diff --git a/app/views/proxy_exercises/index.html.slim b/app/views/proxy_exercises/index.html.slim index 9d317daa8..b29ec2088 100644 --- a/app/views/proxy_exercises/index.html.slim +++ b/app/views/proxy_exercises/index.html.slim @@ -31,7 +31,7 @@ h1 = ProxyExercise.model_name.human(count: :other) span.caret span.visually-hidden Toggle Dropdown ul.dropdown-menu.float-end role='menu' - li = link_to(t('shared.show'), proxy_exercise, 'data-turbo': 'false', class: 'dropdown-item') if policy(proxy_exercise).show? + li = link_to(t('shared.show'), proxy_exercise, class: 'dropdown-item') if policy(proxy_exercise).show? li = link_to(t('shared.destroy'), proxy_exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(proxy_exercise).destroy? li = link_to(t('.clone'), clone_proxy_exercise_path(proxy_exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(proxy_exercise).clone? From 580a3eae631294594e825e0bce3f0cb03078537b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 26 Jun 2025 22:49:43 +0200 Subject: [PATCH 03/19] Ensure HTTP status Unprocessable Content (422) on validation errors Turbo imposes a new requirement for form submissions: Whenever a form is submitted, a redirect is expected. When no redirect is performed (i.e., when the data submitted didn't pass validation and the same form is rendered again), a 4xx or 5xx error code is required. Therefore, this commit changes those occurrences to return a 422 error code. Without this change, an error would be logged by Turbo in the JavaScript console and the response (including the flash message) wouldn't be shown. See https://turbo.hotwired.dev/handbook/drive#redirecting-after-a-form-submission --- app/controllers/concerns/common_behavior.rb | 2 +- .../error_template_attributes_controller.rb | 4 ++-- app/controllers/error_templates_controller.rb | 4 ++-- app/controllers/file_templates_controller.rb | 4 ++-- .../request_for_comments_controller.rb | 2 +- app/controllers/sessions_controller.rb | 2 +- .../codeharbor_links_controller_spec.rb | 2 ++ spec/controllers/consumers_controller_spec.rb | 6 +++--- .../execution_environments_controller_spec.rb | 4 ++-- spec/controllers/exercises_controller_spec.rb | 4 ++-- spec/controllers/file_types_controller_spec.rb | 4 ++-- .../internal_users_controller_spec.rb | 17 +++++++++-------- .../programming_groups_controller_spec.rb | 10 +++++----- spec/controllers/sessions_controller_spec.rb | 1 + 14 files changed, 35 insertions(+), 31 deletions(-) diff --git a/app/controllers/concerns/common_behavior.rb b/app/controllers/concerns/common_behavior.rb index d4f4cac53..91043ed95 100644 --- a/app/controllers/concerns/common_behavior.rb +++ b/app/controllers/concerns/common_behavior.rb @@ -31,7 +31,7 @@ def destroy_and_respond(options = {}) private :destroy_and_respond def respond_with_invalid_object(format, options = {}) - format.html { render(options[:template]) } + format.html { render(options[:template], status: :unprocessable_content) } format.json { render(json: @object.errors, status: :unprocessable_content) } end diff --git a/app/controllers/error_template_attributes_controller.rb b/app/controllers/error_template_attributes_controller.rb index ed5a7d25f..9e29443d1 100644 --- a/app/controllers/error_template_attributes_controller.rb +++ b/app/controllers/error_template_attributes_controller.rb @@ -46,7 +46,7 @@ def create end format.json { render :show, status: :created, location: @error_template_attribute } else - format.html { render :new } + format.html { render :new, status: :unprocessable_content } format.json { render json: @error_template_attribute.errors, status: :unprocessable_content } end end @@ -63,7 +63,7 @@ def update end format.json { render :show, status: :ok, location: @error_template_attribute } else - format.html { render :edit } + format.html { render :edit, status: :unprocessable_content } format.json { render json: @error_template_attribute.errors, status: :unprocessable_content } end end diff --git a/app/controllers/error_templates_controller.rb b/app/controllers/error_templates_controller.rb index ad734148e..cabe2b62f 100644 --- a/app/controllers/error_templates_controller.rb +++ b/app/controllers/error_templates_controller.rb @@ -43,7 +43,7 @@ def create format.html { redirect_to @error_template, notice: t('shared.object_created', model: @error_template.class.model_name.human) } format.json { render :show, status: :created, location: @error_template } else - format.html { render :new } + format.html { render :new, status: :unprocessable_content } format.json { render json: @error_template.errors, status: :unprocessable_content } end end @@ -58,7 +58,7 @@ def update format.html { redirect_to @error_template, notice: t('shared.object_updated', model: @error_template.class.model_name.human) } format.json { render :show, status: :ok, location: @error_template } else - format.html { render :edit } + format.html { render :edit, status: :unprocessable_content } format.json { render json: @error_template.errors, status: :unprocessable_content } end end diff --git a/app/controllers/file_templates_controller.rb b/app/controllers/file_templates_controller.rb index 8ad58ca8b..20802dd61 100644 --- a/app/controllers/file_templates_controller.rb +++ b/app/controllers/file_templates_controller.rb @@ -51,7 +51,7 @@ def create format.html { redirect_to @file_template, notice: t('shared.object_created', model: @file_template.class.model_name.human) } format.json { render :show, status: :created, location: @file_template } else - format.html { render :new } + format.html { render :new, status: :unprocessable_content } format.json { render json: @file_template.errors, status: :unprocessable_content } end end @@ -66,7 +66,7 @@ def update format.html { redirect_to @file_template, notice: t('shared.object_updated', model: @file_template.class.model_name.human) } format.json { render :show, status: :ok, location: @file_template } else - format.html { render :edit } + format.html { render :edit, status: :unprocessable_content } format.json { render json: @file_template.errors, status: :unprocessable_content } end end diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index 19f88e567..b1f8face2 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -155,7 +155,7 @@ def create format.json { render :show, status: :created, location: @request_for_comment } end else - format.html { render :new } + format.html { render :new, status: :unprocessable_content } format.json { render json: @request_for_comment.errors, status: :unprocessable_content } end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 8953db00f..d90b4dda8 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -51,7 +51,7 @@ def create _finalize_login(current_user) unless current_user.webauthn_configured? else flash.now[:danger] = t('.failure') - render(:new) + render :new, status: :unprocessable_content end end diff --git a/spec/controllers/codeharbor_links_controller_spec.rb b/spec/controllers/codeharbor_links_controller_spec.rb index 8a7947fa2..8a9557b84 100644 --- a/spec/controllers/codeharbor_links_controller_spec.rb +++ b/spec/controllers/codeharbor_links_controller_spec.rb @@ -50,6 +50,7 @@ expect { post_request }.not_to change(CodeharborLink, :count) end + expect_http_status(:unprocessable_content) expect_template(:new) end end @@ -84,6 +85,7 @@ expect { put_request }.not_to(change { codeharbor_link.reload.attributes }) end + expect_http_status(:unprocessable_content) expect_template(:edit) end end diff --git a/spec/controllers/consumers_controller_spec.rb b/spec/controllers/consumers_controller_spec.rb index 9467b4d0e..0b7dc943d 100644 --- a/spec/controllers/consumers_controller_spec.rb +++ b/spec/controllers/consumers_controller_spec.rb @@ -30,7 +30,7 @@ before { post :create, params: {consumer: {}} } expect_assigns(consumer: Consumer) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) end @@ -41,7 +41,7 @@ before { perform_request.call } expect_assigns(consumer: Consumer) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) end @@ -111,7 +111,7 @@ before { put :update, params: {consumer: {name: ''}, id: consumer.id} } expect_assigns(consumer: Consumer) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:edit) end end diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index 7830b7219..daee6c55d 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -49,7 +49,7 @@ end expect_assigns(execution_environment: ExecutionEnvironment) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) it 'does not register the execution environment with the runner management' do @@ -211,7 +211,7 @@ end expect_assigns(execution_environment: ExecutionEnvironment) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:edit) it 'does not update the execution environment at the runner management' do diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 51209b3ab..85328cc25 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -137,7 +137,7 @@ before { post :create, params: {exercise: {}} } expect_assigns(exercise: Exercise) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) end end @@ -312,7 +312,7 @@ before { put :update, params: {exercise: {title: ''}, id: exercise.id} } expect_assigns(exercise: Exercise) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:edit) end end diff --git a/spec/controllers/file_types_controller_spec.rb b/spec/controllers/file_types_controller_spec.rb index ef70d1e8c..87f456259 100644 --- a/spec/controllers/file_types_controller_spec.rb +++ b/spec/controllers/file_types_controller_spec.rb @@ -29,7 +29,7 @@ before { post :create, params: {file_type: {}} } expect_assigns(file_type: FileType) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) end end @@ -94,7 +94,7 @@ before { put :update, params: {file_type: {name: ''}, id: file_type.id} } expect_assigns(file_type: FileType) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:edit) end end diff --git a/spec/controllers/internal_users_controller_spec.rb b/spec/controllers/internal_users_controller_spec.rb index 0862745ce..03a877808 100644 --- a/spec/controllers/internal_users_controller_spec.rb +++ b/spec/controllers/internal_users_controller_spec.rb @@ -89,6 +89,7 @@ expect(assigns(:user).errors).to be_present end + expect_http_status(:unprocessable_content) expect_template(:activate) end @@ -181,7 +182,7 @@ expect(InternalUser.authenticate(user.email, password)).not_to eq(user) end - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:change_password) end @@ -204,7 +205,7 @@ end expect_assigns(user: :user) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:change_password) end @@ -214,7 +215,7 @@ end expect_assigns(user: :user) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:change_password) end end @@ -246,7 +247,7 @@ let(:password) { 'foo' } expect_assigns(user: :second_user) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:change_password) end end @@ -289,7 +290,7 @@ before { post :create, params: {internal_user: {email: ''}} } expect_assigns(user: InternalUser) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) end end @@ -440,7 +441,7 @@ expect(InternalUser.authenticate(user.email, password)).not_to eq(user) end - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:reset_password) end @@ -462,7 +463,7 @@ end expect_assigns(user: :user) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:reset_password) end end @@ -495,7 +496,7 @@ before { put :update, params: {internal_user: {email: ''}, id: user.id} } expect_assigns(user: InternalUser) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:edit) end end diff --git a/spec/controllers/programming_groups_controller_spec.rb b/spec/controllers/programming_groups_controller_spec.rb index 015f94fe2..64747c156 100644 --- a/spec/controllers/programming_groups_controller_spec.rb +++ b/spec/controllers/programming_groups_controller_spec.rb @@ -58,7 +58,7 @@ before { post :create, params: {exercise_id:, programming_group: pg_params} } expect_assigns(exercise: :exercise, programming_group: ProgrammingGroup) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) it 'does not create a new programming group' do @@ -89,7 +89,7 @@ before { perform_request.call } expect_assigns(exercise: :exercise, programming_group: ProgrammingGroup) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) end @@ -153,7 +153,7 @@ before { post :create, params: {exercise_id:, programming_group: pg_params} } expect_assigns(exercise: :exercise, programming_group: ProgrammingGroup) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) it 'does not create a new programming group' do @@ -172,7 +172,7 @@ before { post :create, params: {exercise_id:, programming_group: pg_params} } expect_assigns(exercise: :exercise, programming_group: ProgrammingGroup) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:new) it 'does not create a new programming group' do @@ -329,7 +329,7 @@ before { perform_request.call } expect_assigns(programming_group: ProgrammingGroup) - expect_http_status(:ok) + expect_http_status(:unprocessable_content) expect_template(:edit) end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 58ba6c30f..d72ecf2af 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -26,6 +26,7 @@ before { post :create, params: {email: user.email, password: '', remember_me: 1} } expect_flash_message(:danger, :'sessions.create.failure') + expect_http_status(:unprocessable_content) expect_template(:new) end end From 1c0695d1c84dc6026ad11a1ba22d762db081406b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 8 Jul 2025 10:09:06 +0200 Subject: [PATCH 04/19] Ensure HTTP status See Other (303) for form submissions Turbo requires us to use the HTTP status code 303 for successful form submissions after stateful server changes. See https://turbo.hotwired.dev/handbook/drive#redirecting-after-a-form-submission --- app/controllers/application_controller.rb | 8 +++--- .../code_ocean/files_controller.rb | 2 +- app/controllers/concerns/common_behavior.rb | 4 +-- app/controllers/concerns/lti.rb | 4 +-- app/controllers/concerns/redirect_behavior.rb | 8 +++--- .../concerns/webauthn/authentication.rb | 4 +-- .../error_template_attributes_controller.rb | 6 ++--- app/controllers/error_templates_controller.rb | 10 ++++---- .../execution_environments_controller.rb | 8 +++--- app/controllers/exercises_controller.rb | 25 +++++++++---------- app/controllers/file_templates_controller.rb | 6 ++--- app/controllers/internal_users_controller.rb | 8 +++--- app/controllers/live_streams_controller.rb | 4 +-- .../programming_groups_controller.rb | 5 ++-- app/controllers/proxy_exercises_controller.rb | 4 +-- .../request_for_comments_controller.rb | 2 +- app/controllers/sessions_controller.rb | 6 ++--- app/controllers/study_groups_controller.rb | 2 +- app/controllers/submissions_controller.rb | 4 +-- app/controllers/subscriptions_controller.rb | 6 ++--- .../user_exercise_feedbacks_controller.rb | 4 +-- ...hn_credential_authentication_controller.rb | 2 +- config/initializers/rails_admin.rb | 2 +- spec/concerns/lti_spec.rb | 12 ++++----- 24 files changed, 73 insertions(+), 73 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3377db520..8c4fafdbf 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -142,15 +142,15 @@ def render_error(message, status) format.any do # Prevent redirect loop if request.url == request.referer || request.referer&.match?(sign_in_path) - redirect_to :root, alert: message + redirect_to :root, alert: message, status: :see_other # Redirect to main domain if the request originated from our render_host elsif request.path == '/' && request.host == RENDER_HOST - redirect_to Rails.application.config.action_mailer.default_url_options, allow_other_host: true + redirect_to Rails.application.config.action_mailer.default_url_options, allow_other_host: true, status: :see_other elsif current_user.nil? && status == :unauthorized session[:return_to_url] = request.fullpath if current_user.nil? - redirect_to sign_in_path, alert: t('application.not_signed_in') + redirect_to sign_in_path, alert: t('application.not_signed_in'), status: :see_other else - redirect_back fallback_location: :root, allow_other_host: false, alert: message + redirect_back fallback_location: :root, allow_other_host: false, alert: message, status: :see_other end end format.json { render json: {error: message}, status: } diff --git a/app/controllers/code_ocean/files_controller.rb b/app/controllers/code_ocean/files_controller.rb index dbfba3b80..a7a1eae51 100644 --- a/app/controllers/code_ocean/files_controller.rb +++ b/app/controllers/code_ocean/files_controller.rb @@ -67,7 +67,7 @@ def create_and_respond(options = {}) filename = "#{@object.path || ''}/#{@object.name || ''}#{@object.file_type.try(:file_extension) || ''}" format.html do flash[:danger] = t('code_ocean/files.error.filename', name: filename) - redirect_to(options[:path]) + redirect_to options[:path], status: :see_other end format.json { render(json: @object.errors, status: :unprocessable_content) } end diff --git a/app/controllers/concerns/common_behavior.rb b/app/controllers/concerns/common_behavior.rb index 91043ed95..9f5e42bb0 100644 --- a/app/controllers/concerns/common_behavior.rb +++ b/app/controllers/concerns/common_behavior.rb @@ -24,7 +24,7 @@ def destroy_and_respond(options = {}) @object.destroy respond_to do |format| path = options[:path] || send(:"#{@object.class.model_name.collection}_path") - format.html { redirect_to(path, notice: t('shared.object_destroyed', model: @object.class.model_name.human)) } + format.html { redirect_to path, notice: t('shared.object_destroyed', model: @object.class.model_name.human), status: :see_other } format.json { head(:no_content) } end end @@ -36,7 +36,7 @@ def respond_with_invalid_object(format, options = {}) end def respond_with_valid_object(format, options = {}) - format.html { redirect_to(options[:path], notice: options[:notice]) } + format.html { redirect_to options[:path], notice: options[:notice], status: :see_other } format.json { render(:show, location: @object, status: options[:status]) } end private :respond_with_valid_object diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index 80d7fb4d7..8be2d3eb3 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -110,11 +110,11 @@ def return_to_consumer(options = {}) # The `lti_msg` is displayed to the user as an information. return_url = consumer_return_url(@provider, options) if return_url && URI.parse(return_url).absolute? - redirect_to(return_url, allow_other_host: true) + redirect_to return_url, allow_other_host: true, status: :see_other else flash[:danger] = options[:lti_errormsg] flash[:info] = options[:lti_msg] - redirect_to(:root) + redirect_to :root, status: :see_other end end diff --git a/app/controllers/concerns/redirect_behavior.rb b/app/controllers/concerns/redirect_behavior.rb index c10b9886e..3299c4d97 100644 --- a/app/controllers/concerns/redirect_behavior.rb +++ b/app/controllers/concerns/redirect_behavior.rb @@ -27,7 +27,7 @@ def redirect_after_submit def redirect_to_community_solution url = edit_community_solution_path(@community_solution, lock_id: @community_solution_lock.id) respond_to do |format| - format.html { redirect_to(url) } + format.html { redirect_to url, status: :see_other } format.json { render(json: {redirect: url}) } end end @@ -67,7 +67,7 @@ def redirect_to_user_feedback end respond_to do |format| - format.html { redirect_to(url) } + format.html { redirect_to url, status: :see_other } format.json { render(json: {redirect: url}) } end end @@ -81,7 +81,7 @@ def redirect_to_unsolved_rfc(own: false) @rfc.increment!(:times_featured) unless own # rubocop:disable Rails/SkipsModelValidations respond_to do |format| - format.html { redirect_to(@rfc) } + format.html { redirect_to @rfc, status: :see_other } format.json { render(json: {redirect: url_for(@rfc)}) } end end @@ -111,7 +111,7 @@ def redirect_to_lti_return_path path = lti_return_path(submission_id: @submission.id) respond_to do |format| - format.html { redirect_to(path) } + format.html { redirect_to path, status: :see_other } format.json { render(json: {redirect: path}) } end end diff --git a/app/controllers/concerns/webauthn/authentication.rb b/app/controllers/concerns/webauthn/authentication.rb index 52380a98e..6b53aeb5b 100644 --- a/app/controllers/concerns/webauthn/authentication.rb +++ b/app/controllers/concerns/webauthn/authentication.rb @@ -39,7 +39,7 @@ def authenticate(user) _sign_in_as(user) return _finalize_login(user) if user.fully_authenticated? - redirect_to new_webauthn_credential_authentication_path + redirect_to new_webauthn_credential_authentication_path, status: :see_other user end @@ -92,7 +92,7 @@ def destroy_webauthn_cookie(_user = nil) # Redirect to the WebAuthn authentication page if the user has not completed the WebAuthn authentication. def _require_webauthn_credential_authentication(user = current_user) - redirect_to new_webauthn_credential_authentication_path unless _webauthn_credential_authentication_completed?(user) + redirect_to(new_webauthn_credential_authentication_path, status: :see_other) unless _webauthn_credential_authentication_completed?(user) end # Finish the login process and redirect the user to the return_to_url. diff --git a/app/controllers/error_template_attributes_controller.rb b/app/controllers/error_template_attributes_controller.rb index 9e29443d1..219f24d1d 100644 --- a/app/controllers/error_template_attributes_controller.rb +++ b/app/controllers/error_template_attributes_controller.rb @@ -42,7 +42,7 @@ def create respond_to do |format| if @error_template_attribute.save format.html do - redirect_to @error_template_attribute, notice: t('shared.object_created', model: @error_template_attribute.class.model_name.human) + redirect_to @error_template_attribute, notice: t('shared.object_created', model: @error_template_attribute.class.model_name.human), status: :see_other end format.json { render :show, status: :created, location: @error_template_attribute } else @@ -59,7 +59,7 @@ def update respond_to do |format| if @error_template_attribute.update(error_template_attribute_params) format.html do - redirect_to @error_template_attribute, notice: t('shared.object_updated', model: @error_template_attribute.class.model_name.human) + redirect_to @error_template_attribute, notice: t('shared.object_updated', model: @error_template_attribute.class.model_name.human), status: :see_other end format.json { render :show, status: :ok, location: @error_template_attribute } else @@ -76,7 +76,7 @@ def destroy @error_template_attribute.destroy respond_to do |format| format.html do - redirect_to ErrorTemplateAttribute, notice: t('shared.object_destroyed', model: @error_template_attribute.class.model_name.human) + redirect_to ErrorTemplateAttribute, notice: t('shared.object_destroyed', model: @error_template_attribute.class.model_name.human), status: :see_other end format.json { head :no_content } end diff --git a/app/controllers/error_templates_controller.rb b/app/controllers/error_templates_controller.rb index cabe2b62f..b7dfce0d9 100644 --- a/app/controllers/error_templates_controller.rb +++ b/app/controllers/error_templates_controller.rb @@ -40,7 +40,7 @@ def create respond_to do |format| if @error_template.save - format.html { redirect_to @error_template, notice: t('shared.object_created', model: @error_template.class.model_name.human) } + format.html { redirect_to @error_template, notice: t('shared.object_created', model: @error_template.class.model_name.human), status: :see_other } format.json { render :show, status: :created, location: @error_template } else format.html { render :new, status: :unprocessable_content } @@ -55,7 +55,7 @@ def update authorize! respond_to do |format| if @error_template.update(error_template_params) - format.html { redirect_to @error_template, notice: t('shared.object_updated', model: @error_template.class.model_name.human) } + format.html { redirect_to @error_template, notice: t('shared.object_updated', model: @error_template.class.model_name.human), status: :see_other } format.json { render :show, status: :ok, location: @error_template } else format.html { render :edit, status: :unprocessable_content } @@ -70,7 +70,7 @@ def destroy authorize! @error_template.destroy respond_to do |format| - format.html { redirect_to ErrorTemplate, notice: t('shared.object_destroyed', model: @error_template.class.model_name.human) } + format.html { redirect_to ErrorTemplate, notice: t('shared.object_destroyed', model: @error_template.class.model_name.human), status: :see_other } format.json { head :no_content } end end @@ -79,7 +79,7 @@ def add_attribute authorize! @error_template.error_template_attributes << ErrorTemplateAttribute.find(params[:error_template_attribute_id]) respond_to do |format| - format.html { redirect_to @error_template } + format.html { redirect_to @error_template, status: :see_other } format.json { head :no_content } end end @@ -88,7 +88,7 @@ def remove_attribute authorize! @error_template.error_template_attributes.delete(ErrorTemplateAttribute.find(params[:error_template_attribute_id])) respond_to do |format| - format.html { redirect_to @error_template } + format.html { redirect_to @error_template, status: :see_other } format.json { head :no_content } end end diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index 640e47de1..81a92528f 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -194,9 +194,9 @@ def sync_to_runner_management Runner.strategy_class.sync_environment(@execution_environment) rescue Runner::Error => e Rails.logger.warn { "Runner error while synchronizing execution environment with id #{@execution_environment.id}: #{e.message}" } - redirect_to @execution_environment, alert: t('execution_environments.index.synchronize.failure', error: ERB::Util.html_escape(e.message)) + redirect_to @execution_environment, alert: t('execution_environments.index.synchronize.failure', error: ERB::Util.html_escape(e.message)), status: :see_other else - redirect_to @execution_environment, notice: t('execution_environments.index.synchronize.success') + redirect_to @execution_environment, notice: t('execution_environments.index.synchronize.success'), status: :see_other end end @@ -239,9 +239,9 @@ def sync_all_to_runner_management end if success.all? - redirect_to ExecutionEnvironment, notice: t('execution_environments.index.synchronize_all.success') + redirect_to ExecutionEnvironment, notice: t('execution_environments.index.synchronize_all.success'), status: :see_other else - redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure') + redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure'), status: :see_other end end diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index c9ac076f9..c9f992a5f 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -59,10 +59,10 @@ def clone exercise = @exercise.duplicate(public: false, token: nil, user: current_user, uuid: nil) exercise.send(:generate_token) if exercise.save - redirect_to(exercise_path(exercise), notice: t('shared.object_cloned', model: Exercise.model_name.human)) + redirect_to exercise_path(exercise), notice: t('shared.object_cloned', model: Exercise.model_name.human), status: :see_other else flash[:danger] = t('shared.message_failure') - redirect_to(@exercise) + redirect_to @exercise, status: :see_other end end @@ -238,7 +238,7 @@ def download_proforma zip_file = ProformaService::ExportTask.call(exercise: @exercise) send_data(zip_file.string, type: 'application/zip', filename: "exercise_#{@exercise.id}.zip", disposition: 'attachment') rescue ProformaXML::PostGenerateValidationError => e - redirect_to :root, danger: JSON.parse(e.message).join('
') + redirect_to :root, danger: JSON.parse(e.message).join('
'), status: :see_other end def user_from_api_key @@ -331,7 +331,7 @@ def handle_exercise_tips(tips_params) ExerciseTip.destroy(previous_exercise_tips - remaining_exercise_tips) rescue JSON::ParserError => e flash[:danger] = "JSON error: #{e.message}" - redirect_to(edit_exercise_path(@exercise)) + redirect_to edit_exercise_path(@exercise), status: :see_other end end @@ -349,7 +349,7 @@ def update_exercise_tips(exercise_tips, parent_exercise_tip_id, rank) rank += 1 unless current_exercise_tip.save flash[:danger] = current_exercise_tip.errors.full_messages.join('. ') - redirect_to(edit_exercise_path(@exercise)) and break + redirect_to(edit_exercise_path(@exercise), status: :see_other) and break end children = update_exercise_tips exercise_tip[:children], current_exercise_tip.id, rank @@ -371,17 +371,16 @@ def implement # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedC session.delete(:pair_programming) @current_contributor = current_user else - return redirect_back_or_to( - implement_exercise_path(current_contributor.exercise), - alert: t('exercises.implement.existing_programming_group', exercise: current_contributor.exercise.title) - ) + return redirect_back_or_to implement_exercise_path(current_contributor.exercise), + alert: t('exercises.implement.existing_programming_group', exercise: current_contributor.exercise.title), + status: :see_other end elsif session[:pg_id].blank? && (pg = current_user.programming_groups.find_by(exercise: @exercise)) && pg.submissions.where(study_group_id: current_user.current_study_group_id).any? # we are just acting on behalf of a single user who has already worked on this exercise as part of a programming group **in the context of the current study group** session[:pg_id] = pg.id @current_contributor = pg elsif session[:pg_id].blank? && session[:pair_programming] == 'mandatory' - return redirect_back_or_to(new_exercise_programming_group_path(@exercise)) + return redirect_back_or_to new_exercise_programming_group_path(@exercise), status: :see_other elsif session[:pg_id].blank? && session[:pair_programming] == 'optional' && current_user.submissions.where(study_group_id: current_user.current_study_group_id, exercise: @exercise).none? Event.find_or_create_by(category: 'pp_work_alone', user: current_user, exercise: @exercise, data: nil, file_id: nil) current_user.pair_programming_waiting_users&.find_by(exercise: @exercise)&.update(status: :worked_alone) @@ -492,9 +491,9 @@ def not_authorized_for_exercise(_exception) return render_not_authorized unless %w[implement working_times intervention reload].include?(action_name) if current_user.admin? || current_user.teacher? - redirect_to(@exercise, alert: t('exercises.implement.unpublished')) if @exercise.unpublished? - redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists? - redirect_to(@exercise, alert: t('exercises.implement.no_execution_environment')) if @exercise.execution_environment.blank? + redirect_to(@exercise, alert: t('exercises.implement.unpublished'), status: :see_other) if @exercise.unpublished? + redirect_to(@exercise, alert: t('exercises.implement.no_files'), status: :see_other) unless @exercise.files.visible.exists? + redirect_to(@exercise, alert: t('exercises.implement.no_execution_environment'), status: :see_other) if @exercise.execution_environment.blank? else render_not_authorized end diff --git a/app/controllers/file_templates_controller.rb b/app/controllers/file_templates_controller.rb index 20802dd61..8b1799009 100644 --- a/app/controllers/file_templates_controller.rb +++ b/app/controllers/file_templates_controller.rb @@ -48,7 +48,7 @@ def create respond_to do |format| if @file_template.save - format.html { redirect_to @file_template, notice: t('shared.object_created', model: @file_template.class.model_name.human) } + format.html { redirect_to @file_template, notice: t('shared.object_created', model: @file_template.class.model_name.human), status: :see_other } format.json { render :show, status: :created, location: @file_template } else format.html { render :new, status: :unprocessable_content } @@ -63,7 +63,7 @@ def update authorize! respond_to do |format| if @file_template.update(file_template_params) - format.html { redirect_to @file_template, notice: t('shared.object_updated', model: @file_template.class.model_name.human) } + format.html { redirect_to @file_template, notice: t('shared.object_updated', model: @file_template.class.model_name.human), status: :see_other } format.json { render :show, status: :ok, location: @file_template } else format.html { render :edit, status: :unprocessable_content } @@ -78,7 +78,7 @@ def destroy authorize! @file_template.destroy respond_to do |format| - format.html { redirect_to FileTemplate, notice: t('shared.object_destroyed', model: @file_template.class.model_name.human) } + format.html { redirect_to FileTemplate, notice: t('shared.object_destroyed', model: @file_template.class.model_name.human), status: :see_other } format.json { head :no_content } end end diff --git a/app/controllers/internal_users_controller.rb b/app/controllers/internal_users_controller.rb index 41d538092..9a2c5827e 100644 --- a/app/controllers/internal_users_controller.rb +++ b/app/controllers/internal_users_controller.rb @@ -95,7 +95,7 @@ def update_password if @user.update(params[:internal_user].permit(:password, :password_confirmation)) @user.change_password!(params[:internal_user][:password]) redirect_target = current_user ? internal_user_path(@user) : sign_in_path - format.html { redirect_to(redirect_target, notice: t('internal_users.reset_password.success')) } + format.html { redirect_to redirect_target, notice: t('internal_users.reset_password.success'), status: :see_other } format.json { head :ok } else respond_with_invalid_object(format, object: @user, template: action_name.to_sym) @@ -107,7 +107,7 @@ def deliver_reset_password_instructions if params[:email].present? user = InternalUser.arel_table InternalUser.where(user[:email].matches(params[:email])).first&.deliver_reset_password_instructions! - redirect_to(:root, notice: t('internal_users.forgot_password.success')) + redirect_to :root, notice: t('internal_users.forgot_password.success'), status: :see_other end end @@ -132,7 +132,7 @@ def platform_admin_param end def render_forgot_password_form - redirect_to(:root, alert: t('shared.already_signed_in')) if current_user + redirect_to(:root, alert: t('shared.already_signed_in'), status: :see_other) if current_user end def require_activation_token @@ -154,7 +154,7 @@ def set_up_password respond_to do |format| if @user.update(params[:internal_user].permit(:password, :password_confirmation)) @user.activate! - format.html { redirect_to(sign_in_path, notice: t('internal_users.activate.success')) } + format.html { redirect_to sign_in_path, notice: t('internal_users.activate.success'), status: :see_other } format.json { head :ok } else respond_with_invalid_object(format, object: @user, template: :activate) diff --git a/app/controllers/live_streams_controller.rb b/app/controllers/live_streams_controller.rb index 90ca52c9f..d525248db 100644 --- a/app/controllers/live_streams_controller.rb +++ b/app/controllers/live_streams_controller.rb @@ -21,7 +21,7 @@ def download_submission_file # Using the submission ID parameter would allow looking up the corresponding exercise ID # Therefore, we just redirect to the root_path, but actually expect to redirect back (that should work!) skip_authorization - redirect_back_or_to(root_path, allow_other_host: true, alert: t('exercises.download_file_tree.gone')) + redirect_back_or_to root_path, allow_other_host: true, alert: t('exercises.download_file_tree.gone'), status: :see_other else desired_file = params[:filename].to_s runner = Runner.for(current_contributor, @submission.exercise.execution_environment) @@ -65,7 +65,7 @@ def send_runner_file(runner, desired_file, redirect_fallback = root_path, privil end end rescue Runner::Error - redirect_back_or_to(redirect_fallback, alert: t('exercises.download_file_tree.gone')) + redirect_back_or_to redirect_fallback, alert: t('exercises.download_file_tree.gone'), status: :see_other end end end diff --git a/app/controllers/programming_groups_controller.rb b/app/controllers/programming_groups_controller.rb index d1304202b..71d6fe2cc 100644 --- a/app/controllers/programming_groups_controller.rb +++ b/app/controllers/programming_groups_controller.rb @@ -121,7 +121,8 @@ def set_programming_group_and_authorize def redirect_to_exercise skip_authorization - redirect_to(implement_exercise_path(@exercise), - notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise, current_user) ? 'with' : 'without'}_outcome", consumer: @consumer)) + redirect_to implement_exercise_path(@exercise), + notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise, current_user) ? 'with' : 'without'}_outcome", consumer: @consumer), + status: :see_other end end diff --git a/app/controllers/proxy_exercises_controller.rb b/app/controllers/proxy_exercises_controller.rb index 74e673696..91963f087 100644 --- a/app/controllers/proxy_exercises_controller.rb +++ b/app/controllers/proxy_exercises_controller.rb @@ -15,10 +15,10 @@ def clone user: current_user) proxy_exercise.send(:generate_token) if proxy_exercise.save - redirect_to(proxy_exercise_path(proxy_exercise), notice: t('shared.object_cloned', model: ProxyExercise.model_name.human)) + redirect_to proxy_exercise_path(proxy_exercise), notice: t('shared.object_cloned', model: ProxyExercise.model_name.human), status: :see_other else flash[:danger] = t('shared.message_failure') - redirect_to(@proxy_exercise) + redirect_to @proxy_exercise, status: :see_other end end diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index b1f8face2..3e3b9e13d 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -168,7 +168,7 @@ def report ReportMailer.with(reported_content: @request_for_comment).report_content.deliver_later - redirect_to(@request_for_comment, notice: t('.report.reported')) + redirect_to @request_for_comment, notice: t('.report.reported'), status: :see_other end private diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index d90b4dda8..952a4ac3c 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -16,7 +16,7 @@ class SessionsController < ApplicationController after_action :set_sentry_context, only: %i[create_through_lti create] def new - redirect_to(:root, alert: t('shared.already_signed_in')) if current_user + redirect_to(:root, alert: t('shared.already_signed_in'), status: :see_other) if current_user end def create_through_lti @@ -83,7 +83,7 @@ def destroy logout end flash[:notice] = t('.success') - redirect_to(:root) unless performed? + redirect_to(:root, status: :see_other) unless performed? end private @@ -126,6 +126,6 @@ def redirect_to_survey uri.query_values = query_params # This redirect skips the WebAuthn requirement - redirect_to uri.to_s, allow_other_host: true + redirect_to uri.to_s, allow_other_host: true, status: :see_other end end diff --git a/app/controllers/study_groups_controller.rb b/app/controllers/study_groups_controller.rb index 443457d09..e1ae20674 100644 --- a/app/controllers/study_groups_controller.rb +++ b/app/controllers/study_groups_controller.rb @@ -39,7 +39,7 @@ def study_group_params def set_as_current session[:study_group_id] = @study_group.id current_user.store_current_study_group_id(@study_group.id) - redirect_back_or_to(root_path, notice: t('study_groups.set_as_current.success')) + redirect_back_or_to root_path, notice: t('study_groups.set_as_current.success'), status: :see_other end def set_group diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index a19787dec..81f87af64 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -66,7 +66,7 @@ def download_file raise Pundit::NotAuthorizedError if @embed_options[:disable_download] if @file.native_file? - redirect_to protected_upload_path(id: @file.id, filename: @file.filepath) + redirect_to protected_upload_path(id: @file.id, filename: @file.filepath), status: :see_other else response.set_header('Content-Length', @file.size) send_data(@file.content, type: 'application/octet-stream', filename: @file.name_with_extension, disposition: 'attachment') @@ -98,7 +98,7 @@ def render_file # Finally grant access and send the file if @file.native_file? url = render_protected_upload_url(id: @file.id, filename: @file.filepath) - redirect_to AuthenticatedUrlHelper.sign(url, @file) + redirect_to AuthenticatedUrlHelper.sign(url, @file), status: :see_other else response.set_header('Content-Length', @file.size) send_data(@file.content, filename: @file.name_with_extension, disposition: 'inline') diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index d9182903f..f6a686c01 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -26,7 +26,7 @@ def destroy rescue StandardError skip_authorization respond_to do |format| - format.html { redirect_to RequestForComment, alert: t('subscriptions.subscription_not_existent') } + format.html { redirect_to RequestForComment, alert: t('subscriptions.subscription_not_existent'), status: :see_other } format.json { render json: {message: t('subscriptions.subscription_not_existent')}, status: :not_found } end else @@ -35,12 +35,12 @@ def destroy @subscription.deleted = true if @subscription.save respond_to do |format| - format.html { redirect_to rfc, notice: t('subscriptions.successfully_unsubscribed') } + format.html { redirect_to rfc, notice: t('subscriptions.successfully_unsubscribed'), status: :see_other } format.json { render json: {message: t('subscriptions.successfully_unsubscribed')}, status: :ok } end else respond_to do |format| - format.html { redirect_to rfc, flash: {danger: t('shared.message_failure')} } + format.html { redirect_to rfc, flash: {danger: t('shared.message_failure')}, status: :see_other } format.json { render json: {message: t('shared.message_failure')}, status: :internal_server_error } end end diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index 3d6263345..4976921e6 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -53,7 +53,7 @@ def create create_and_respond(object: @uef, path: proc { path }) else flash.now[:danger] = t('shared.message_failure') - redirect_back fallback_location: exercise_user_exercise_feedback_path(@uef) + redirect_back fallback_location: exercise_user_exercise_feedback_path(@uef), status: :see_other end end @@ -75,7 +75,7 @@ def update update_and_respond(object: @uef, params: uef_params, path:) else flash.now[:danger] = t('shared.message_failure') - redirect_back fallback_location: exercise_user_exercise_feedback_path(@uef) + redirect_back fallback_location: exercise_user_exercise_feedback_path(@uef), status: :see_other end end diff --git a/app/controllers/webauthn_credential_authentication_controller.rb b/app/controllers/webauthn_credential_authentication_controller.rb index 76171253f..633865414 100644 --- a/app/controllers/webauthn_credential_authentication_controller.rb +++ b/app/controllers/webauthn_credential_authentication_controller.rb @@ -36,7 +36,7 @@ def create authenticate_webauthn_for(credential) rescue WebAuthn::Error => e - redirect_to new_webauthn_credential_authentication_path, danger: t('.failed', error: e.message) + redirect_to new_webauthn_credential_authentication_path, danger: t('.failed', error: e.message), status: :see_other end private diff --git a/config/initializers/rails_admin.rb b/config/initializers/rails_admin.rb index 9fc9c39c3..824bce773 100644 --- a/config/initializers/rails_admin.rb +++ b/config/initializers/rails_admin.rb @@ -20,7 +20,7 @@ # Important! We need to check the authorization here, we skip Pundit checks in the RailsAdminController. unless current_user&.admin? flash[:alert] = t('application.not_authorized') - redirect_to main_app.root_path + redirect_to main_app.root_path, status: :see_other end end diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index 5e1890cf0..50cab158f 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -57,13 +57,13 @@ class Controller < AnonymousController before { controller.instance_variable_set(:@provider, provider) } it 'redirects to the tool consumer' do - expect(controller).to receive(:redirect_to).with(consumer_return_url, allow_other_host: true) + expect(controller).to receive(:redirect_to).with(consumer_return_url, allow_other_host: true, status: :see_other) controller.send(:return_to_consumer) end it 'passes messages to the consumer' do message = I18n.t('sessions.oauth.failure', error: 'dummy error') - expect(controller).to receive(:redirect_to).with("#{consumer_return_url}?lti_errorlog=#{CGI.escape(message)}", allow_other_host: true) + expect(controller).to receive(:redirect_to).with("#{consumer_return_url}?lti_errorlog=#{CGI.escape(message)}", allow_other_host: true, status: :see_other) controller.send(:return_to_consumer, lti_errorlog: message) end @@ -72,7 +72,7 @@ class Controller < AnonymousController it 'correctly appends query parameters' do message = I18n.t('sessions.oauth.failure', error: 'dummy error') - expect(controller).to receive(:redirect_to).with("#{consumer_return_url}<i_errorlog=#{CGI.escape(message)}<i_msg=#{CGI.escape(message)}", allow_other_host: true) + expect(controller).to receive(:redirect_to).with("#{consumer_return_url}<i_errorlog=#{CGI.escape(message)}<i_msg=#{CGI.escape(message)}", allow_other_host: true, status: :see_other) controller.send(:return_to_consumer, lti_errorlog: message, lti_msg: message) end end @@ -81,7 +81,7 @@ class Controller < AnonymousController let(:consumer_return_url) { '' } it 'redirects to the root URL' do - expect(controller).to receive(:redirect_to).with(:root) + expect(controller).to receive(:redirect_to).with(:root, status: :see_other) controller.send(:return_to_consumer) end @@ -96,7 +96,7 @@ class Controller < AnonymousController let(:consumer_return_url) { '/path' } it 'redirects to the root URL' do - expect(controller).to receive(:redirect_to).with(:root) + expect(controller).to receive(:redirect_to).with(:root, status: :see_other) controller.send(:return_to_consumer) end @@ -114,7 +114,7 @@ class Controller < AnonymousController end it 'redirects to the root URL' do - expect(controller).to receive(:redirect_to).with(:root) + expect(controller).to receive(:redirect_to).with(:root, status: :see_other) controller.send(:return_to_consumer) end From a61d667bb7b0561868eb25f0cd96ca31e1585578 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 8 Jul 2025 00:06:39 +0200 Subject: [PATCH 05/19] Migrate `link_to` to `button_to` for stateful actions in views For these actions, we use use a non-default `method` (other than `GET`). To indicate that within the DOM (i.e., for Screenreaders) but also enhance compatibility with Turbo, we switch to proper `button_to` forms here. As part of this migration, some buttons should still be shown inline (hence, we add the `form_class: 'd-inline-block'`) or with the same height as regular links (hence, we add `class: 'h-100'`). See https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-%5BHotwire-Turbo-integration%5D#data-method-vs-data-turbo-method See https://gorails.com/episodes/link_to-vs-button_to-in-rails --- app/views/application/_session.html.slim | 6 +++--- app/views/codeharbor_links/_form.html.slim | 2 +- app/views/consumers/index.html.slim | 2 +- app/views/error_template_attributes/index.html.slim | 2 +- app/views/error_templates/index.html.slim | 2 +- app/views/error_templates/show.html.slim | 2 +- app/views/execution_environments/index.html.slim | 2 +- app/views/execution_environments/show.html.slim | 4 ++-- app/views/exercise_collections/index.html.slim | 2 +- app/views/exercises/index.html.slim | 4 ++-- app/views/exercises/show.html.slim | 6 +++--- app/views/external_users/show.html.slim | 2 +- app/views/file_templates/index.html.slim | 2 +- app/views/file_types/index.html.slim | 2 +- app/views/internal_users/index.html.slim | 2 +- app/views/internal_users/show.html.slim | 2 +- app/views/programming_groups/index.html.slim | 2 +- app/views/proxy_exercises/index.html.slim | 4 ++-- app/views/request_for_comments/_admin_menu.html.slim | 2 +- app/views/study_groups/_table.html.slim | 2 +- app/views/tags/index.html.slim | 2 +- app/views/tips/index.html.slim | 2 +- .../webauthn_credential_authentication/_form.html.slim | 2 +- app/views/webauthn_credentials/_list.html.slim | 2 +- 24 files changed, 31 insertions(+), 31 deletions(-) diff --git a/app/views/application/_session.html.slim b/app/views/application/_session.html.slim index c62dbf2f2..ecfa91c90 100644 --- a/app/views/application/_session.html.slim +++ b/app/views/application/_session.html.slim @@ -17,11 +17,11 @@ - if current_user.admin? || current_user.teacher? || current_user.internal_user? li = link_to(t('consumers.show.link'), current_user.consumer, class: 'dropdown-item') if current_user.consumer && policy(current_user.consumer).show? li = link_to(t('internal_users.show.link'), current_user, class: 'dropdown-item') if policy(current_user).show? - li = link_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') + li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') - elsif current_user.webauthn_configured? - li = link_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') + li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') - else - li = link_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') + li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') - else li.nav-item = link_to(sign_in_path, class: 'nav-link') do i.fa-solid.fa-arrow-right-to-bracket diff --git a/app/views/codeharbor_links/_form.html.slim b/app/views/codeharbor_links/_form.html.slim index 0d69e8ea5..0c2fda13d 100644 --- a/app/views/codeharbor_links/_form.html.slim +++ b/app/views/codeharbor_links/_form.html.slim @@ -15,4 +15,4 @@ .actions = render('shared/submit_button', f:, object: @codeharbor_link) - if @codeharbor_link.persisted? - = link_to(t('codeharbor_link.delete'), polymorphic_path([@codeharbor_link.user, @codeharbor_link]), data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-danger float-end') + = button_to(t('codeharbor_link.delete'), polymorphic_path([@codeharbor_link.user, @codeharbor_link]), data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-danger float-end') diff --git a/app/views/consumers/index.html.slim b/app/views/consumers/index.html.slim index 9805fa18b..7184aca58 100644 --- a/app/views/consumers/index.html.slim +++ b/app/views/consumers/index.html.slim @@ -12,7 +12,7 @@ h1 = Consumer.model_name.human(count: :other) td = link_to_if(policy(consumer).show?, consumer.name, consumer) td = link_to(t('shared.show'), consumer) if policy(consumer).show? td = link_to(t('shared.edit'), edit_consumer_path(consumer)) if policy(consumer).edit? - td = link_to(t('shared.destroy'), consumer, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(consumer).destroy? + td = button_to(t('shared.destroy'), consumer, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(consumer).destroy? = render('shared/pagination', collection: @consumers) p = render('shared/new_button', model: Consumer) diff --git a/app/views/error_template_attributes/index.html.slim b/app/views/error_template_attributes/index.html.slim index e83b592cb..1293b9bce 100644 --- a/app/views/error_template_attributes/index.html.slim +++ b/app/views/error_template_attributes/index.html.slim @@ -23,7 +23,7 @@ h1 = ErrorTemplateAttribute.model_name.human(count: :other) code = error_template_attribute.regex td = link_to(t('shared.show'), error_template_attribute) if policy(error_template_attribute).show? td = link_to(t('shared.edit'), edit_error_template_attribute_path(error_template_attribute)) if policy(error_template_attribute).edit? - td = link_to(t('shared.destroy'), error_template_attribute, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(error_template_attribute).destroy? + td = button_to(t('shared.destroy'), error_template_attribute, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(error_template_attribute).destroy? = render('shared/pagination', collection: @error_template_attributes) p = render('shared/new_button', model: ErrorTemplateAttribute) diff --git a/app/views/error_templates/index.html.slim b/app/views/error_templates/index.html.slim index f07281818..ad18f74fc 100644 --- a/app/views/error_templates/index.html.slim +++ b/app/views/error_templates/index.html.slim @@ -16,7 +16,7 @@ h1 = ErrorTemplate.model_name.human(count: :other) td = link_to(error_template.execution_environment) td = link_to(t('shared.show'), error_template) if policy(error_template).show? td = link_to(t('shared.edit'), edit_error_template_path(error_template)) if policy(error_template).edit? - td = link_to(t('shared.destroy'), error_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(error_template).destroy? + td = button_to(t('shared.destroy'), error_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(error_template).destroy? = render('shared/pagination', collection: @error_templates) p = render('shared/new_button', model: ErrorTemplate) diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index 258e528c0..7fa2e4160 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -35,7 +35,7 @@ h2.mt-4 code = attribute.regex td = link_to(t('shared.show'), attribute) if policy(attribute).show? td = link_to(t('shared.edit'), edit_error_template_attribute_path(attribute)) if policy(attribute).edit? - td = link_to(t('shared.destroy'), attribute_error_template_url(error_template_attribute_id: attribute.id), method: :delete) if policy(attribute).destroy? + td = button_to(t('shared.destroy'), attribute_error_template_url(error_template_attribute_id: attribute.id), method: :delete, class: 'btn btn-sm btn-link') if policy(attribute).destroy? #add-attribute = collection_select({}, :error_template_attribute_id, diff --git a/app/views/execution_environments/index.html.slim b/app/views/execution_environments/index.html.slim index 9c869bde6..cdd397321 100644 --- a/app/views/execution_environments/index.html.slim +++ b/app/views/execution_environments/index.html.slim @@ -31,7 +31,7 @@ h1.d-inline-block = ExecutionEnvironment.model_name.human(count: :other) td = execution_environment.permitted_execution_time td = link_to(t('shared.show'), execution_environment) if policy(execution_environment).show? td = link_to(t('shared.edit'), edit_execution_environment_path(execution_environment)) if policy(execution_environment).edit? - td = link_to(t('shared.destroy'), execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(execution_environment).destroy? + td = button_to(t('shared.destroy'), execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(execution_environment).destroy? td = link_to(t('.shell'), shell_execution_environment_path(execution_environment)) if policy(execution_environment).shell? td = link_to(t('shared.statistics'), statistics_execution_environment_path(execution_environment)) if policy(execution_environment).statistics? diff --git a/app/views/execution_environments/show.html.slim b/app/views/execution_environments/show.html.slim index f85b01f17..64cb18f5b 100644 --- a/app/views/execution_environments/show.html.slim +++ b/app/views/execution_environments/show.html.slim @@ -3,10 +3,10 @@ h1.d-inline-block = @execution_environment = render('shared/edit_button', object: @execution_environment) button.btn.btn-secondary.float-end.dropdown-toggle data-bs-toggle='dropdown' type='button' ul.dropdown-menu.dropdown-menu-end role='menu' - li = link_to(t('execution_environments.index.synchronize.button'), sync_to_runner_management_execution_environment_path(@execution_environment), method: :post, class: 'dropdown-item') if policy(@execution_environment).sync_to_runner_management? + li = button_to(t('execution_environments.index.synchronize.button'), sync_to_runner_management_execution_environment_path(@execution_environment), method: :post, class: 'dropdown-item') if policy(@execution_environment).sync_to_runner_management? li = link_to(t('execution_environments.index.shell'), shell_execution_environment_path(@execution_environment), class: 'dropdown-item') if policy(@execution_environment).shell? li = link_to(t('shared.statistics'), statistics_execution_environment_path(@execution_environment), 'data-turbo': 'false', class: 'dropdown-item') if policy(@execution_environment).statistics? - li = link_to(t('shared.destroy'), @execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(@execution_environment).destroy? + li = button_to(t('shared.destroy'), @execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(@execution_environment).destroy? = row(label: 'execution_environment.name', value: @execution_environment.name) = row(label: 'execution_environment.user', value: link_to_if(policy(@execution_environment.author).show?, @execution_environment.author, @execution_environment.author)) diff --git a/app/views/exercise_collections/index.html.slim b/app/views/exercise_collections/index.html.slim index f9e4f16b4..6270a5728 100644 --- a/app/views/exercise_collections/index.html.slim +++ b/app/views/exercise_collections/index.html.slim @@ -19,7 +19,7 @@ h1 = ExerciseCollection.model_name.human(count: :other) td = link_to(t('shared.show'), collection) if policy(collection).show? td = link_to(t('shared.edit'), edit_exercise_collection_path(collection)) if policy(collection).edit? td = link_to(t('shared.statistics'), statistics_exercise_collection_path(collection)) if policy(collection).statistics? - td = link_to(t('shared.destroy'), collection, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(collection).destroy? + td = button_to(t('shared.destroy'), collection, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(collection).destroy? = render('shared/pagination', collection: @exercise_collections) p = render('shared/new_button', model: ExerciseCollection) diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index cf140e160..3172342d7 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -51,8 +51,8 @@ h1 = Exercise.model_name.human(count: :other) li = link_to(UserExerciseFeedback.model_name.human(count: :other), feedback_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).feedback? li = link_to(RequestForComment.model_name.human(count: :other), exercise_request_for_comments_path(exercise), class: 'dropdown-item') if policy(exercise).rfcs_for_exercise? li = link_to(ProgrammingGroup.model_name.human(count: :other), exercise_programming_groups_path(exercise), class: 'dropdown-item') if policy(exercise).programming_groups_for_exercise? - li = link_to(t('shared.destroy'), exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(exercise).destroy? - li = link_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(exercise).clone? + li = button_to(t('shared.destroy'), exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(exercise).destroy? + li = button_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(exercise).clone? li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start', data: {'exercise-id': exercise.id, 'bs-toggle': 'modal', 'bs-target': '#transfer-modal'}) if policy(exercise).export_external_confirm? li = link_to(t('exercises.download_proforma.label'), download_proforma_exercise_path(exercise), class: 'dropdown-item', target: '_blank', rel: 'noopener noreferrer') if policy(exercise).download_proforma? diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index 05e58f84c..77ca9edf1 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -20,8 +20,8 @@ h1.d-inline-block li = link_to(UserExerciseFeedback.model_name.human(count: :other), feedback_exercise_path(@exercise), class: 'dropdown-item') if policy(@exercise).feedback? li = link_to(RequestForComment.model_name.human(count: :other), exercise_request_for_comments_path(@exercise), class: 'dropdown-item') if policy(@exercise).rfcs_for_exercise? li = link_to(ProgrammingGroup.model_name.human(count: :other), exercise_programming_groups_path(@exercise), class: 'dropdown-item') if policy(@exercise).programming_groups_for_exercise? - li = link_to(t('shared.destroy'), @exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(@exercise).destroy? - li = link_to(t('exercises.index.clone'), clone_exercise_path(@exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(@exercise).clone? + li = button_to(t('shared.destroy'), @exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(@exercise).destroy? + li = button_to(t('exercises.index.clone'), clone_exercise_path(@exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(@exercise).clone? li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start', data: {'exercise-id': @exercise.id, 'bs-toggle': 'modal', 'bs-target': '#transfer-modal'}) if policy(@exercise).export_external_confirm? li = link_to(t('exercises.download_proforma.label'), download_proforma_exercise_path(@exercise), class: 'dropdown-item', target: '_blank', rel: 'noopener noreferrer') if policy(@exercise).download_proforma? @@ -62,7 +62,7 @@ ul.list-unstyled#files .card-collapse.collapse class="collapse#{file.id}" role='tabpanel' .card-body - if policy(file).destroy? - .clearfix = link_to(t('shared.destroy'), file, class: 'btn btn-warning btn-sm float-end', data: {confirm: t('shared.confirm_destroy')}, method: :delete) + .clearfix = button_to(t('shared.destroy'), file, class: 'btn btn-warning btn-sm float-end', data: {confirm: t('shared.confirm_destroy')}, method: :delete) = render('shared/file', file:) - if policy(@exercise).export_external_confirm? diff --git a/app/views/external_users/show.html.slim b/app/views/external_users/show.html.slim index 9579bd967..21db379dd 100644 --- a/app/views/external_users/show.html.slim +++ b/app/views/external_users/show.html.slim @@ -23,7 +23,7 @@ h1 = @user.displayname - if study_group_membership.study_group_id == current_user.current_study_group_id span.text-success =< t('users.show.current_study_group') - else - =< link_to(t('users.show.set_as_current_study_group'), set_as_current_study_group_path(study_group_membership.study_group), method: :post, class: 'text-body-secondary') + =< button_to(t('users.show.set_as_current_study_group'), set_as_current_study_group_path(study_group_membership.study_group), method: :post, class: 'btn btn-sm btn-link text-body-secondary') - elsif @user == current_user && study_group_membership.role_teacher? && study_group_membership.study_group_id == current_user.current_study_group_id | , span.text-success =< t('users.show.current_study_group') diff --git a/app/views/file_templates/index.html.slim b/app/views/file_templates/index.html.slim index 21ecb7b5f..4efda205c 100644 --- a/app/views/file_templates/index.html.slim +++ b/app/views/file_templates/index.html.slim @@ -14,7 +14,7 @@ h1 = FileTemplate.model_name.human(count: :other) td = link_to_if(policy(file_template.file_type).show?, file_template.file_type, file_type_path(file_template.file_type)) td = link_to(t('shared.show'), file_template) if policy(file_template).show? td = link_to(t('shared.edit'), edit_file_template_path(file_template)) if policy(file_template).edit? - td = link_to(t('shared.destroy'), file_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(file_template).destroy? + td = button_to(t('shared.destroy'), file_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(file_template).destroy? = render('shared/pagination', collection: @file_templates) p = render('shared/new_button', model: FileTemplate) diff --git a/app/views/file_types/index.html.slim b/app/views/file_types/index.html.slim index 7128ca576..d47d8e4e8 100644 --- a/app/views/file_types/index.html.slim +++ b/app/views/file_types/index.html.slim @@ -16,7 +16,7 @@ h1 = FileType.model_name.human(count: :other) td = file_type.file_extension td = link_to(t('shared.show'), file_type) if policy(file_type).show? td = link_to(t('shared.edit'), edit_file_type_path(file_type)) if policy(file_type).edit? - td = link_to(t('shared.destroy'), file_type, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(file_type).destroy? + td = button_to(t('shared.destroy'), file_type, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(file_type).destroy? = render('shared/pagination', collection: @file_types) p = render('shared/new_button', model: FileType) diff --git a/app/views/internal_users/index.html.slim b/app/views/internal_users/index.html.slim index a5aec1d0a..6cf368cc7 100644 --- a/app/views/internal_users/index.html.slim +++ b/app/views/internal_users/index.html.slim @@ -31,7 +31,7 @@ h1 = InternalUser.model_name.human(count: :other) td = symbol_for(user.webauthn_configured?) td = link_to(t('shared.show'), user) if policy(user).show? td = link_to(t('shared.edit'), edit_internal_user_path(user)) if policy(user).edit? - td = link_to(t('shared.destroy'), user, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(user).destroy? + td = button_to(t('shared.destroy'), user, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(user).destroy? = render('shared/pagination', collection: @users) p = render('shared/new_button', model: InternalUser) diff --git a/app/views/internal_users/show.html.slim b/app/views/internal_users/show.html.slim index a77ff7e90..1f96960c9 100644 --- a/app/views/internal_users/show.html.slim +++ b/app/views/internal_users/show.html.slim @@ -23,7 +23,7 @@ h1 - if study_group_membership.study_group_id == current_user.current_study_group_id span.text-success =< t('users.show.current_study_group') - else - =< link_to(t('users.show.set_as_current_study_group'), set_as_current_study_group_path(study_group_membership.study_group), method: :post, class: 'text-body-secondary') + =< button_to(t('users.show.set_as_current_study_group'), set_as_current_study_group_path(study_group_membership.study_group), method: :post, class: 'btn btn-sm btn-link text-body-secondary') | ) - else = t('users.show.no_groups') diff --git a/app/views/programming_groups/index.html.slim b/app/views/programming_groups/index.html.slim index 7a0ca76f7..ab8c6f6b0 100644 --- a/app/views/programming_groups/index.html.slim +++ b/app/views/programming_groups/index.html.slim @@ -36,5 +36,5 @@ td = l(programming_group.created_at, format: :short) td = link_to(t('shared.show'), [@exercise, programming_group]) if policy(programming_group).show? td = link_to(t('shared.edit'), polymorphic_path([@exercise, programming_group], action: :edit)) if policy(programming_group).edit? - td = link_to(t('shared.destroy'), [@exercise, programming_group], data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(programming_group).destroy? + td = button_to(t('shared.destroy'), [@exercise, programming_group], data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(programming_group).destroy? = render('shared/pagination', collection: @programming_groups) diff --git a/app/views/proxy_exercises/index.html.slim b/app/views/proxy_exercises/index.html.slim index b29ec2088..894506123 100644 --- a/app/views/proxy_exercises/index.html.slim +++ b/app/views/proxy_exercises/index.html.slim @@ -32,8 +32,8 @@ h1 = ProxyExercise.model_name.human(count: :other) span.visually-hidden Toggle Dropdown ul.dropdown-menu.float-end role='menu' li = link_to(t('shared.show'), proxy_exercise, class: 'dropdown-item') if policy(proxy_exercise).show? - li = link_to(t('shared.destroy'), proxy_exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(proxy_exercise).destroy? - li = link_to(t('.clone'), clone_proxy_exercise_path(proxy_exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(proxy_exercise).clone? + li = button_to(t('shared.destroy'), proxy_exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(proxy_exercise).destroy? + li = button_to(t('.clone'), clone_proxy_exercise_path(proxy_exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(proxy_exercise).clone? = render('shared/pagination', collection: @proxy_exercises) p = render('shared/new_button', model: ProxyExercise) diff --git a/app/views/request_for_comments/_admin_menu.html.slim b/app/views/request_for_comments/_admin_menu.html.slim index e22a4677e..d8ead67e9 100644 --- a/app/views/request_for_comments/_admin_menu.html.slim +++ b/app/views/request_for_comments/_admin_menu.html.slim @@ -1,7 +1,7 @@ hr h5.mt-4 Admin Menu ul.text - li = link_to 'Clear question text (in case of explicit text)', clear_question_request_for_comment_path(id: @request_for_comment.id), method: :post if policy(@request_for_comment).clear_question? + li = button_to('Clear question text (in case of explicit text)', clear_question_request_for_comment_path(id: @request_for_comment.id), method: :post, class: 'btn btn-sm btn-link') if policy(@request_for_comment).clear_question? li = link_to "User's current status of this exercise", statistics_external_user_exercise_path(id: @request_for_comment.exercise_id, external_user_id: @request_for_comment.user_id) if policy(@request_for_comment.exercise).statistics? li = link_to 'All exercises of this user', statistics_external_user_path(id: @request_for_comment.user_id) if policy(@request_for_comment.user).statistics? ul.text diff --git a/app/views/study_groups/_table.html.slim b/app/views/study_groups/_table.html.slim index be0aee883..773b777e4 100644 --- a/app/views/study_groups/_table.html.slim +++ b/app/views/study_groups/_table.html.slim @@ -17,4 +17,4 @@ td = group.study_group_memberships.size td = link_to(t('shared.show'), group) if policy(group).show? td = link_to(t('shared.edit'), edit_study_group_path(group)) if policy(group).edit? - td = link_to(t('shared.destroy'), group, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(group).destroy? + td = button_to(t('shared.destroy'), group, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(group).destroy? diff --git a/app/views/tags/index.html.slim b/app/views/tags/index.html.slim index 5463e98b5..37861f90e 100644 --- a/app/views/tags/index.html.slim +++ b/app/views/tags/index.html.slim @@ -12,7 +12,7 @@ h1 = Tag.model_name.human(count: :other) td = link_to_if(policy(tag).show?, tag.name, tag) td = link_to(t('shared.show'), tag) if policy(tag).show? td = link_to(t('shared.edit'), edit_tag_path(tag)) if policy(tag).edit? - td = link_to(t('shared.destroy'), tag, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if tag.can_be_destroyed? && policy(tag).destroy? + td = button_to(t('shared.destroy'), tag, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if tag.can_be_destroyed? && policy(tag).destroy? = render('shared/pagination', collection: @tags) p = render('shared/new_button', model: Tag, path: new_tag_path) diff --git a/app/views/tips/index.html.slim b/app/views/tips/index.html.slim index 8908d4288..2865467c0 100644 --- a/app/views/tips/index.html.slim +++ b/app/views/tips/index.html.slim @@ -14,7 +14,7 @@ h1 = Tip.model_name.human(count: :other) td = tip.file_type ? link_to_if(policy(tip.file_type).show?, tip.file_type.name, tip.file_type) : '' td = link_to(t('shared.show'), tip) if policy(tip).show? td = link_to(t('shared.edit'), edit_tip_path(tip)) if policy(tip).edit? - td = link_to(t('shared.destroy'), tip, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if tip.can_be_destroyed? && policy(tip).destroy? + td = button_to(t('shared.destroy'), tip, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if tip.can_be_destroyed? && policy(tip).destroy? = render('shared/pagination', collection: @tips) p = render('shared/new_button', model: Tip, path: new_tip_path) diff --git a/app/views/webauthn_credential_authentication/_form.html.slim b/app/views/webauthn_credential_authentication/_form.html.slim index f4dbf7ebf..f97c8adb5 100644 --- a/app/views/webauthn_credential_authentication/_form.html.slim +++ b/app/views/webauthn_credential_authentication/_form.html.slim @@ -3,4 +3,4 @@ = hidden_field_tag('webauthn_credential[credential]', '') .actions.mb-0 = submit_tag(t('.verify_identity'), class: 'btn btn-primary me-2 mb-2') - = link_to(t('.cancel'), sign_out_path, method: :delete, class: 'btn btn-outline-secondary mb-2') + = button_to(t('.cancel'), sign_out_path, method: :delete, class: 'btn btn-outline-secondary mb-2') diff --git a/app/views/webauthn_credentials/_list.html.slim b/app/views/webauthn_credentials/_list.html.slim index cdc393aac..ad1259e87 100644 --- a/app/views/webauthn_credentials/_list.html.slim +++ b/app/views/webauthn_credentials/_list.html.slim @@ -20,6 +20,6 @@ h4.mt-4 = WebauthnCredential.model_name.human(count: :other) = empty td = link_to(t('shared.show'), [@user, webauthn_credential]) if policy(webauthn_credential).show? td = link_to(t('shared.edit'), polymorphic_path([@user, webauthn_credential], action: :edit)) if policy(webauthn_credential).edit? - td = link_to(t('shared.destroy'), [@user, webauthn_credential], data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(webauthn_credential).destroy? + td = button_to(t('shared.destroy'), [@user, webauthn_credential], data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'btn btn-sm btn-link') if policy(webauthn_credential).destroy? p = render('shared/new_button', model: WebauthnCredential, path: polymorphic_path([@user, WebauthnCredential], action: :new)) From 9b6905a1cffd671f6ddb7fa1f7d672f9b01835f0 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 8 Jul 2025 12:31:11 +0200 Subject: [PATCH 06/19] Ensure consistent appearance of `btn-link` After migrating `link_to` to `button_to` (in the previous commit), we need to adjust the styles for a uniform and correct appearance. --- app/assets/stylesheets/base.css.scss | 2 +- app/assets/stylesheets/editor.css.scss | 10 +++++++--- app/javascript/stylesheets.scss | 9 +++++++++ app/views/execution_environments/shell.html.slim | 2 +- app/views/exercises/_editor.html.slim | 4 ++-- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/base.css.scss b/app/assets/stylesheets/base.css.scss index 5a458ab48..b1aa92b03 100644 --- a/app/assets/stylesheets/base.css.scss +++ b/app/assets/stylesheets/base.css.scss @@ -51,7 +51,7 @@ span.caret { margin-left: 0.5em; } -.btn { +.btn:not(.btn-link) { -webkit-font-smoothing: antialiased; font-weight: 500; :not(.btn-lg) { diff --git a/app/assets/stylesheets/editor.css.scss b/app/assets/stylesheets/editor.css.scss index 224eba2cc..202a6324a 100644 --- a/app/assets/stylesheets/editor.css.scss +++ b/app/assets/stylesheets/editor.css.scss @@ -96,15 +96,19 @@ html[data-bs-theme="light"] { display: none; } -#statusbar{ +#statusbar { visibility: hidden; margin-top: .2em; height: 1.6em; color: var(--bs-tertiary-color); - font-size: 0.8em; + font-size: 0.8rem; } -#turtlecanvas{ +.btn-statusbar { + --bs-btn-font-size: 0.8rem; +} + +#turtlecanvas { border-style:solid; border-width:thin; display: block; diff --git a/app/javascript/stylesheets.scss b/app/javascript/stylesheets.scss index c20da6212..c07932e78 100644 --- a/app/javascript/stylesheets.scss +++ b/app/javascript/stylesheets.scss @@ -40,6 +40,15 @@ html[data-bs-theme="light"] { } } + +// Match the button styles for .btn-link to regular links. +.btn-link { + @extend .p-0; + @extend .border-0; + @extend .align-baseline; +} + + $fa-font-path: '@fortawesome/fontawesome-free/webfonts/'; @import '@fortawesome/fontawesome-free/scss/fontawesome'; @import '@fortawesome/fontawesome-free/scss/solid'; diff --git a/app/views/execution_environments/shell.html.slim b/app/views/execution_environments/shell.html.slim index 33d843e81..561b945cf 100644 --- a/app/views/execution_environments/shell.html.slim +++ b/app/views/execution_environments/shell.html.slim @@ -20,7 +20,7 @@ h1 = @execution_environment .card-body.pt-0.pe-0.ps-1.pb-1 #download-file-tree.justify-content-center.d-flex.my-3 span.mx-1 = t('execution_environments.shell.file_tree.empty') - button#reload-now-link.btn.btn-link.p-0.m-0.border-0 = t('execution_environments.shell.file_tree.list_now') + button#reload-now-link.btn.btn-link = t('execution_environments.shell.file_tree.list_now') - unless @execution_environment.privileged_execution? .card-footer.justify-content-center.align-items-center.d-flex.text-body-secondary i.fa-solid.fa-info diff --git a/app/views/exercises/_editor.html.slim b/app/views/exercises/_editor.html.slim index 494a53702..3beec6435 100644 --- a/app/views/exercises/_editor.html.slim +++ b/app/views/exercises/_editor.html.slim @@ -33,7 +33,7 @@ #statusbar.d-flex.justify-content-between div - if !@embed_options[:disable_download] && @exercise.hide_file_tree? - button#download.p-0.border-0.btn-link.visible.bg-body.text-primary + button#download.btn.btn-link.btn-statusbar.visible i.fa-solid.fa-arrow-down = t('exercises.editor.download') @@ -55,7 +55,7 @@ = ' | ' - button#start-over-active-file.p-0.border-0.btn-link.bg-body.text-primary data-message-confirm=t('exercises.editor.confirm_start_over_active_file') data-url=reload_exercise_path(@exercise) + button#start-over-active-file.btn.btn-link.btn-statusbar data-message-confirm=t('exercises.editor.confirm_start_over_active_file') data-url=reload_exercise_path(@exercise) i.fa-solid.fa-circle-notch.fa-spin.d-none i.fa-solid.fa-clock-rotate-left = t('exercises.editor.start_over_active_file') From 6ab3b4e67bbed5dd6c2fb169965b5e1d18fba94a Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 8 Jul 2025 12:16:08 +0200 Subject: [PATCH 07/19] Quick Fix: Increase margin in calculateEditorHeight In some cases, the left sidebar was too high, causing an undesired scroll bar to be displayed. We increase the magic number to hide the scrollbar in more cases. This is not a proper solution, but enough to hide the scroll bars once again. We need to investigate further. --- app/assets/javascripts/editor/editor.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/editor/editor.js b/app/assets/javascripts/editor/editor.js index 8b9f9e4f4..70407d8d8 100644 --- a/app/assets/javascripts/editor/editor.js +++ b/app/assets/javascripts/editor/editor.js @@ -242,8 +242,8 @@ var CodeOceanEditor = { } const bottom = considerStatusbar ? ($('#statusbar').height() || 0) : 0; - // calculate needed size: window height - position of top of ACE editor - height of autosave label below editor - 5 for bar margins - return window.innerHeight - jqueryElement.offset().top - bottom - 5; + // calculate needed size: window height - position of top of ACE editor - height of autosave label below editor - 7 for bar margins + return window.innerHeight - jqueryElement.offset().top - bottom - 7; }, resizeParentOfAceEditor: function (element) { From 70f9275934423b7e0f40400bad510a198d12632d Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 29 Jun 2025 20:04:39 +0200 Subject: [PATCH 08/19] Re-initialize JavaScript elements on failed form submission When the form is submitted but no redirect occurs (e.g., after a validation error), the page content is simply replaced. However, for this occasion Turbo misses to emit a new `turbo:load` event, which makes it more difficult to use custom JavaScript waiting on page initializations. Potentially, we could use `turbo:render` here to fix these occasions. --- app/javascript/turbo-migration.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/app/javascript/turbo-migration.js b/app/javascript/turbo-migration.js index abe5b2f89..5498c40b9 100644 --- a/app/javascript/turbo-migration.js +++ b/app/javascript/turbo-migration.js @@ -11,6 +11,7 @@ let sprocketsLoaded = false; const sprocketsLoadQueue = []; +const turboRenderQueue = []; document.addEventListener('turbo:load', (event) => { sprocketsLoaded ? forwardTurboLoad(event) : sprocketsLoadQueue.push(event); @@ -22,6 +23,25 @@ document.addEventListener('sprockets:load', () => { flushQueue(sprocketsLoadQueue); }); +// Handle failed form submissions by waiting for `turbo:render` events +document.addEventListener('turbo:submit-end', (event) => { + if (!event.detail.success) { + // If the form submission was _not_ successful, we need to re-initialize JavaScript elements. + // This is necessary since Turbo does not dispatch a `turbo:load` event in this case. + turboRenderQueue.push(event); + } +}); + +document.addEventListener('turbo:render', () => { + if (sprocketsLoaded) { + flushQueue(turboRenderQueue); + } else { + // In the unlikely case that Sprockets isn't ready yet, we queue the events. + sprocketsLoadQueue.push(...turboRenderQueue); + turboRenderQueue.length = 0; + } +}); + function forwardTurboLoad(event) { requestAnimationFrame(() => { const delayedEvent = new CustomEvent('turbo-migration:load', { detail: { ...event.detail } }); From 97eebd49e5a6be6352b4da2f0abc07b249bbe166 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 29 Jun 2025 20:06:51 +0200 Subject: [PATCH 09/19] Disable Turbo for form submissions with custom packs With Turbo enabled on forms with custom JavaScript / CSS packs and a successful change (no validation error), the final page would be rendered twice: The first rendering would occur after sending the form data using Turbo. This response is fetched and potentially displayed (first visit). The resulting DOM contains a change in the packs (with `'data-turbo-track': 'reload'`), causing a page reload (without Turbo). This is the second visit. Through this "double-visit", any flash message intended to be shown after the form submission is simply "lost". Therefore, we disable Turbo in these cases. This holds also true for signout actions, that can (potentially) originate from any page, including those with custom packs, and redirect to the root page (without any additional packs). --- app/views/application/_session.html.slim | 6 +++--- app/views/execution_environments/_form.html.slim | 2 +- app/views/exercises/_form.html.slim | 2 +- app/views/exercises/index.html.slim | 2 +- app/views/proxy_exercises/_form.html.slim | 2 +- app/views/tips/_form.html.slim | 2 +- .../webauthn_credential_authentication/_form.html.slim | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/views/application/_session.html.slim b/app/views/application/_session.html.slim index ecfa91c90..b05455f88 100644 --- a/app/views/application/_session.html.slim +++ b/app/views/application/_session.html.slim @@ -17,11 +17,11 @@ - if current_user.admin? || current_user.teacher? || current_user.internal_user? li = link_to(t('consumers.show.link'), current_user.consumer, class: 'dropdown-item') if current_user.consumer && policy(current_user.consumer).show? li = link_to(t('internal_users.show.link'), current_user, class: 'dropdown-item') if policy(current_user).show? - li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') + li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item', data: {turbo: false}) - elsif current_user.webauthn_configured? - li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') + li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item', data: {turbo: false}) - else - li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item') + li = button_to(t('sessions.destroy.link'), sign_out_path, method: :delete, class: 'dropdown-item', data: {turbo: false}) - else li.nav-item = link_to(sign_in_path, class: 'nav-link') do i.fa-solid.fa-arrow-right-to-bracket diff --git a/app/views/execution_environments/_form.html.slim b/app/views/execution_environments/_form.html.slim index 24267a4a1..065ff909a 100644 --- a/app/views/execution_environments/_form.html.slim +++ b/app/views/execution_environments/_form.html.slim @@ -5,7 +5,7 @@ - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') -= form_for(@execution_environment, builder: MarkdownFormBuilder) do |f| += form_for(@execution_environment, builder: MarkdownFormBuilder, data: {turbo: false}) do |f| = render('shared/form_errors', object: @execution_environment) .mb-3 = f.label(:name, class: 'form-label') diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index 7cce42f29..f8d0563dd 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -10,7 +10,7 @@ - execution_environments = ExecutionEnvironment.where.not(file_type_id: nil).select(:file_type_id, :id) - file_types = FileType.where.not(file_extension: nil).select(:file_extension, :id) -= form_for(@exercise, data: {execution_environments:, file_types:}, multipart: true, builder: MarkdownFormBuilder) do |f| += form_for(@exercise, data: {execution_environments:, file_types:, turbo: false}, multipart: true, builder: MarkdownFormBuilder) do |f| = render('shared/form_errors', object: @exercise) .mb-3 = f.label(:title, class: 'form-label') diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index 3172342d7..0f91886ca 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -52,7 +52,7 @@ h1 = Exercise.model_name.human(count: :other) li = link_to(RequestForComment.model_name.human(count: :other), exercise_request_for_comments_path(exercise), class: 'dropdown-item') if policy(exercise).rfcs_for_exercise? li = link_to(ProgrammingGroup.model_name.human(count: :other), exercise_programming_groups_path(exercise), class: 'dropdown-item') if policy(exercise).programming_groups_for_exercise? li = button_to(t('shared.destroy'), exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(exercise).destroy? - li = button_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(exercise).clone? + li = button_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy'), turbo: false}, method: :post, class: 'dropdown-item') if policy(exercise).clone? li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start', data: {'exercise-id': exercise.id, 'bs-toggle': 'modal', 'bs-target': '#transfer-modal'}) if policy(exercise).export_external_confirm? li = link_to(t('exercises.download_proforma.label'), download_proforma_exercise_path(exercise), class: 'dropdown-item', target: '_blank', rel: 'noopener noreferrer') if policy(exercise).download_proforma? diff --git a/app/views/proxy_exercises/_form.html.slim b/app/views/proxy_exercises/_form.html.slim index 269fb2fb5..8937ead84 100644 --- a/app/views/proxy_exercises/_form.html.slim +++ b/app/views/proxy_exercises/_form.html.slim @@ -5,7 +5,7 @@ - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') -= form_for(@proxy_exercise, multipart: true, builder: MarkdownFormBuilder) do |f| += form_for(@proxy_exercise, multipart: true, builder: MarkdownFormBuilder, data: {turbo: false}) do |f| = render('shared/form_errors', object: @proxy_exercise) .mb-3 = f.label(:title, class: 'form-label') diff --git a/app/views/tips/_form.html.slim b/app/views/tips/_form.html.slim index 04238c1d4..c5044972e 100644 --- a/app/views/tips/_form.html.slim +++ b/app/views/tips/_form.html.slim @@ -6,7 +6,7 @@ - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') -= form_for(@tip, builder: MarkdownFormBuilder) do |f| += form_for(@tip, builder: MarkdownFormBuilder, data: {turbo: false}) do |f| = render('shared/form_errors', object: @tip) .mb-3 = f.label(:title, class: 'form-label') diff --git a/app/views/webauthn_credential_authentication/_form.html.slim b/app/views/webauthn_credential_authentication/_form.html.slim index f97c8adb5..568021b71 100644 --- a/app/views/webauthn_credential_authentication/_form.html.slim +++ b/app/views/webauthn_credential_authentication/_form.html.slim @@ -3,4 +3,4 @@ = hidden_field_tag('webauthn_credential[credential]', '') .actions.mb-0 = submit_tag(t('.verify_identity'), class: 'btn btn-primary me-2 mb-2') - = button_to(t('.cancel'), sign_out_path, method: :delete, class: 'btn btn-outline-secondary mb-2') + = button_to(t('.cancel'), sign_out_path, method: :delete, class: 'btn btn-outline-secondary mb-2', data: {turbo: false}) From b8d2a5b1e77155b6d227d7a0c8aeaa0287498fab Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 29 Jun 2025 19:49:00 +0200 Subject: [PATCH 10/19] Remove meta name='turbo-visit-control' content='reload' --- app/views/admin/dashboard/show.html.slim | 3 --- app/views/community_solutions/edit.html.slim | 5 ----- app/views/execution_environments/_form.html.slim | 3 --- app/views/exercises/_form.html.slim | 3 --- app/views/exercises/_tips_content.html.slim | 3 --- app/views/exercises/show.html.slim | 3 --- app/views/exercises/statistics.html.slim | 4 +--- app/views/exercises/study_group_dashboard.html.slim | 3 --- app/views/proxy_exercises/_form.html.slim | 3 --- app/views/statistics/activity_history.html.slim | 3 --- app/views/statistics/graphs.html.slim | 3 --- app/views/submissions/show.html.slim | 3 --- app/views/tips/_form.html.slim | 3 --- app/views/tips/show.html.slim | 3 --- app/views/webauthn_credential_authentication/new.html.slim | 3 --- app/views/webauthn_credentials/new.html.slim | 3 --- 16 files changed, 1 insertion(+), 50 deletions(-) diff --git a/app/views/admin/dashboard/show.html.slim b/app/views/admin/dashboard/show.html.slim index a9b1daf8c..f0f60c17a 100644 --- a/app/views/admin/dashboard/show.html.slim +++ b/app/views/admin/dashboard/show.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('vis') - append_stylesheet_pack_tag('vis') diff --git a/app/views/community_solutions/edit.html.slim b/app/views/community_solutions/edit.html.slim index 703bba84f..fbedb45b4 100644 --- a/app/views/community_solutions/edit.html.slim +++ b/app/views/community_solutions/edit.html.slim @@ -1,6 +1 @@ -- content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - == render 'form' diff --git a/app/views/execution_environments/_form.html.slim b/app/views/execution_environments/_form.html.slim index 065ff909a..d125d59db 100644 --- a/app/views/execution_environments/_form.html.slim +++ b/app/views/execution_environments/_form.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index f8d0563dd..1cc70308d 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - append_javascript_pack_tag('sortable') - append_javascript_pack_tag('toast-ui') diff --git a/app/views/exercises/_tips_content.html.slim b/app/views/exercises/_tips_content.html.slim index 2d8b38eca..aef2dcab0 100644 --- a/app/views/exercises/_tips_content.html.slim +++ b/app/views/exercises/_tips_content.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') - append_stylesheet_pack_tag("multilang_#{I18n.locale}") diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index 77ca9edf1..d69bf5433 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') diff --git a/app/views/exercises/statistics.html.slim b/app/views/exercises/statistics.html.slim index aae517fbe..90fce6dc2 100644 --- a/app/views/exercises/statistics.html.slim +++ b/app/views/exercises/statistics.html.slim @@ -1,8 +1,6 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('d3-tip') + h1 = @exercise - statistic_base = policy_scope(Submission).where(exercise: @exercise).unscope(where: :cause) diff --git a/app/views/exercises/study_group_dashboard.html.slim b/app/views/exercises/study_group_dashboard.html.slim index 832442b16..9eff698d6 100644 --- a/app/views/exercises/study_group_dashboard.html.slim +++ b/app/views/exercises/study_group_dashboard.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('d3-tip') h1 diff --git a/app/views/proxy_exercises/_form.html.slim b/app/views/proxy_exercises/_form.html.slim index 8937ead84..0718fbc4f 100644 --- a/app/views/proxy_exercises/_form.html.slim +++ b/app/views/proxy_exercises/_form.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/statistics/activity_history.html.slim b/app/views/statistics/activity_history.html.slim index 03e5ec03a..1fb120c07 100644 --- a/app/views/statistics/activity_history.html.slim +++ b/app/views/statistics/activity_history.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('vis') - append_stylesheet_pack_tag('vis') diff --git a/app/views/statistics/graphs.html.slim b/app/views/statistics/graphs.html.slim index 97f7946d0..b82cf7d5e 100644 --- a/app/views/statistics/graphs.html.slim +++ b/app/views/statistics/graphs.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('vis') - append_stylesheet_pack_tag('vis') diff --git a/app/views/submissions/show.html.slim b/app/views/submissions/show.html.slim index e2da80929..e5f5406a2 100644 --- a/app/views/submissions/show.html.slim +++ b/app/views/submissions/show.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') diff --git a/app/views/tips/_form.html.slim b/app/views/tips/_form.html.slim index c5044972e..0fcb8a960 100644 --- a/app/views/tips/_form.html.slim +++ b/app/views/tips/_form.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/tips/show.html.slim b/app/views/tips/show.html.slim index 5bce95473..5b4bcf988 100644 --- a/app/views/tips/show.html.slim +++ b/app/views/tips/show.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, code might not be highlighted correctly (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') - append_stylesheet_pack_tag("multilang_#{I18n.locale}") diff --git a/app/views/webauthn_credential_authentication/new.html.slim b/app/views/webauthn_credential_authentication/new.html.slim index bcf78d6a7..8a08bbaf1 100644 --- a/app/views/webauthn_credential_authentication/new.html.slim +++ b/app/views/webauthn_credential_authentication/new.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('webauthn') h1 = t('.headline') diff --git a/app/views/webauthn_credentials/new.html.slim b/app/views/webauthn_credentials/new.html.slim index 15a5a8115..341e9f0cc 100644 --- a/app/views/webauthn_credentials/new.html.slim +++ b/app/views/webauthn_credentials/new.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, the global variable `vis` might be uninitialized in the assets (race condition) - meta name='turbo-visit-control' content='reload' - append_javascript_pack_tag('webauthn') h1 = t('shared.new_model', model: WebauthnCredential.model_name.human) From 351923b29aaed05024ac5d4e0dde4d3c8f7e1d16 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 26 Jun 2025 01:18:02 +0200 Subject: [PATCH 11/19] Move initialization of Tooltips to app/javascript With this new method, Tooltips are correctly initialized and potential loading errors/race conditions are prevented. --- app/assets/javascripts/base.js | 3 --- app/assets/javascripts/editor/editor.js | 5 ----- app/javascript/tooltips.js | 11 +++++++++++ app/javascript/turbo-migration.js | 8 ++++++++ 4 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 app/javascript/tooltips.js diff --git a/app/assets/javascripts/base.js b/app/assets/javascripts/base.js index 5418bac26..7edd7bb07 100644 --- a/app/assets/javascripts/base.js +++ b/app/assets/javascripts/base.js @@ -66,9 +66,6 @@ $(document).on('turbo-migration:load', function() { }); } - // Enable all tooltips - $('[data-bs-toggle="tooltip"]').tooltip(); - // Enable sorttable again, as it is disabled otherwise by Turbo if (sorttable) { sorttable.init.done = false; diff --git a/app/assets/javascripts/editor/editor.js b/app/assets/javascripts/editor/editor.js index 70407d8d8..52d1ed6ef 100644 --- a/app/assets/javascripts/editor/editor.js +++ b/app/assets/javascripts/editor/editor.js @@ -935,10 +935,6 @@ var CodeOceanEditor = { $('#output_sidebar').removeClass('output-col').addClass('output-col-collapsed'); }, - initializeSideBarTooltips: function () { - $('[data-bs-toggle="tooltip"]').tooltip() - }, - initializeDescriptionToggle: function () { $('#exercise-headline').on('click', this.toggleDescriptionCard.bind(this)); $('a#toggle').on('click', this.toggleDescriptionCard.bind(this)); @@ -1096,7 +1092,6 @@ var CodeOceanEditor = { this.initializeSideBarCollapse(); this.initializeOutputBarToggle(); this.initializeDescriptionToggle(); - this.initializeSideBarTooltips(); this.initializeInterventionTimer(); this.initPrompt(); this.renderScore(); diff --git a/app/javascript/tooltips.js b/app/javascript/tooltips.js new file mode 100644 index 000000000..da77fea64 --- /dev/null +++ b/app/javascript/tooltips.js @@ -0,0 +1,11 @@ +let tooltipList = []; + +export function initializeTooltips() { + const tooltipTriggerList = document.querySelectorAll('[data-bs-toggle="tooltip"]'); + tooltipList = [...tooltipTriggerList].map(tooltipTriggerEl => new bootstrap.Tooltip(tooltipTriggerEl)); +} + +export function destroyTooltips() { + tooltipList.map(tooltip => tooltip.dispose()); + tooltipList = []; +} diff --git a/app/javascript/turbo-migration.js b/app/javascript/turbo-migration.js index 5498c40b9..ad57ec6d3 100644 --- a/app/javascript/turbo-migration.js +++ b/app/javascript/turbo-migration.js @@ -1,3 +1,5 @@ +import { initializeTooltips, destroyTooltips } from './tooltips'; + // `turbo:load` is dispatched earlier than the previous `turbolinks:load` event. // This is causing issues for our migration, since some assets are not fully loaded // when the event is dispatched. To ensure that the DOM content is fully rendered, @@ -46,6 +48,8 @@ function forwardTurboLoad(event) { requestAnimationFrame(() => { const delayedEvent = new CustomEvent('turbo-migration:load', { detail: { ...event.detail } }); document.dispatchEvent(delayedEvent); + + initializeTooltips(); }); } @@ -53,3 +57,7 @@ const flushQueue = (queue) => { queue.forEach(forwardTurboLoad); queue.length = 0; }; + +document.addEventListener('turbo:visit', (event) => { + destroyTooltips(); +}) From 78172171fc12f255d36f55251559bb7226592c8e Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 29 Jun 2025 21:28:38 +0200 Subject: [PATCH 12/19] Enable proper Turbo caching for /implement --- app/assets/javascripts/editor/editor.js | 29 +++++++++++++++++++++---- app/views/exercises/implement.html.slim | 3 --- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/editor/editor.js b/app/assets/javascripts/editor/editor.js index 52d1ed6ef..7bf10ac7e 100644 --- a/app/assets/javascripts/editor/editor.js +++ b/app/assets/javascripts/editor/editor.js @@ -1101,12 +1101,33 @@ var CodeOceanEditor = { this.initializeDeadlines(); CodeOceanEditorTips.initializeEventHandlers(); - window.addEventListener("turbo:visit", App.synchronized_editor?.disconnect.bind(App.synchronized_editor)); - window.addEventListener("beforeunload", App.synchronized_editor?.disconnect.bind(App.synchronized_editor)); + $(document).one("turbo:visit", this.unloadEverything.bind(this, App.synchronized_editor)); + $(window).one("beforeunload", this.unloadEverything.bind(this, App.synchronized_editor)); - window.addEventListener("turbo:visit", this.autosaveIfChanged.bind(this)); - window.addEventListener("beforeunload", this.autosaveIfChanged.bind(this)); // create autosave when the editor is opened the first time this.autosave(); + }, + + unloadEverything: function () { + App.synchronized_editor?.disconnect(); + this.autosaveIfChanged(); + this.cacheEditorContent(); + this.teardownEventHandlers(); + this.destroyEditors(); + }, + + cacheEditorContent: function () { + // Persist the content of the editors in a hidden textarea to enable Turbo caching. + // In this case, we iterate over _all_ editors, not just writable ones. + for (const [file_id, editor] of this.editor_for_file) { + const file_content = editor.getValue(); + const editorContent = $(`.editor-content[data-file-id='${file_id}']`); + editorContent.text(file_content); + } + }, + + destroyEditors: function () { + CodeOceanEditor.editors.forEach(editor => editor.destroy()); + CodeOceanEditor.editors = []; } }; diff --git a/app/views/exercises/implement.html.slim b/app/views/exercises/implement.html.slim index 32a54e5e4..bdc897edb 100644 --- a/app/views/exercises/implement.html.slim +++ b/app/views/exercises/implement.html.slim @@ -1,7 +1,4 @@ - content_for :head do - // Force a full page reload, see https://turbo.hotwired.dev/handbook/drive#ensuring-specific-pages-trigger-a-full-reload. - Otherwise, lti_parameters might be nil - meta name='turbo-cache-control' content='no-cache' - append_stylesheet_pack_tag("multilang_#{I18n.locale}") #editor-column From f607d8b4d835fc0410c333e763e6b889063cae4d Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 1 Jul 2025 22:31:24 +0200 Subject: [PATCH 13/19] Install event listener only once --- app/assets/javascripts/editor.js | 14 +++++++------- lib/assets/javascripts/color_mode_picker.js | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js index 3a8930e59..581f01e0e 100644 --- a/app/assets/javascripts/editor.js +++ b/app/assets/javascripts/editor.js @@ -16,13 +16,13 @@ $(document).on('turbo-migration:load', function(event) { if ($('#editor').isPresent() && CodeOceanEditor && event.detail.url.includes("/implement")) { CodeOceanEditor.initializeEverything(); } +}); - function handleThemeChangeEvent(event) { - if (CodeOceanEditor) { - CodeOceanEditor.THEME = event.detail.currentTheme === 'dark' ? 'ace/theme/tomorrow_night' : 'ace/theme/tomorrow'; - document.dispatchEvent(new Event('theme:change:ace')); - } +function handleThemeChangeEvent(event) { + if (CodeOceanEditor) { + CodeOceanEditor.THEME = event.detail.currentTheme === 'dark' ? 'ace/theme/tomorrow_night' : 'ace/theme/tomorrow'; + document.dispatchEvent(new Event('theme:change:ace')); } +} - $(document).on('theme:change', handleThemeChangeEvent.bind(this)); -}); +$(document).on('theme:change', handleThemeChangeEvent); diff --git a/lib/assets/javascripts/color_mode_picker.js b/lib/assets/javascripts/color_mode_picker.js index 59b9edc81..e14e873c0 100644 --- a/lib/assets/javascripts/color_mode_picker.js +++ b/lib/assets/javascripts/color_mode_picker.js @@ -77,16 +77,9 @@ function showActiveTheme(theme, focus = false) { } } -$(document).on('turbo-migration:load', function() { +$(document).on('turbo:load', function() { setTheme(getPreferredTheme()) - window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', () => { - const storedTheme = getStoredTheme() - if (storedTheme !== 'light' && storedTheme !== 'dark') { - setTheme(getPreferredTheme()) - } - }) - showActiveTheme(getPreferredTheme()) document.querySelectorAll('[data-bs-theme-value]') @@ -99,3 +92,10 @@ $(document).on('turbo-migration:load', function() { }) }) }) + +window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', () => { + const storedTheme = getStoredTheme() + if (storedTheme !== 'light' && storedTheme !== 'dark') { + setTheme(getPreferredTheme()) + } +}) From 552f59fbaaf08d89d2c88d070ff5f558c864ccea Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 2 Jul 2025 10:30:41 +0200 Subject: [PATCH 14/19] Specs: Wait for successful login before navigation Without this change, the new path might be visited before the login succeeded. This causes authorization issues, where another login screen was shown. --- spec/system/editor_system_spec.rb | 1 + spec/system/external_user_statistics_system_spec.rb | 4 ++-- spec/system/request_for_comments_filter_system_spec.rb | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/system/editor_system_spec.rb b/spec/system/editor_system_spec.rb index 11a093061..b3bb25d7d 100644 --- a/spec/system/editor_system_spec.rb +++ b/spec/system/editor_system_spec.rb @@ -30,6 +30,7 @@ fill_in('email', with: contributor.email) fill_in('password', with: attributes_for(:teacher)[:password]) click_button(I18n.t('sessions.new.link')) + has_content?(I18n.t('sessions.create.success')) allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(true) visit(implement_exercise_path(exercise)) end diff --git a/spec/system/external_user_statistics_system_spec.rb b/spec/system/external_user_statistics_system_spec.rb index ea9e7a4eb..a8cfa0b36 100644 --- a/spec/system/external_user_statistics_system_spec.rb +++ b/spec/system/external_user_statistics_system_spec.rb @@ -22,12 +22,12 @@ fill_in('email', with: user.email) fill_in('password', with: password) click_button(I18n.t('sessions.new.link')) - wait_for_ajax + has_content?(I18n.t('sessions.create.success')) allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(true) visit(statistics_external_user_exercise_path(id: exercise.id, external_user_id: learner.id)) end - context 'when a admin accesses the page' do + context 'when an admin accesses the page' do let(:user) { create(:admin, password:) } it 'does display the option to enable autosaves' do diff --git a/spec/system/request_for_comments_filter_system_spec.rb b/spec/system/request_for_comments_filter_system_spec.rb index a4ff2d3d0..d263bcde8 100644 --- a/spec/system/request_for_comments_filter_system_spec.rb +++ b/spec/system/request_for_comments_filter_system_spec.rb @@ -10,6 +10,7 @@ fill_in('email', with: user.email) fill_in('password', with: attributes_for(:teacher)[:password]) click_button(I18n.t('sessions.new.link')) + has_content?(I18n.t('sessions.create.success')) end it 'does not contain rfcs for unpublished exercises' do From 7624cce9372cee0d56705db6c491a4106738494a Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 2 Jul 2025 10:31:11 +0200 Subject: [PATCH 15/19] Specs: Wait for JavaScript actions on /implement The Score system specs are very tricky and error-prone with the current setup. Mostly, this is due to the headless Chrome browser used on CI, which has a weird race condition preventing a regular use. Until said issue is resolved, we add additional expectations (to wait for the page to finish loading). See https://github.com/SeleniumHQ/selenium/issues/15273 --- spec/system/score_system_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/system/score_system_spec.rb b/spec/system/score_system_spec.rb index 6e00579fc..52641efcf 100644 --- a/spec/system/score_system_spec.rb +++ b/spec/system/score_system_spec.rb @@ -84,7 +84,9 @@ allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(lti_outcome_service?) allow(submission).to receive(:calculate_score).and_return(calculate_response) allow_any_instance_of(SubmissionsController).to receive(:send_scores).and_return(scoring_response) + has_content?(exercise.files.visible.first.name_with_extension) # Wait for JsTree to finish loading click_on(I18n.t('exercises.editor.score')) + has_content?(I18n.t('exercises.editor.collapse_output_sidebar')) # Wait for the output sidebar to appear wait_for_ajax wait_for_websocket end @@ -293,7 +295,9 @@ context 'when the desired runner is already in use' do before do allow(submission).to receive(:calculate_score).and_raise(Runner::Error::RunnerInUse) + has_content?(exercise.files.visible.first.name_with_extension) # Wait for JsTree to finish loading click_on(I18n.t('exercises.editor.score')) + has_content?(I18n.t('exercises.editor.collapse_output_sidebar')) # Wait for the output sidebar to appear wait_for_ajax wait_for_websocket end @@ -308,7 +312,9 @@ context 'when no runner is available' do before do allow(submission).to receive(:calculate_score).and_raise(Runner::Error::NotAvailable) + has_content?(exercise.files.visible.first.name_with_extension) # Wait for JsTree to finish loading click_on(I18n.t('exercises.editor.score')) + has_content?(I18n.t('exercises.editor.collapse_output_sidebar')) # Wait for the output sidebar to appear wait_for_ajax wait_for_websocket end From f68936d81a0d9910bd219d620e1225c9bfca82ba Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 2 Jul 2025 10:33:58 +0200 Subject: [PATCH 16/19] JS Performance: Unload and destroy ACE editor on navigation With this change, each editor instance is destroyed upon changing the page. This prevents memory leaks and thus optimizes the performance. The editor will be restored upon visiting the sites again. --- app/assets/javascripts/community_solution.js | 17 +++++++++++++---- app/assets/javascripts/editor/editor.js | 9 +++++++-- app/assets/javascripts/exercises.js | 14 ++++++++++++-- app/assets/javascripts/request_for_comments.js | 14 ++++++++++++++ app/assets/javascripts/submission_statistics.js | 13 ++++++++++++- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/community_solution.js b/app/assets/javascripts/community_solution.js index 1ce46f610..5386a57ca 100644 --- a/app/assets/javascripts/community_solution.js +++ b/app/assets/javascripts/community_solution.js @@ -25,6 +25,18 @@ $(document).on('turbo-migration:load', function() { } }); +$(document).one('turbo-migration:load', function() { + if ($.isController('community_solutions') && $('#community-solution-editor').isPresent()) { + $(document).one('turbo:visit', unloadEditorHandler); + $(window).one('beforeunload', unloadEditorHandler); + } +}); + +function unloadEditorHandler() { + CodeOceanEditor.autosaveIfChanged(); + CodeOceanEditor.unloadEditor(); +} + function submitCode(event) { const button = $(event.target) || $('#submit'); this.newSentryTransaction(button, async () => { @@ -35,10 +47,7 @@ function submitCode(event) { if (!submission) return; if (!submission.redirect) return; - this.autosaveIfChanged(); - await this.stopCode(event); - this.editors = []; - Turbo.cache.clear(); + unloadEditorHandler(); Turbo.visit(submission.redirect); }); } diff --git a/app/assets/javascripts/editor/editor.js b/app/assets/javascripts/editor/editor.js index 7bf10ac7e..88781e588 100644 --- a/app/assets/javascripts/editor/editor.js +++ b/app/assets/javascripts/editor/editor.js @@ -1111,9 +1111,14 @@ var CodeOceanEditor = { unloadEverything: function () { App.synchronized_editor?.disconnect(); this.autosaveIfChanged(); - this.cacheEditorContent(); + this.unloadEditor(); this.teardownEventHandlers(); - this.destroyEditors(); + }, + + unloadEditor: function () { + $(document).off('theme:change:ace'); + CodeOceanEditor.cacheEditorContent(); + CodeOceanEditor.destroyEditors(); }, cacheEditorContent: function () { diff --git a/app/assets/javascripts/exercises.js b/app/assets/javascripts/exercises.js index 21d6959b1..2e0c003f1 100644 --- a/app/assets/javascripts/exercises.js +++ b/app/assets/javascripts/exercises.js @@ -34,6 +34,17 @@ $(document).on('turbo-migration:load', function () { session.setTabSize(element_selector.data('indent-size')); session.setUseSoftTabs(true); session.setUseWrapMode(true); + + function unloadTeacherEditor() { + $(document).off('theme:change:ace'); + const value = editor.getValue(); + editor.destroy(); + // "Restore" the editor's content to the original element for caching. + textarea.val = value; + } + + $(document).one('turbo:visit', unloadTeacherEditor); + $(window).one('beforeunload', unloadTeacherEditor); } const handleAceThemeChangeEvent = function(_event) { @@ -42,12 +53,11 @@ $(document).on('turbo-migration:load', function () { }.bind(this)); }; - $(document).on('theme:change:ace', handleAceThemeChangeEvent.bind(this)); - var initializeEditors = function () { // initialize ace editors for all code textareas in the dom except the last one. The last one is the dummy area for new files, which is cloned when needed. // this one must NOT! be initialized. $('.editor:not(:last)').each(initializeEditor) + $(document).on('theme:change:ace', handleAceThemeChangeEvent.bind(this)); }; var addFileForm = function (event) { diff --git a/app/assets/javascripts/request_for_comments.js b/app/assets/javascripts/request_for_comments.js index 00fa73074..bd55c93fa 100644 --- a/app/assets/javascripts/request_for_comments.js +++ b/app/assets/javascripts/request_for_comments.js @@ -394,4 +394,18 @@ $(document).on('turbo-migration:load', function () { showPermanent: response.status === 422, }); } + + function unloadRfCEditors() { + $(document).off('theme:change:ace'); + $('.editor').each(function (_, editor) { + const aceEditor = ace.edit(editor); + const value = aceEditor.getValue(); + aceEditor.destroy(); + // "Restore" the editor's content to the original element for caching. + editor.textContent = value; + }); + } + + $(document).one('turbo:visit', unloadRfCEditors); + $(window).one('beforeunload', unloadRfCEditors); }); diff --git a/app/assets/javascripts/submission_statistics.js b/app/assets/javascripts/submission_statistics.js index 539f58cec..f651d14de 100644 --- a/app/assets/javascripts/submission_statistics.js +++ b/app/assets/javascripts/submission_statistics.js @@ -110,7 +110,7 @@ $(document).on('turbo-migration:load', function(event) { slider.on('change', onSliderChange); - stopReplay = function() { + const stopReplay = function() { clearInterval(playInterval); playInterval = undefined; playButton.find('span.fa-solid').removeClass('fa-pause').addClass('fa-play') @@ -145,6 +145,17 @@ $(document).on('turbo-migration:load', function(event) { // Start with newest submission slider.val(submissions.length - 1); onSliderChange(); + + const unloadSubmissionStatistics = function() { + if (playInterval) { + stopReplay(); + } + $(document).off('theme:change:ace'); + editor.destroy(); + } + + $(document).on('turbo:visit', unloadSubmissionStatistics); + $(window).on('beforeunload', unloadSubmissionStatistics); } }); From 4e62ffba4060a5a9a8263ba991c7cce4a1c70e66 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 2 Jul 2025 10:34:34 +0200 Subject: [PATCH 17/19] JS Performance: Unload JsTree on navigation Similar to the ACE editor, we unload the JsTree to prevent memory leaks. Also, we use this opportunity to reduce code duplication. --- app/assets/javascripts/editor/editor.js | 28 +++++++++++++++++-- app/assets/javascripts/shell.js | 6 +--- .../javascripts/submission_statistics.js | 6 +--- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/editor/editor.js b/app/assets/javascripts/editor/editor.js index 88781e588..c6c237709 100644 --- a/app/assets/javascripts/editor/editor.js +++ b/app/assets/javascripts/editor/editor.js @@ -418,7 +418,31 @@ var CodeOceanEditor = { this.showFrame(frame); this.toggleButtonStates(); }.bind(this)); - $(document).on('theme:change', function(event) { + + this.installFileTreeEventHandlers(filesInstance); + }, + + installFileTreeEventHandlers: function (filesInstance) { + // Prevent duplicate event listeners by removing them during unload. + const themeListener = this.createFileTreeThemeChangeListener(filesInstance); + const jsTree = filesInstance?.jstree(true); + $(document).on('theme:change', themeListener); + $(document).one('turbo:visit', function() { + $(document).off('theme:change', themeListener); + if (jsTree && jsTree.element) { + jsTree.destroy(true); + } + }); + $(window).one('beforeunload', function() { + $(document).off('theme:change', themeListener); + if (jsTree && jsTree.element) { + jsTree.destroy(true); + } + }); + }, + + createFileTreeThemeChangeListener: function (filesInstance) { + return function (event) { const jsTree = filesInstance?.jstree(true); if (jsTree) { @@ -426,7 +450,7 @@ var CodeOceanEditor = { // Update the JStree theme jsTree?.set_theme(newColorScheme === "dark" ? "default-dark" : "default"); } - }); + } }, initializeFileTreeButtons: function () { diff --git a/app/assets/javascripts/shell.js b/app/assets/javascripts/shell.js index e819d2535..1f74d7c54 100644 --- a/app/assets/javascripts/shell.js +++ b/app/assets/javascripts/shell.js @@ -133,11 +133,7 @@ $(document).on('turbo-migration:load', function () { window.location = downloadPath; } }.bind(this)); - $(document).on('theme:change', function(event) { - const newColorScheme = event.detail.currentTheme; - // Update the JStree theme - fileTree.jstree(true).set_theme(newColorScheme === "dark" ? "default-dark" : "default"); - }); + CodeOceanEditor.installFileTreeEventHandlers(fileTree); } } diff --git a/app/assets/javascripts/submission_statistics.js b/app/assets/javascripts/submission_statistics.js index f651d14de..b3c391b53 100644 --- a/app/assets/javascripts/submission_statistics.js +++ b/app/assets/javascripts/submission_statistics.js @@ -46,11 +46,7 @@ $(document).on('turbo-migration:load', function(event) { }); showActiveFile(); }); - $(document).on('theme:change', function(event) { - const newColorScheme = event.detail.currentTheme; - // Update the JStree theme - fileTree.jstree(true).set_theme(newColorScheme === "dark" ? "default-dark" : "default"); - }); + CodeOceanEditor.installFileTreeEventHandlers(fileTree); fileTrees.push(fileTree); }); }; From 105ff623ed1613da8b1cf445a65b4e3fcd9b862b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 2 Jul 2025 10:14:07 +0200 Subject: [PATCH 18/19] Use Firefox for system specs in GitHub Actions In general, we can run system specs with four browser combinations: 1. Chrome 2. Chrome Headless 3. Firefox 4. Firefox Headless Unfortunately, Selenium has an ongoing issue with Chrome Headless and Alerts, making the test execution slow or causing random failures. Non-headless Chrome works fine. To overcome these issues, we switch to Firefox for the time being in GitHub Actions. Non-headless browsers are not supported in GitHub Actions. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5c168d8d..6c1cef1e1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,9 @@ jobs: env: RAILS_ENV: test CC_TEST_REPORTER_ID: true + # Use Firefox for system tests until Chrome headless works reliably again. + # See https://github.com/SeleniumHQ/selenium/issues/15273 + BROWSER: firefox run: bundle exec rspec --color --format RSpec::Github::Formatter --format progress - name: Upload coverage reports to Codecov From 88b2010661419cb615c4ca998ee10ec4ced57cee Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 2 Jul 2025 10:54:07 +0200 Subject: [PATCH 19/19] Load multilang support globally While loading the multilang support only on the respective pages sounds like a good idea, it complicates matters with the Turbo migration (since we would need to opt-out of some cases). Also, the exercise description is used in too many places, whereas the snippet is rather short and the performance impact to load it globally is rather minimal. Amends 7d4dd2c7 --- app/views/exercises/_form.html.slim | 1 - app/views/exercises/_tips_content.html.slim | 1 - app/views/exercises/implement.html.slim | 3 --- app/views/layouts/application.html.slim | 2 +- app/views/tips/_form.html.slim | 1 - app/views/tips/show.html.slim | 1 - 6 files changed, 1 insertion(+), 8 deletions(-) diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index 1cc70308d..051b2ebfc 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -1,5 +1,4 @@ - content_for :head do - - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - append_javascript_pack_tag('sortable') - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/exercises/_tips_content.html.slim b/app/views/exercises/_tips_content.html.slim index aef2dcab0..77e72851d 100644 --- a/app/views/exercises/_tips_content.html.slim +++ b/app/views/exercises/_tips_content.html.slim @@ -1,7 +1,6 @@ - content_for :head do - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') - - append_stylesheet_pack_tag("multilang_#{I18n.locale}") #tips.card.d-block.overflow-scroll role='tab' .card-header.py-2 diff --git a/app/views/exercises/implement.html.slim b/app/views/exercises/implement.html.slim index bdc897edb..7aadf099c 100644 --- a/app/views/exercises/implement.html.slim +++ b/app/views/exercises/implement.html.slim @@ -1,6 +1,3 @@ -- content_for :head do - - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - #editor-column - unless @embed_options[:hide_exercise_description] .exercise.clearfix diff --git a/app/views/layouts/application.html.slim b/app/views/layouts/application.html.slim index 177eaa091..d1ad6f0a4 100644 --- a/app/views/layouts/application.html.slim +++ b/app/views/layouts/application.html.slim @@ -12,7 +12,7 @@ html lang=I18n.locale data-default-locale=I18n.default_locale = favicon_link_tag('/icon.png', rel: 'apple-touch-icon', type: 'image/png') = tag.link rel: 'manifest', href: pwa_manifest_path = action_cable_meta_tag - = stylesheet_pack_tag('application', 'stylesheets', media: 'all', 'data-turbo-track': 'reload', integrity: true, crossorigin: 'anonymous') + = stylesheet_pack_tag('application', 'stylesheets', "multilang_#{I18n.locale}", media: 'all', 'data-turbo-track': 'reload', integrity: true, crossorigin: 'anonymous') = stylesheet_link_tag('application', media: 'all', 'data-turbo-track': 'reload', integrity: true, crossorigin: 'anonymous') // Since d3-tip is loaded via a separate pack and requires the application pack to be loaded first, we cannot use `defer` here. = javascript_pack_tag('application', 'data-turbo-track': 'reload', defer: false, integrity: true, crossorigin: 'anonymous') diff --git a/app/views/tips/_form.html.slim b/app/views/tips/_form.html.slim index 0fcb8a960..d2de67299 100644 --- a/app/views/tips/_form.html.slim +++ b/app/views/tips/_form.html.slim @@ -1,5 +1,4 @@ - content_for :head do - - append_stylesheet_pack_tag("multilang_#{I18n.locale}") - append_javascript_pack_tag('toast-ui') - append_stylesheet_pack_tag('toast-ui') diff --git a/app/views/tips/show.html.slim b/app/views/tips/show.html.slim index 5b4bcf988..e7339130a 100644 --- a/app/views/tips/show.html.slim +++ b/app/views/tips/show.html.slim @@ -1,7 +1,6 @@ - content_for :head do - append_javascript_pack_tag('highlight') - append_stylesheet_pack_tag('highlight') - - append_stylesheet_pack_tag("multilang_#{I18n.locale}") h1 = @tip.to_s