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

Add draft flag to activities #4998

Merged
merged 35 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5a45932
Add draft flag to DB (and config?)
jorg-vr Sep 22, 2023
08a6d41
Add draft mode to edit screen
jorg-vr Sep 22, 2023
685ed5d
Update access rights for draft activities
jorg-vr Sep 22, 2023
8be1451
Do not email draft errors
jorg-vr Sep 22, 2023
64b2d96
Fix don't sent email function
jorg-vr Sep 22, 2023
4c4d4c3
Allow editing draft in the policy
jorg-vr Sep 22, 2023
2f570b8
Show draft images on home page
jorg-vr Sep 25, 2023
adb0e50
Visualise draft modus for lists
jorg-vr Sep 25, 2023
a4307f5
Filter activities by draft flag
jorg-vr Sep 25, 2023
8535338
Add draft warning to the top of the page
jorg-vr Sep 26, 2023
71c5747
Fix server file
jorg-vr Sep 26, 2023
0c56209
Fix existing tests
jorg-vr Sep 26, 2023
ea8a8ef
Fix linting
jorg-vr Sep 26, 2023
fb6cbae
Add page access tests
jorg-vr Sep 26, 2023
4e0fceb
Update app/views/pages/_user_card.html.erb
jorg-vr Sep 27, 2023
7b8bfaa
Update config/locales/views/activities/nl.yml
jorg-vr Sep 27, 2023
2e9dfbf
Mark as draft during repository reprocess
jorg-vr Oct 11, 2023
15736e2
Update translation from ontwerp to concept
jorg-vr Oct 11, 2023
d34e984
Revert "Mark as draft during repository reprocess"
jorg-vr Oct 11, 2023
5cf0f25
Always update draft flag
jorg-vr Oct 11, 2023
a8647ac
Apply suggestions from code review
jorg-vr Oct 19, 2023
0d493d2
Concept => ongepubliceerd
jorg-vr Oct 19, 2023
29deee3
Merge branch 'main' into feat/add-draft-flag
jorg-vr Oct 19, 2023
925b65a
Add links to doc
jorg-vr Oct 19, 2023
3a39a7a
Undo unwanted annotations
jorg-vr Oct 19, 2023
4817cf4
Merge branch 'main' into feat/add-draft-flag
jorg-vr Oct 26, 2023
9a4499c
Apply feedback to simplify pr
jorg-vr Oct 26, 2023
242f61a
Do not allow reset to draft
jorg-vr Oct 26, 2023
7fc6a26
Fix tests
jorg-vr Oct 26, 2023
87a0c7d
Update icon layout in lists
jorg-vr Oct 26, 2023
4f601a9
Restore always showing col
jorg-vr Oct 26, 2023
715f25c
Reset code to original
jorg-vr Oct 26, 2023
587056b
Add some draft activities in the seed
jorg-vr Oct 26, 2023
bd08828
Add draft notice to all pages
jorg-vr Oct 26, 2023
a17cb6a
Move validation to policy
jorg-vr Oct 26, 2023
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
4 changes: 4 additions & 0 deletions app/assets/stylesheets/models/activities.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
color: var(--d-on-surface-muted);
}

.activity-title .mdi::before {
vertical-align: bottom;
}

