Skip to content

Add ProFormA validity checks and disable exporting not invalid #2054

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions app/assets/stylesheets/tasks.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,16 @@
margin-bottom: 0 !important;
}
}
.button-box{

.btn.disabled{
border: 1px solid lightgray;
}
}

.disabled-btn-wrapper {
width: 100%;
}

.completeness-checklist-container {
background: white;
Expand Down
1 change: 1 addition & 0 deletions app/controllers/task_contributions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def show
@files = @task.files
@tests = @task.tests
@model_solutions = @task.model_solutions
@proforma_valid = [nil, *ProformaXML::SCHEMA_VERSIONS].index_with {|v| @task.proforma_valid?(schema_version: v) }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't easy for me to figure this out. The nil case is the default case? It will be one of the versions listed in ProformaXML::SCHEMA_VERSIONS (the default version). The rest of the versions are used to enable the downloads for the specific versions?

I think this hash is a bit difficult to understand. Maybe we can create a new object like ProformaTaskValidation with the task as an input. This object to then do the validation you added to the model and integrate the check for each version. 🤔


@user_rating = @task.ratings&.find_by(user: current_user)&.rating
render 'tasks/show'
Expand Down
1 change: 1 addition & 0 deletions app/controllers/tasks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def show
@tests = @task.tests
@model_solutions = @task.model_solutions

@proforma_valid = [nil, *ProformaXML::SCHEMA_VERSIONS].index_with {|v| @task.proforma_valid?(schema_version: v) }
@user_rating = @task.ratings&.find_by(user: current_user) || Rating.new(Rating::CATEGORIES.index_with {|_category| 0 })
end

Expand Down
11 changes: 11 additions & 0 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,17 @@ def url_for(action)
Rails.application.routes.url_helpers.url_for(controller:, action:, **id_options, only_path: true)
end

def proforma_valid?(schema_version: nil)
check_versions = schema_version.nil? ? ProformaXML::SCHEMA_VERSIONS : [schema_version]

check_versions.all? do |version|
ProformaService::ExportTask.call(task: self, options: {version:})
true
rescue ProformaXML::PostGenerateValidationError
false
end
end

private

def duplicate_tests(set_parent_id: true)
Expand Down
23 changes: 19 additions & 4 deletions app/views/tasks/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -519,13 +519,22 @@
.wrapper
- if policy(@task).download?
.dropdown.btn-group
= button_tag class: 'btn btn-light dropdown-toggle', 'data-bs-toggle': 'dropdown' do
= t('common.button.download_zip')
- if @proforma_valid[nil]
= button_tag class: 'btn btn-light dropdown-toggle', 'data-bs-toggle': 'dropdown' do
= t('common.button.download_zip')
- else
.disabled-btn-wrapper data-bs-toggle='tooltip' title=t('.not_proforma_valid', version: '')
= button_tag class: 'btn btn-light dropdown-toggle disabled', 'data-bs-toggle': 'dropdown' do
= t('common.button.download_zip')
ul.scrollable.dropdown-menu role='menu'
li.dropdown-header = "#{t('common.button.available_versions')}: "
- ProformaXML::SCHEMA_VERSIONS.each do |proforma_version|
li
= link_to proforma_version, download_task_path(@task, version: proforma_version), class: 'btn btn-light dropdown-item', target: '_blank', rel: 'noopener noreferrer'
- if @proforma_valid[proforma_version]
= link_to proforma_version, download_task_path(@task, version: proforma_version), class: 'btn btn-light dropdown-item', target: '_blank', rel: 'noopener noreferrer'
- else
.disabled-btn-wrapper data-bs-toggle='tooltip' title=t('.not_proforma_valid', version: " (#{proforma_version})")
= link_to proforma_version, download_task_path(@task, version: proforma_version), class: 'btn btn-light dropdown-item disabled', target: '_blank', rel: 'noopener noreferrer'

- else
div data-bs-toggle='tooltip' title=unavailable_tooltip data-bs-delay=150
Expand All @@ -535,6 +544,7 @@
.dropdown.btn-group
= button_tag class: 'btn btn-light dropdown-toggle', 'data-bs-toggle': 'dropdown' do
= t('.button.export')

ul.scrollable.dropdown-menu role='menu'
li.dropdown-header = "#{t('tasks.show.export_to')}: "
- if current_user.available_account_links.empty?
Expand All @@ -543,7 +553,12 @@
- else
- current_user.available_account_links.each do |acc_link|
li
= link_to acc_link.name, export_external_start_task_path(account_link: acc_link), method: :post, remote: true, class: 'dropdown-item export-test'
- if @proforma_valid[acc_link.proforma_version || ProformaXML::SCHEMA_VERSION_LATEST]
= link_to acc_link.name, export_external_start_task_path(account_link: acc_link), method: :post, remote: true, class: 'dropdown-item export-test'
- else
.disabled-btn-wrapper data-bs-toggle='tooltip' title=t('.not_proforma_valid', version: " (#{acc_link.proforma_version || ProformaXML::SCHEMA_VERSION_LATEST})")
= link_to acc_link.name, export_external_start_task_path(account_link: acc_link), method: :post, remote: true, class: 'dropdown-item export-test disabled'

- else
div data-bs-toggle='tooltip' title=unavailable_tooltip data-bs-delay=150
= button_tag class: 'btn btn-outline-dark dropdown-toggle disabled', 'data-bs-toggle': 'dropdown' do
Expand Down
1 change: 1 addition & 0 deletions config/locales/de/views/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ de:
no_license: Keine
no_model_solution_present: Keine Musterlösungen vorhanden
no_tests: Keine Tests enthalten
not_proforma_valid: Die Aufgabe ist nicht in ProFormA%{version} exportierbar
owner_required_tooltip: Sie müssen der Autor der Aufgabe sein, um die Funktion zu verwenden.
remove_task_from_collection_warning: Sind Sie sicher, dass Sie diese Aufgabe aus der Sammlung entfernen wollen?
task_contribution:
Expand Down
1 change: 1 addition & 0 deletions config/locales/en/views/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ en:
no_license: None
no_model_solution_present: No Model Solutions present
no_tests: No Tests included
not_proforma_valid: The task is not exportable in ProFormA%{version}.
owner_required_tooltip: This feature can only be used by the owner of the task.
remove_task_from_collection_warning: Are you sure you want to remove this Task from the Collection?
task_contribution:
Expand Down
23 changes: 23 additions & 0 deletions spec/controllers/tasks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,29 @@
expect(response).to redirect_to([contribution.base, contribution])
end
end

context 'when checking proforma validity' do
before do
stub_const('ProformaXML::SCHEMA_VERSIONS', ['2.0'])

task_double = task

allow(Task).to receive(:find).with(task.to_param).and_return(task_double)

allow(task_double).to receive(:proforma_valid?).with(schema_version: nil).and_return(true)
allow(task_double).to receive(:proforma_valid?).with(schema_version: '2.0').and_return(false)
end

it 'assigns proforma_valid to instance variable' do
get_request
expect(assigns(:proforma_valid)).to be_a(Hash)
end

it 'includes all values in proforma_valid hash' do
get_request
expect(assigns(:proforma_valid)).to eql({nil => true, '2.0' => false})
end
end
end
end

Expand Down
43 changes: 43 additions & 0 deletions spec/models/task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -666,4 +666,47 @@
end
end
end

describe '#proforma_valid?' do
subject(:proforma_valid) { task.proforma_valid?(schema_version:) }

let(:task) { create(:task) }
let(:schema_version) { nil }

context 'when the task is valid for all schema versions' do
before do
allow(ProformaService::ExportTask).to receive(:call).and_return(true)
end

it { is_expected.to be true }
end

context 'when the task is invalid for a schema version' do
before do
allow(ProformaService::ExportTask).to receive(:call).and_raise(ProformaXML::PostGenerateValidationError, '["version not supported"]')
end

it { is_expected.to be false }
end

context 'when a specific schema version is provided' do
let(:schema_version) { '2.0' }

context 'when the task is valid for the specified schema version' do
before do
allow(ProformaService::ExportTask).to receive(:call).with(task: task, options: {version: schema_version}).and_return(true)
end

it { is_expected.to be true }
end

context 'when the task is invalid for the specified schema version' do
before do
allow(ProformaService::ExportTask).to receive(:call).with(task: task, options: {version: schema_version}).and_raise(ProformaXML::PostGenerateValidationError, '["version not supported"]')
end

it { is_expected.to be false }
end
end
end
end
Loading