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

Require at least one correct submission and a valid config before publishing #5330

Merged
merged 11 commits into from
Feb 2, 2024
20 changes: 20 additions & 0 deletions app/assets/stylesheets/models/activities.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,23 @@ center img {
background-color: var(--d-code-bg);
}
}

.draft-notice {
.form-check-input[disabled] ~ .form-check-label, .form-check-input:disabled ~ .form-check-label {
opacity: 1;
}

.form-check-input:disabled {
opacity: 1;
}

.form-check-input:checked {
background-color: var(--d-warning);
border-color: var(--d-warning);
}

.form-check-input {
background-color: transparent;
border-color: var(--d-warning);
}
}
24 changes: 24 additions & 0 deletions app/models/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
has_many :labels, through: :activity_labels

validates :path, uniqueness: { scope: :repository_id, case_sensitive: false }, allow_nil: true
validate :require_correct_submission_before_publish, on: :update

token_generator :repository_token, length: 64
token_generator :access_token
Expand Down Expand Up @@ -442,6 +443,19 @@
end
end

def correct_submission?
return true if content_page?

submissions.any?(&:accepted?)
end

def valid_config?
config
true
rescue ConfigParseError
false

Check warning on line 456 in app/models/activity.rb

View check run for this annotation

Codecov / codecov/patch

app/models/activity.rb#L456

Added line #L456 was not covered by tests
end

private

def activity_status_for(user, series = nil)
Expand Down Expand Up @@ -510,4 +524,14 @@
hash['labels'] = hash['labels'].uniq if hash.key? 'labels'
hash
end

def require_correct_submission_before_publish
return if draft?
return unless draft_was

errors.add(:base, I18n.t('activerecord.errors.models.activity.no_correct_submission')) unless correct_submission?
errors.add(:base, I18n.t('activerecord.errors.models.activity.not_valid')) if not_valid?
errors.add(:base, I18n.t('activerecord.errors.models.activity.invalid_config')) unless valid_config?
self.draft = true if errors.any?
end
end
29 changes: 26 additions & 3 deletions app/views/activities/_draft_notice.html.erb
Original file line number Diff line number Diff line change
@@ -1,13 +1,36 @@
<% if @activity.draft? %>
<div class="alert alert-warning">
<div class="alert alert-warning draft-notice">
<i class="mdi mdi-file-document-edit-outline"></i>
<%= t "activities.show.alert_draft_html" %>
<%= t "activities.show.alert_draft.text_html" %>
<% if policy(@activity).edit? %>
<% edit_path = @series.present? ?
course_series_activity_path(@series&.course, @series, @activity, {activity: {draft: false}}) :
activity_path(@activity, {activity: {draft: false}})
%>
<%= link_to t("activities.show.alert_draft_edit"), edit_path, method: :put %>

<% if @activity.ok? && @activity.correct_submission? && @activity.valid_config? %>
<%= link_to t("activities.show.alert_draft.edit"), edit_path, method: :put %>
<% else %>
<div style="margin-left: 30px; margin-top: 8px;">
<%= t("activities.show.alert_draft.before_publish") %>
<div style="margin-left: 16px;margin-top: 8px;">
<div class="form-check">
<input type="checkbox" id="valid-config" class="form-check-input" disabled <%= "checked" if @activity.valid_config? %>>
<label class="form-check-label" for="valid-config"><%= t('activities.show.alert_draft.valid_config') %></label>
</div>
<div class="form-check">
<input type="checkbox" id="is-valid" class="form-check-input" disabled <%= "checked" if @activity.ok? %>>
<label class="form-check-label" for="is-valid"><%= t('activities.show.alert_draft.is_valid') %></label>
</div>
<% if @activity.exercise? %>
<div class="form-check">
<input type="checkbox" id="correct-submission" class="form-check-input" disabled <%= "checked" if @activity.correct_submission? %>>
<label class="form-check-label" for="correct-submission"><%= t('activities.show.alert_draft.correct_submission') %></label>
</div>
<% end %>
</div>
</div>
<% end %>

