diff --git a/app/assets/javascripts/components/search/loading_bar.ts b/app/assets/javascripts/components/search/loading_bar.ts index 208f05fac3..a2f70f09de 100644 --- a/app/assets/javascripts/components/search/loading_bar.ts +++ b/app/assets/javascripts/components/search/loading_bar.ts @@ -11,12 +11,17 @@ import { DodonaElement } from "components/meta/dodona_element"; */ @customElement("d-loading-bar") export class LoadingBar extends DodonaElement { - @property({ type: Boolean, state: true }) + @property({ type: Boolean, attribute: "search-based" }) + searchBased = false; + + @property({ type: Boolean }) loading = false; constructor() { super(); - search.loadingBars.push(this); + if (this.searchBased) { + search.loadingBars.push(this); + } } show(): void { diff --git a/app/assets/javascripts/export.ts b/app/assets/javascripts/export.ts index b41076f466..bd7e8a69d8 100644 --- a/app/assets/javascripts/export.ts +++ b/app/assets/javascripts/export.ts @@ -1,4 +1,5 @@ import { i18n } from "i18n/i18n"; +import { Collapse } from "bootstrap"; function initSelection(): void { const selectAll = document.querySelector("#check-all") as HTMLInputElement; @@ -7,23 +8,34 @@ function initSelection(): void { const errorWrapper = document.querySelector("#errors-wrapper"); const choosePanel = document.querySelector("#choose-panel"); - const chooseCollapse = new bootstrap.Collapse(choosePanel.querySelector(".panel-collapse"), { toggle: false }); + const chooseCollapse = new Collapse(choosePanel.querySelector(".panel-collapse"), { toggle: false }); const chooseOptionsPanel = document.querySelector("#choose-options-panel"); const chooseOptionsElement = chooseOptionsPanel.querySelector(".panel-collapse"); - const chooseOptionsCollapse = new bootstrap.Collapse(chooseOptionsElement, { toggle: false }); + const chooseOptionsCollapse = new Collapse(chooseOptionsElement, { toggle: false }); const form = document.querySelector("#download_submissions") as HTMLFormElement; const defaultAction = form.action; + const startDownload = document.getElementById("start-download") as HTMLButtonElement; + const downloadingPanel = document.getElementById("downloading-panel") as HTMLDivElement; + const downloadingCollapse = new Collapse(downloadingPanel.querySelector(".panel-collapse"), { toggle: false }); + function init(): void { initCheckboxes(); initContinueButton(); + initDownloadButton(); choosePanel.querySelector(".panel-collapse").addEventListener("show.bs.collapse", () => { chooseOptionsCollapse.hide(); + downloadingCollapse.hide(); }); chooseOptionsPanel.querySelector(".panel-collapse").addEventListener("show.bs.collapse", () => { chooseCollapse.hide(); + downloadingCollapse.hide(); + }); + downloadingPanel.querySelector(".panel-collapse").addEventListener("show.bs.collapse", () => { + chooseCollapse.hide(); + chooseOptionsCollapse.hide(); }); } @@ -69,20 +81,71 @@ function initSelection(): void { if (formUrl) { errorWrapper.classList.add("hidden"); chooseOptionsPanel.classList.remove("hidden"); // this panel is initially hidden - chooseCollapse.hide(); chooseOptionsCollapse.show(); form.action = formUrl; } else { chooseCollapse.show(); - chooseOptionsCollapse.hide(); errorWrapper.classList.remove("hidden"); document.querySelector("#warning-message-wrapper").innerHTML = i18n.t("js.no_selection"); } })); } + function initDownloadButton(): void { + startDownload.addEventListener("click", async () => { + const data = new FormData(form); + downloadingPanel.classList.remove("hidden"); + downloadingCollapse.show(); + + // disable the collapse steppers + document.querySelectorAll(".panel-heading a").forEach(a => { + a.classList.add("disabled"); + a.attributes.removeNamedItem("data-bs-toggle"); + }); + + const exportDataUrl = await prepareExport(form.action, data); + const downloadUrl = await exportLocation(exportDataUrl); + + // Update the stepper content + downloadingPanel.querySelector(".stepper-part").innerHTML = i18n.t("js.export.ready_html", { url: downloadUrl }); + + window.location.href = downloadUrl; + }); + } + init(); } -export { initSelection }; +/** + * Prepare the download of a file by sending a POST request to the server. + * Returns the URL of the metadata for the file to download. + * @param url The URL of the export endpoint + * @param data The export settings to send to the server + */ +async function prepareExport(url: string, data: FormData): Promise { + const response = await fetch(url, { + method: "POST", + body: data, + headers: { + "Accept": "application/json" + } + }); + const json = await response.json(); + return json.url; +} + +/** + * Returns the url of the blob to download when the download is ready. + * @param url The URL of the download endpoint + */ +async function exportLocation(url: string): Promise { + const response = await fetch(url); + const data = await response.json(); + if (!data.ready) { + await new Promise(resolve => setTimeout(resolve, 1000)); + return await exportLocation(url); + } + return data.url; +} +export { initSelection, prepareExport, exportLocation }; diff --git a/app/assets/javascripts/i18n/translations.json b/app/assets/javascripts/i18n/translations.json index df69044f7e..afb19a9203 100644 --- a/app/assets/javascripts/i18n/translations.json +++ b/app/assets/javascripts/i18n/translations.json @@ -225,6 +225,9 @@ }, "en": "English", "event_types": "Event types", + "export": { + "ready_html": "Export is ready. The download should start automatically. If not, click this link to start the download." + }, "favorite-course-do": "Favorite", "favorite-course-failed": "Favoriting course failed", "favorite-course-succeeded": "Favorited course", @@ -752,6 +755,9 @@ }, "en": "Engels", "event_types": "Event types", + "export": { + "ready_html": "De export is klaar. De download start normaal automatisch. Als dit niet gebeurt, klik dan hier." + }, "favorite-course-do": "Voeg toe aan favorieten", "favorite-course-failed": "Cursus aan favorieten toevoegen mislukt", "favorite-course-succeeded": "Cursus toegevoegd aan favorieten", diff --git a/app/assets/stylesheets/components/btn.css.scss b/app/assets/stylesheets/components/btn.css.scss index 1e3684c8a0..0637169121 100644 --- a/app/assets/stylesheets/components/btn.css.scss +++ b/app/assets/stylesheets/components/btn.css.scss @@ -134,6 +134,7 @@ border-color: var(--d-btn-color); } + &.disabled, &:disabled { color: var(--d-on-surface); border: 1px solid rgba(var(--d-on-surface-rgb), 0.12); diff --git a/app/controllers/exports_controller.rb b/app/controllers/exports_controller.rb index da839c71d2..5096674153 100644 --- a/app/controllers/exports_controller.rb +++ b/app/controllers/exports_controller.rb @@ -4,11 +4,19 @@ class ExportsController < ApplicationController before_action :set_user, only: %i[new_series_export new_course_export create_series_export create_course_export] - def index - authorize Export - @title = I18n.t('exports.index.title') - @highlighted_id = (params[:highlighted] || 0).to_i - @exports = policy_scope(Export) + def show + @export = Export.find(params[:id]) + authorize @export + respond_to do |format| + format.json + format.any(:zip, :html) do + if @export.finished? && @export.archive.attached? + redirect_to rails_blob_path(@export.archive, disposition: 'attachment') + else + head :not_found + end + end + end end def new_series_export @@ -89,13 +97,10 @@ def create(item, list) # Only retain supported options from the received function parameters options = params.permit(Export::Zipper::SUPPORTED_OPTIONS) - Export.create(user: current_user).delay(queue: 'exports').start(item, list, ([@user] if @user), options) + export = Export.create(user: current_user) + export.delay(queue: 'exports').start(item, list, ([@user] if @user), options) respond_to do |format| - format.html do - flash[:notice] = I18n.t('exports.index.export_started') - redirect_to action: 'index' - end - format.json { head :accepted } + format.json { render json: { url: export_path(export) } } end end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index cd9d6c0d4f..b60d2976a8 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,20 +1,17 @@ module NotificationsHelper def base_notifiable_url_params(notification) - return { controller: 'exports', action: 'index' } if notification.notifiable_type == 'Export' return { controller: 'submissions', action: 'show', id: notification.notifiable_id } if notification.notifiable_type == 'Submission' { controller: 'evaluations', action: 'overview', id: notification.notifiable_id } if notification.notifiable_type == 'Evaluation' end def notifiable_url(notification) - return exports_path(highlighted: notification.notifiable_id) if notification.notifiable_type == 'Export' return submission_path(notification.notifiable_id, anchor: 'code') if notification.notifiable_type == 'Submission' overview_evaluation_path(notification.notifiable_id) if notification.notifiable_type == 'Evaluation' end def notifiable_icon(notification) - return 'mdi-file-download-outline' if notification.notifiable_type == 'Export' return 'mdi-comment-account-outline' if notification.notifiable_type == 'Submission' 'mdi-comment-multiple-outline' if notification.notifiable_type == 'Evaluation' diff --git a/app/javascript/packs/export.js b/app/javascript/packs/export.js index 69e75482b0..0cda9bfeaf 100644 --- a/app/javascript/packs/export.js +++ b/app/javascript/packs/export.js @@ -1,3 +1,4 @@ import { initSelection } from "export.ts"; +import "components/search/loading_bar.ts"; window.dodona.initSelection = initSelection; diff --git a/app/models/export.rb b/app/models/export.rb index 97a78b469f..d616e16079 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -12,10 +12,9 @@ class Export < ApplicationRecord include ExportHelper - AUTOMATICALLY_DELETE_AFTER = 30.days + AUTOMATICALLY_DELETE_AFTER = 1.day belongs_to :user - has_one :notification, as: :notifiable, dependent: :destroy has_one_attached :archive enum status: { started: 0, finished: 1, failed: 2 } @@ -38,7 +37,6 @@ def start(item, list, users, params) ) delay(queue: 'cleaning', run_at: AUTOMATICALLY_DELETE_AFTER.from_now).destroy - notification = Notification.new(user: user, message: 'exports.index.ready_for_download') - update(status: :finished, notification: notification) + update(status: :finished) end end diff --git a/app/views/exports/download_submissions.html.erb b/app/views/exports/download_submissions.html.erb index 873aef4473..f50b7d54e6 100644 --- a/app/views/exports/download_submissions.html.erb +++ b/app/views/exports/download_submissions.html.erb @@ -146,7 +146,26 @@ <% end %>
- + +
+ + + + + - +
<%= render partial: "course_cards", locals: {courses: @courses} %> diff --git a/config/locales/js/en.yml b/config/locales/js/en.yml index 92002cf14f..2777714544 100644 --- a/config/locales/js/en.yml +++ b/config/locales/js/en.yml @@ -327,5 +327,7 @@ en: feedbacks: submission: submit: Submit this solution + export: + ready_html: Export is ready. The download should start automatically. If not, click this link to start the download. draft: Draft popularity: Popularity diff --git a/config/locales/js/nl.yml b/config/locales/js/nl.yml index 7118423332..274d261d2d 100644 --- a/config/locales/js/nl.yml +++ b/config/locales/js/nl.yml @@ -327,5 +327,7 @@ nl: feedbacks: submission: submit: Deze oplossing indienen + export: + ready_html: De export is klaar. De download start normaal automatisch. Als dit niet gebeurt, klik dan hier. draft: Concept popularity: Populariteit diff --git a/config/locales/views/exports/en.yml b/config/locales/views/exports/en.yml index bcbea3f50f..319f3e0611 100644 --- a/config/locales/views/exports/en.yml +++ b/config/locales/views/exports/en.yml @@ -46,3 +46,5 @@ en: course: Course series: Series no_series: no-series + download: Download file + downloading: Preparing submissions for download, this might take a couple of minutes. Do not close this page. diff --git a/config/locales/views/exports/nl.yml b/config/locales/views/exports/nl.yml index cd091df658..859aa853d6 100644 --- a/config/locales/views/exports/nl.yml +++ b/config/locales/views/exports/nl.yml @@ -46,3 +46,5 @@ nl: course: Cursus series: Reeks no_series: geen-reeks + download: Bestand downloaden + downloading: Bezig met het voorbereiden van de download, dit kan enkele minuten duren. Sluit deze pagina niet. diff --git a/config/routes.rb b/config/routes.rb index a151851658..7d8d9b2493 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -79,7 +79,7 @@ end end - resources :exports, except: %i[show edit update new destroy create] do + resources :exports, except: %i[edit update new destroy create] do get 'users/:id', on: :collection, to: 'exports#new_user_export', as: 'users' post 'users/:id', on: :collection, to: 'exports#create_user_export' get 'courses/:id', on: :collection, to: 'exports#new_course_export', as: 'courses' diff --git a/db/migrate/20240305141148_remove_export_notifications.rb b/db/migrate/20240305141148_remove_export_notifications.rb new file mode 100644 index 0000000000..8300a79870 --- /dev/null +++ b/db/migrate/20240305141148_remove_export_notifications.rb @@ -0,0 +1,5 @@ +class RemoveExportNotifications < ActiveRecord::Migration[7.1] + def change + Notification.where(notifiable_type: 'Export').destroy_all + end +end diff --git a/db/schema.rb b/db/schema.rb index 61ed82a9c9..a49ca4d527 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_02_26_090515) do +ActiveRecord::Schema[7.1].define(version: 2024_03_05_141148) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false diff --git a/test/controllers/exports_controller_test.rb b/test/controllers/exports_controller_test.rb index 750be0f294..3c51d40307 100644 --- a/test/controllers/exports_controller_test.rb +++ b/test/controllers/exports_controller_test.rb @@ -31,39 +31,32 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest end test 'should download only last submissions' do - post series_exports_path(@series), params: { all: true, only_last_submission: true, with_info: true } + post series_exports_path(@series), params: { all: true, only_last_submission: true, with_info: true, format: :json } - assert_redirected_to exports_path + assert_response :success count = @students.map { |u| @series.exercises.map { |e| e.last_submission(u, @series) } }.flatten.select(&:present?).count assert_zip ActiveStorage::Blob.last.download, with_info: true, solution_count: count, data: @data end - test 'should create notification' do - assert_difference('Notification.count', 1) do - post series_exports_path(@series), params: { all: true, only_last_submission: true, with_info: true } - end - assert_redirected_to exports_path - end - test 'should be grouped by user' do - post series_exports_path(@series), params: { all: true, group_by: 'user' } + post series_exports_path(@series), params: { all: true, group_by: 'user', format: :json } - assert_redirected_to exports_path + assert_response :success assert_zip ActiveStorage::Blob.last.download, group_by: 'user', data: @data end test 'should be grouped by exercise' do - post series_exports_path(@series), params: { all: true, group_by: 'exercise' } + post series_exports_path(@series), params: { all: true, group_by: 'exercise', format: :json } - assert_redirected_to exports_path + assert_response :success assert_zip ActiveStorage::Blob.last.download, group_by: 'exercise', data: @data end test 'should retrieve all submissions' do - post series_exports_path(@series), params: { all: true } + post series_exports_path(@series), params: { all: true, format: :json } - assert_redirected_to exports_path + assert_response :success assert_zip ActiveStorage::Blob.last.download, solution_count: Submission.count, data: @data end @@ -79,25 +72,25 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest end end.flatten.length - post series_exports_path(@series), params: { all: true, filter_students: 'all' } + post series_exports_path(@series), params: { all: true, filter_students: 'all', format: :json } - assert_redirected_to exports_path + assert_response :success assert_zip ActiveStorage::Blob.last.download, solution_count: zip_submission_count, data: @data end test 'zip should only contain submissions before deadline' do @series.update(deadline: 1.year.ago) - post series_exports_path(@series), params: { all: true, deadline: true } + post series_exports_path(@series), params: { all: true, deadline: true, format: :json } - assert_redirected_to exports_path + assert_response :success zip_submission_count = @series.exercises.map { |ex| ex.submissions.before_deadline(@series.deadline) }.flatten.length assert_zip ActiveStorage::Blob.last.download, solution_count: zip_submission_count, data: @data @series.update(deadline: 2.years.from_now) - post series_exports_path(@series), params: { all: true, deadline: true } + post series_exports_path(@series), params: { all: true, deadline: true, format: :json } - assert_redirected_to exports_path + assert_response :success zip_submission_count = @series.exercises.map { |ex| ex.submissions.before_deadline(@series.deadline) }.flatten.length assert_zip ActiveStorage::Blob.last.download, solution_count: zip_submission_count, data: @data @@ -105,7 +98,7 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest test 'should only download from specific exercises' do sample_exercises = @series.exercises.sample(3) - post series_exports_path(@series), params: { selected_ids: sample_exercises.map(&:id), filter_students: 'all' } + post series_exports_path(@series), params: { selected_ids: sample_exercises.map(&:id), filter_students: 'all', format: :json } zip_submission_count = @data[:users].map do |u| sample_exercises.map do |ex| subs = ex.submissions.of_user(u).in_course(@series.course) @@ -127,7 +120,8 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest course: @series.course, with_info: true, group_by: 'exercise', - data: @data } + data: @data, + format: :json } options[:solution_count] = @data[:users].map do |u| sample_exercises.map do |ex| ex.submissions.of_user(u).in_course(@series.course).before_deadline(@series.deadline).limit(1).first @@ -144,11 +138,12 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest only_last_submission: false, data: @data, solution_count: Submission.all.in_course(@course).count, - all: true + all: true, + format: :json } post courses_exports_path(@course), params: options - assert_redirected_to exports_path + assert_response :success options[:group_by] = 'series' assert_zip ActiveStorage::Blob.last.download, options @@ -164,11 +159,12 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest only_last_submission: false, data: @data, solution_count: Submission.all.in_course(@course).count, - all: true + all: true, + format: :json } post courses_exports_path(@course), params: options - assert_redirected_to exports_path + assert_response :success options[:group_by] = 'series' assert_zip ActiveStorage::Blob.last.download, options @@ -186,12 +182,13 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest only_last_submission: false, data: @data, solution_count: Submission.all.in_course(@course).count, - all: true + all: true, + format: :json } sign_in u post courses_exports_path(@course), params: options - assert_redirected_to exports_path + assert_response :success options[:group_by] = 'series' assert_zip ActiveStorage::Blob.last.download, options @@ -206,11 +203,12 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest with_info: true, data: @data, all: true, - solution_count: @course.users.count * @course.series.map(&:exercises).flatten.count + solution_count: @course.users.count * @course.series.map(&:exercises).flatten.count, + format: :json } post courses_exports_path(@course), params: options - assert_redirected_to exports_path + assert_response :success assert_zip ActiveStorage::Blob.last.download, options end @@ -219,7 +217,8 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest group_by: 'exercise', filter_students: 'all', data: @data, - all: true + all: true, + format: :json } options[:solution_count] = @course.series.map do |series| @course.users.map do |user| @@ -239,7 +238,8 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest group_by: 'exercise', filter_students: 'correct', data: @data, - all: true + all: true, + format: :json } options[:solution_count] = @course.series.map do |series| @course.users.map do |user| @@ -263,7 +263,8 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest options = { data: @data, all: true, - solution_count: Submission.all.of_user(student).count + solution_count: Submission.all.of_user(student).count, + format: :json } @data[:user] = student post users_exports_path(student), params: options @@ -278,7 +279,8 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest options = { data: @data, all: true, - solution_count: Submission.all.of_user(other_student).count + solution_count: Submission.all.of_user(other_student).count, + format: :json } post users_exports_path(other_student, format: :json), params: options @@ -295,7 +297,7 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest post courses_exports_path(@course, user_id: @students[0].id, format: :json) - assert_response :accepted + assert_response :success end test 'should not be able to export course if not course member' do @@ -320,7 +322,7 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest post series_exports_path(@course.series.first, user_id: @students[0].id, format: :json) - assert_response :accepted + assert_response :success end test 'should not be able to export series if not course member' do