Merge pull request #5330 from dodona-edu/enhance/draft-remove

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

app/assets/stylesheets/models/activities.css.scss

@@ -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);
+  }
+}

app/models/activity.rb

@@ -57,6 +57,7 @@ class Activity < ApplicationRecord
   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
@@ -442,6 +443,19 @@ def types
     end
   end
 
+  def correct_submission?
+    return true if content_page?
+
+    submissions.any?(&:accepted?)
+  end
+
+  def valid_config?
+    config
+    true
+  rescue ConfigParseError
+    false
+  end
+
   private
 
   def activity_status_for(user, series = nil)
@@ -510,4 +524,14 @@ def unique_labels(hash)
     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

app/views/activities/_draft_notice.html.erb

@@ -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>

config/locales/models/en.yml

@@ -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:

config/locales/models/nl.yml

@@ -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:

config/locales/views/activities/en.yml

@@ -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:

config/locales/views/activities/nl.yml

@@ -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:

test/controllers/activities_controller_test.rb

@@ -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 } }
 

test/models/activity_test.rb

@@ -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