<% end %>
</div>
Expand Down
4 changes: 4 additions & 0 deletions config/locales/models/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ en:
activerecord:
errors:
models:
activity:
no_correct_submission: "At least one correct submission is required, before the exercise can be published."
not_valid: "The exercise must have a name and a description."
invalid_config: "The JSON configuration is invalid."
course_membership:
at_least_one_admin: The course should always have at least one course administrator.
api_token:
Expand Down
4 changes: 4 additions & 0 deletions config/locales/models/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ nl:
activerecord:
errors:
models:
activity:
no_correct_submission: "Er moet minstens één correcte oplossing zijn, vooraleer de oefening kan worden gepubliceerd."
not_valid: "De oefening moet een naam en een beschrijving hebben."
invalid_config: "De JSON-configuratie is ongeldig."
course_membership:
at_least_one_admin: Een cursus moet altijd minstens één cursusbeheerder hebben.
api_token:
Expand Down
9 changes: 7 additions & 2 deletions config/locales/views/activities/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,13 @@ en:
preloaded_info: "We have preloaded your latest submission into the editor."
preloaded_clear: Clear editor.
preloaded_restore: Restore the initial code.
alert_draft_html: This activity is a <a href="https://docs.dodona.be/en/faq/activities/#what-is-a-draft-activity" target="_blank">draft</a> and thus not visible for students.
alert_draft_edit: Publish activity.
alert_draft:
text_html: This activity is currently a <a href="https://docs.dodona.be/en/faq/activities/#what-is-a-draft-activity" target="_blank">draft</a> and is not yet visible to students.
edit: Publish activity.
before_publish: "Before you can publish this activity, please ensure the following criteria are met:"
valid_config: "The exercise must have a valid configuration file."
is_valid: "The exercise must have a name and a description."
correct_submission: "You must submit at least one correct solution."
series_activities_add_table:
course_added_to_usable: "Adding this exercise will allow this course to use all of the private exercises in this exercise's repository. Are you sure?"
edit:
Expand Down
9 changes: 7 additions & 2 deletions config/locales/views/activities/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,13 @@ nl:
preloaded_info: We hebben jouw laatste oplossing ingeladen in de editor.
preloaded_clear: Maak de editor leeg.
preloaded_restore: Zet de voorbeeldcode terug.
alert_draft_html: Deze oefening is nog een <a href="https://docs.dodona.be/nl/faq/activities/#wat-is-een-conceptactiviteit" target="_blank">concept</a>. Ze is niet zichtbaar voor studenten.
alert_draft_edit: Deze oefening publiceren.
alert_draft:
text_html: Deze oefening is momenteel een <a href="https://docs.dodona.be/nl/faq/activities/#wat-is-een-conceptactiviteit" target="_blank">concept</a> en nog niet zichtbaar voor studenten.
edit: Deze oefening publiceren.
before_publish: "Vooraleer je deze oefening kan publiceren, moet aan de volgende voorwaarden voldaan zijn:"
valid_config: "De oefening moet een geldig configuratiebestand hebben."
is_valid: "De oefening moet een naam en een beschrijving hebben."
correct_submission: "Je moet minstens één correcte oplossing indienen."
series_activities_add_table:
course_added_to_usable: "Deze oefening toevoegen zal deze cursus toegang geven tot alle privé oefeningen in de repository van deze oefening. Ben je zeker?"
edit:
Expand Down
1 change: 1 addition & 0 deletions test/controllers/activities_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,7 @@ def create_exercises_return_valid
repo = create :repository, :git_stubbed
repo.admins << user
exercise = create :exercise, repository: repo, draft: true
create :correct_submission, exercise: exercise

put activity_url(exercise), params: { activity: { draft: false } }

Expand Down
28 changes: 28 additions & 0 deletions test/models/activity_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,32 @@ class ActivityTest < ActiveSupport::TestCase
assert_empty Activity.repository_scope(scope: :my_institution, user: nil)
assert_empty Activity.repository_scope(scope: :my_institution, user: create(:user, institution_id: nil))
end

test 'should have at least one valid submission and a valid config before publishing' do
activity = create :exercise, draft: true
activity.update(draft: false)

assert_equal 2, activity.errors.count
assert_predicate activity.reload, :draft?

# reset errors
activity = Activity.find(activity.id)
create :correct_submission, exercise: activity
activity.update(draft: false)

assert_equal 1, activity.errors.count
assert_predicate activity.reload, :draft?

# reset errors
activity = Activity.find(activity.id)
stub_status(activity, 'ok')
# don't do config related checks as there is no config
activity.stubs(:check_memory_limit)
activity.stubs(:update_config)

activity.update(draft: false)

assert_equal 0, activity.errors.count
assert_not_predicate activity.reload, :draft?
end
end
Loading