Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically trigger download of exported files #5418

Merged
merged 20 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions app/assets/javascripts/components/download_button.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { DodonaElement } from "components/meta/dodona_element";
import { html, TemplateResult } from "lit";
import { customElement, property } from "lit/decorators.js";
import "components/search/loading_bar";
import { i18n } from "i18n/i18n";

Check warning on line 5 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L1-L5

Added lines #L1 - L5 were not covered by tests

/**
* This component represents a download button.
* It should be used within a form.
*
* @element d-download-button
*/
@customElement("d-download-button")
export class DownloadButton extends DodonaElement {

Check warning on line 14 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L14

Added line #L14 was not covered by tests
@property({ state: true })
ready = false;

Check warning on line 16 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L16

Added line #L16 was not covered by tests
@property({ state: true })
url: string | undefined = undefined;

Check warning on line 18 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L18

Added line #L18 was not covered by tests

private get form(): HTMLFormElement {
return document.querySelector("#download_submissions") as HTMLFormElement;

Check warning on line 21 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L20-L21

Added lines #L20 - L21 were not covered by tests
}

private get started(): boolean {
return this.url !== undefined;

Check warning on line 25 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L24-L25

Added lines #L24 - L25 were not covered by tests
}

private async prepareDownload(): Promise<void> {
const data = new FormData(this.form);

Check warning on line 29 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L28-L29

Added lines #L28 - L29 were not covered by tests
// disable the form
this.form.querySelectorAll("input, button")
.forEach(e => e.setAttribute("disabled", "true"));
const response = await fetch(this.form.action, {

Check warning on line 33 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L31-L33

Added lines #L31 - L33 were not covered by tests
method: this.form.method,
body: data,
headers: {
"Accept": "application/json"
}
});
const json = await response.json();
this.url = json.url;
this.tryDownload();

Check warning on line 42 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L40-L42

Added lines #L40 - L42 were not covered by tests
}

private async tryDownload(): Promise<void> {

Check warning on line 45 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L45

Added line #L45 was not covered by tests
if (!this.started || this.ready) {
return;

Check warning on line 47 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L47

Added line #L47 was not covered by tests
}

const response = await fetch(this.url);
const data = await response.json();

Check warning on line 51 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L50-L51

Added lines #L50 - L51 were not covered by tests
if (data.ready) {
window.location.href = data.url;
this.ready = true;
} else {
setTimeout(() => this.tryDownload(), 1000);

Check warning on line 56 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L53-L56

Added lines #L53 - L56 were not covered by tests
}
}

render(): TemplateResult {

Check warning on line 60 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L60

Added line #L60 was not covered by tests
if (!this.started) {
return html`
<button @click=${() => this.prepareDownload()} class="btn btn-filled">

Check warning on line 63 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L62-L63

Added lines #L62 - L63 were not covered by tests
${i18n.t("js.download_button.download")}
</button>
`;
} else if (this.ready) {
return html`
<button class="btn btn-filled" @click=${() => window.history.back()}>

Check warning on line 69 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L68-L69

Added lines #L68 - L69 were not covered by tests
${i18n.t("js.download_button.done")}
</button>
`;
} else {
return html`

Check warning on line 74 in app/assets/javascripts/components/download_button.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/download_button.ts#L73-L74

Added lines #L73 - L74 were not covered by tests
<d-loading-bar loading="true"></d-loading-bar>
<p class="help-block">${i18n.t("js.download_button.downloading")}</p>
`;
}
}
}
9 changes: 7 additions & 2 deletions app/assets/javascripts/components/search/loading_bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@
*/
@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);

Check warning on line 23 in app/assets/javascripts/components/search/loading_bar.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/search/loading_bar.ts#L23

Added line #L23 was not covered by tests
}
}