th.type-icon,
td.type-icon {
width: 18px;
Expand Down
1 change: 1 addition & 0 deletions app/controllers/activities_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ActivitiesController < ApplicationController
has_scope :by_description_languages, as: 'description_languages', type: :array
has_scope :by_judge, as: 'judge_id'
has_scope :by_popularities, as: 'popularity', type: :array
has_scope :is_draft, as: 'draft', type: :boolean

has_scope :repository_scope, as: 'tab' do |controller, scope, value|
course = Series.find(controller.params[:id]).course if controller.params[:id]
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ def home
@homepage_series = courses.map { |c| c.homepage_series(current_user, 0) }.flatten.sort_by(&:deadline)

@jump_back_in = current_user.jump_back_in

@draft_exercises = current_user.draft_exercises.reorder(updated_at: :desc) if current_user.a_repository_admin?
else
set_metrics
respond_to do |format|
Expand Down
8 changes: 7 additions & 1 deletion app/models/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
# description_nl_present :boolean default(FALSE)
# description_en_present :boolean default(FALSE)
# series_count :integer default(0), not null
# draft :boolean default(TRUE)
#

require 'pathname'
Expand Down Expand Up @@ -56,6 +57,7 @@ class Activity < ApplicationRecord
has_many :labels, through: :activity_labels

validates :path, uniqueness: { scope: :repository_id, case_sensitive: false }, allow_nil: true
validates :draft, inclusion: { in: [false] }, unless: :draft_was
Copy link
Member

Choose a reason for hiding this comment

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

what does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if not draft_was, validate if draft is in the list of possible values, with the only possible value being false
draft_was is the previous value of draft

so if the previous value of draft was false, only allow false as new value

Copy link
Member

Choose a reason for hiding this comment

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

You could argument that this check should be in the controller instead of the model, but not a strong opinion so ok for me.


token_generator :repository_token, length: 64
token_generator :access_token
Expand All @@ -80,6 +82,7 @@ class Activity < ApplicationRecord
scope :by_programming_language, ->(programming_language) { includes(:programming_language).where(programming_languages: { name: programming_language }) }
scope :by_type, ->(type) { where(type: type) }
scope :by_judge, ->(judge) { where(judge_id: judge) }
scope :is_draft, -> { where(draft: true) }
scope :by_description_languages, lambda { |languages|
by_language = all # allow chaining of scopes
by_language = by_language.where(description_en_present: true) if languages.include? 'en'
Expand Down Expand Up @@ -322,12 +325,15 @@ def accessible?(user, course)
return true if user&.zeus?
return false unless access_public? \
|| repository.allowed_courses.pluck(:id).include?(course&.id)
return true if user&.member_of? course
return true if user&.course_admin? course
return false if draft?
return true if user&.member_of?(course)
return false if course.moderated && access_private?

course.open_for_user?(user)
else
return true if user&.repository_admin? repository
return false if draft?

access_public?
end
Expand Down
1 change: 1 addition & 0 deletions app/models/content_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
# description_nl_present :boolean default(FALSE)
# description_en_present :boolean default(FALSE)
# series_count :integer default(0), not null
# draft :boolean default(TRUE)
#

class ContentPage < Activity
Expand Down
1 change: 1 addition & 0 deletions app/models/exercise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
# description_nl_present :boolean default(FALSE)
# description_en_present :boolean default(FALSE)
# series_count :integer default(0), not null
# draft :boolean default(TRUE)
#

require 'pathname'
Expand Down
1 change: 1 addition & 0 deletions app/models/submission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ def update_fs
end

def report_if_internal_error
return if exercise&.draft?
return unless status_changed? && send(:'internal error?')

ExceptionNotifier.notify_exception(
Expand Down
7 changes: 7 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ class User < ApplicationRecord
has_many :repositories,
through: :repository_admins,
source: :repository
has_many :draft_exercises,
lambda {
where activities:
{ draft: true }
},
through: :repositories,
source: :exercises

has_many :annotations, dependent: :restrict_with_error
has_many :questions, dependent: :restrict_with_error
Expand Down
4 changes: 2 additions & 2 deletions app/policies/activity_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def resolve
if user&.zeus?
scope.all
else
scope.where(access: :public, status: :ok).or(scope.where(repository: user&.repositories))
scope.where(access: :public, status: :ok, draft: false).or(scope.where(repository: user&.repositories))
end
end
end
Expand Down Expand Up @@ -75,7 +75,7 @@ def read?

def permitted_attributes
if update?
%i[access name_nl name_en]
%i[access name_nl name_en draft]
Copy link
Member

Choose a reason for hiding this comment

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

You could also check here that draft can only be updated if it's currently true. Might be a more clear than the validates in the model.

else
[]
end
Expand Down
12 changes: 6 additions & 6 deletions app/views/activities/_activities_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@
<% local_assigns[:activities].each do |activity| %>
<tr>
<td class='status-icon'>
<% if user_signed_in? %>
<% if current_user.repository_admin?(activity.repository) || current_user.course_admin?(@course) %>
<%= render partial: 'activities/repository_status', locals: {activity: activity, series: local_assigns[:series]} %>
<% end %>
<%= render partial: 'activities/user_status_icon', locals: {activity: activity, series: local_assigns[:series], user: user} %>
<% if user_signed_in? && (current_user.repository_admin?(activity.repository) || current_user.course_admin?(@course)) %>
<%= render partial: 'activities/repository_status', locals: {activity: activity, series: local_assigns[:series]} %>
<% end %>
</td>

Expand All @@ -38,7 +35,10 @@
</td>

<td class="link">
<span class="ellipsis-overflow" title="<%= activity.name %>">
<span class="ellipsis-overflow activity-title" title="<%= activity.name %>">
<% if user_signed_in? %>
<%= render partial: 'activities/user_status_icon', locals: {activity: activity, series: local_assigns[:series], user: user} %>
<% end %>
<% if activity.accessible?(current_user, @course) %>
<%= link_to activity.name, get_activity_path.call(activity) %>
<% elsif activity.access_public? %>
Expand Down
14 changes: 14 additions & 0 deletions app/views/activities/_draft_notice.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<% if @activity.draft? %>
<div class="alert alert-warning">
<i class="mdi mdi-file-document-edit-outline"></i>
<%= t "activities.show.alert_draft_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 %>

<% end %>
</div>
<% end %>
11 changes: 2 additions & 9 deletions app/views/activities/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
<% content_for :javascripts do %>
<% # TODO rename pack to activity %>
<% javascript_include_tag 'exercise' %>
<% end %>
jorg-vr marked this conversation as resolved.
Show resolved Hide resolved
<%= form_for(activity.becomes(Activity), :url => activity_scoped_path(activity: activity, course: @course, series: @series), :html => {:class => 'form-horizontal'}) do |f| %>
<% if activity.errors.any? %>
<div class="callout callout-danger" id="error_explanation">
Expand Down Expand Up @@ -32,13 +28,10 @@
</div>
<div class="field form-group row">
<label class="col-sm-3 col-12 col-form-label"><%= t ".edit_activity" %></label>
<div class="col-sm-6 col-12 form-control-static"><%= link_to t(".open_on_github"), activity.github_url, target: '_blank', rel: 'noopener' %></div>
<div class="col-sm-6 col-12 form-control-static pt-2"><%= link_to t(".open_on_github"), activity.github_url, target: '_blank', rel: 'noopener' %></div>
</div>
<div class="field form-group row">
<label class="col-sm-3 col-12 col-form-label"><%= t ".info" %></label>
<div class="col-sm-6 col-12 form-control-static"><%= link_to t(".info_description"), info_activity_scoped_path(activity: @activity, course: @course, series: @series), rel: 'noopener' %></div>
<div class="col-sm-6 col-12 form-control-static pt-2"><%= link_to t(".info_description"), info_activity_scoped_path(activity: @activity, course: @course, series: @series), rel: 'noopener' %></div>
</div>
<% end %>
<script>
dodona.initLabelsEdit(<%= raw render template: 'labels/index', formats: :json %>, <%= raw (activity.merged_dirconfig['labels'] || []).to_json %>);
</script>
3 changes: 3 additions & 0 deletions app/views/activities/_repository_status.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@
<% elsif activity.not_valid? %>
<i class="mdi mdi-alert-circle mdi-18" title="<%= t('activities.index.invalid') %>"></i>
<% end %>
<% if activity.draft? %>
<i class="mdi mdi-file-document-edit-outline mdi-18 colored-warning" title="<%= t('activities.index.draft-notice') %>"></i>
<% end %>
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@
<% end %>
</td>
<td class="status-icon">
<%= raw "<i class='mdi mdi-eye-off mdi-18' title='#{t("activities.index.private")}'></i>" if activity.access_private? %>
<%= raw "<i class='mdi mdi-delete mdi-18' title='#{t("activities.index.removed")}'></i>" if activity.removed? %>
<%= raw "<i class='mdi mdi-alert-circle mdi-18' title='#{t("activities.index.invalid")}'></i>" if activity.not_valid? %>
<%= render partial: 'activities/repository_status', locals: { activity: activity } %>
<%= activity_icon(activity) %>
</td>
<td class="link">
Expand Down
1 change: 1 addition & 0 deletions app/views/activities/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<%= render 'navbar_links' %>
<div class="row">
<div class="col-md-10 offset-md-1 col-12">
<%= render partial: 'draft_notice' %>
<div class="card">
<div class="card-title card-title-colored">
<h2 class="card-title-text">
Expand Down
1 change: 1 addition & 0 deletions app/views/activities/info.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
%>
<div class="row">
<div class="col-lg-10 offset-lg-1 col-12">
<%= render partial: 'draft_notice' %>
<div class="alert alert-info">
<% if @activity.exercise? && @activity.solutions.any? %>
<%= t '.visibility_solution_warning' %>
Expand Down
2 changes: 2 additions & 0 deletions app/views/activities/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
next_tooltip_actionable = next_activity ? t('.navigation.next_actionable') : t('.navigation.back_to_course_actionable')
end %>

<%= render partial: 'draft_notice' %>

<div class="row">
<% if @activity.exercise? %>
<div class="col-md-1 p-0">
Expand Down
21 changes: 21 additions & 0 deletions app/views/pages/_user_card.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,27 @@
</div>
</div>

<% if @draft_exercises.present? %>
<div class="card home-summary-card user">
<div class="card-supporting-text recents">
<h5><%= t ".draft-exercises" %></h5>
<% @draft_exercises.first(5).each do |exercise| %>
<p>
<%= link_to activity_submissions_path(exercise), class: 'btn-icon float-end' do %>
<i class="mdi mdi-chevron-right"></i>
<% end %>
<span class='float-start'><%= activity_icon exercise %></span>
<%= link_to exercise.name, activity_path(exercise), class: "course-link", title: exercise.name %>
<%= link_to exercise.repository.name, repository_path(exercise.repository), class: "small text-muted course-link", title: exercise.repository.name %>
</p>
<% end %>
<% if @draft_exercises.count > 5 %>
<%= link_to t(".all-draft-exercises"), activities_path(draft: true) %>
<% end %>
</div>
</div>
<% end %>

<% deadlines = @homepage_series %>
<% if deadlines.any? %>
<div class="card home-summary-card user">
Expand Down
3 changes: 3 additions & 0 deletions config/locales/views/activities/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ en:
mine: "My activities"
featured: "Featured activities"
all: "All activities"
draft-notice: "This activity is a draft and thus not visible for students."
show:
code: Code
handin: Hand in
Expand Down Expand Up @@ -97,6 +98,8 @@ 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.
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
3 changes: 3 additions & 0 deletions config/locales/views/activities/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ nl:
mine: "Mijn activiteiten"
featured: "Uitgelichte activiteiten"
all: "Alle activiteiten"
draft-notice: "Deze activiteit is nog een concept. Ze is niet zichtbaar voor studenten."
show:
code: Code
handin: Indienen
Expand Down Expand Up @@ -97,6 +98,8 @@ 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.
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
2 changes: 2 additions & 0 deletions config/locales/views/pages/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ en:
submissions: Submissions
correct-exercises: Correct exercises
recent-exercises: Recent exercises
draft-exercises: Draft exercises
all-draft-exercises: All draft exercises
getting_started_card:
title: Hi there,
text: It looks like you're new here. To get started, register for a course.
Expand Down
2 changes: 2 additions & 0 deletions config/locales/views/pages/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ nl:
submissions: Ingediende oplossingen
correct-exercises: Correcte oefeningen
recent-exercises: Recente oefeningen
draft-exercises: Ongepubliceerde oefeningen
all-draft-exercises: Alle ongepubliceerde oefeningen
getting_started_card:
title: Hallo,
text: Welkom op Dodona! Registreer je voor een cursus om van start te gaan.
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20230922115708_add_draft_to_activities.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddDraftToActivities < ActiveRecord::Migration[7.0]
def change
add_column :activities, :draft, :boolean, default: false
end
end
5 changes: 5 additions & 0 deletions db/migrate/20231026075353_make_draft_default_true.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class MakeDraftDefaultTrue < ActiveRecord::Migration[7.1]
def change
change_column_default :activities, :draft, from: false, to: true
end
end
3 changes: 2 additions & 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.0].define(version: 2023_08_10_105908) do
ActiveRecord::Schema[7.1].define(version: 2023_10_26_075353) 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 Expand Up @@ -59,6 +59,7 @@
t.boolean "description_nl_present", default: false
t.boolean "description_en_present", default: false
t.integer "series_count", default: 0, null: false
t.boolean "draft", default: true
t.index ["judge_id"], name: "index_activities_on_judge_id"
t.index ["name_nl"], name: "index_activities_on_name_nl"
t.index ["path", "repository_id"], name: "index_activities_on_path_and_repository_id", unique: true
Expand Down
7 changes: 5 additions & 2 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,11 @@ def fill_series_with_realistic_submissions(s)
RepositoryAdmin.create(repository: activity_repo, user: zeus)
RepositoryAdmin.create(repository: big_activity_repo, user: zeus)

contents_list = ContentPage.all.to_a
exercises_list = Exercise.all.to_a
# remove draft status from all activities except the first 5
Activity.where.not(id: Activity.first(5)).update_all(draft: false)

contents_list = ContentPage.where(draft: false).to_a
exercises_list = Exercise.where(draft: false).all.to_a

puts "Add series, content pages, exercises, read states and submissions to courses (#{Time.now - start})"

Expand Down
Loading