show(): void {
Expand Down
10 changes: 10 additions & 0 deletions app/assets/javascripts/i18n/translations.json
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@
"date_before": "before",
"date_on": "on",
"description_languages": "Language of the description",
"download_button": {
"done": "Downloaded, go back",
"download": "Download",
"downloading": "Preparing submissions for download, this might take a couple of minutes. Do not close this page."
},
"dropdown": {
"multi": {
"activity_types": "Activity types",
Expand Down Expand Up @@ -712,6 +717,11 @@
"date_before": "voor",
"date_on": "op",
"description_languages": "Taal van de beschrijving",
"download_button": {
"done": "Gedownload, ga terug",
"download": "Downloaden",
"downloading": "Bezig met het voorbereiden van de download, dit kan enkele minuten duren. Sluit dit deze pagina niet."
},
"dropdown": {
"multi": {
"activity_types": "Activiteitstypes",
Expand Down
27 changes: 16 additions & 11 deletions app/controllers/exports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@
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')

Check warning on line 14 in app/controllers/exports_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/exports_controller.rb#L8-L14

Added lines #L8 - L14 were not covered by tests
else
head :not_found

Check warning on line 16 in app/controllers/exports_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/exports_controller.rb#L16

Added line #L16 was not covered by tests
end
end
end
end

def new_series_export
Expand Down Expand Up @@ -89,13 +97,10 @@
# 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
3 changes: 0 additions & 3 deletions app/helpers/notifications_helper.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/packs/export.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { initSelection } from "export.ts";
import "components/download_button";

window.dodona.initSelection = initSelection;
6 changes: 2 additions & 4 deletions app/models/export.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand All @@ -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
2 changes: 1 addition & 1 deletion app/views/exports/download_submissions.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
<% end %>
</div>
<div class="stepper-actions stepper-border">
<button type="submit" form="download_submissions" class="btn btn-filled"><%= t('.start_download') %></button>
<d-download-button></d-download-button>
</div>
</div>
</div>
Expand Down
63 changes: 0 additions & 63 deletions app/views/exports/index.html.erb

This file was deleted.

2 changes: 2 additions & 0 deletions app/views/exports/show.json.jbuilder
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
json.ready @export.finished?
json.url rails_blob_path(@export.archive, disposition: 'attachment') if @export.finished? && @export.archive.attached?
2 changes: 1 addition & 1 deletion app/views/layouts/_searchbar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<i class="mdi mdi-magnify"></i>
<form class='search' onsubmit="return false;">
<d-search-field placeholder="<%= t("layout.search.search") %>"></d-search-field>
<d-loading-bar></d-loading-bar>
<d-loading-bar search-based></d-loading-bar>
</form>
<% if local_assigns[:actions]&.any? %>
<d-search-actions actions='<%= raw (local_assigns.fetch :actions, false).to_json %>'></d-search-actions>
Expand Down
2 changes: 1 addition & 1 deletion app/views/pages/home.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
default="<%= @year %>">
</d-standalone-dropdown-filter>
</div>
<d-loading-bar style="margin-top: -24px;"></d-loading-bar>
<d-loading-bar search-based style="margin-top: -24px;"></d-loading-bar>
</div>
<div class="row" id="course-for-year-row">
<%= render partial: "course_cards", locals: {courses: @courses} %>
Expand Down
5 changes: 5 additions & 0 deletions config/locales/js/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,8 @@ en:
feedbacks:
submission:
submit: Submit this solution
download_button:
download: Download
done: Downloaded, go back
downloading: Preparing submissions for download, this might take a couple of minutes. Do not close this page.

4 changes: 4 additions & 0 deletions config/locales/js/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,7 @@ nl:
feedbacks:
submission:
submit: Deze oplossing indienen
download_button:
download: Downloaden
done: Gedownload, ga terug
downloading: Bezig met het voorbereiden van de download, dit kan enkele minuten duren. Sluit dit deze pagina niet.
jorg-vr marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20240305141148_remove_export_notifications.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RemoveExportNotifications < ActiveRecord::Migration[7.1]
def change
Notification.where(notifiable_type: 'Export').destroy_all
end
end
2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading