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 filter on courses for which you can register when relevant #3902

Merged
merged 8 commits into from
Aug 19, 2022
Merged
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
5 changes: 5 additions & 0 deletions app/controllers/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ class CoursesController < ApplicationController
scope.by_course_labels(value, controller.params[:id])
end

has_scope :can_register, type: :boolean, only: :index do |controller, scope|
scope.can_register(controller.current_user)
end

has_scope :order_by, using: %i[column direction], only: :scoresheet, type: :hash do |controller, scope, value|
column, direction = value
if %w[ASC DESC].include?(direction)
Expand Down Expand Up @@ -63,6 +67,7 @@ def index
end

@courses = @courses.paginate(page: parse_pagination_param(params[:page]))
@membership_status = current_user&.course_memberships&.where(course_id: @courses.pluck(:id))&.map { |c| [c.course_id, c.status] }&.to_h || {}
@repository = Repository.find(params[:repository_id]) if params[:repository_id]
@institution = Institution.find(params[:institution_id]) if params[:institution_id]
@copy_courses = params[:copy_courses]
Expand Down
10 changes: 10 additions & 0 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,16 @@ class Course < ApplicationRecord
scope :by_name, ->(name) { where('name LIKE ?', "%#{name}%") }
scope :by_teacher, ->(teacher) { where('teacher LIKE ?', "%#{teacher}%") }
scope :by_institution, ->(institution) { where(institution: institution) }
scope :can_register, lambda { |user|
if user&.institutional?
where(registration: %i[open_for_all open_for_institutional_users])
.or(where(registration: :open_for_institution, institution_id: user.institution_id))
.or(where(id: user.subscribed_courses.pluck(:id)))
else
where(registration: :open_for_all)
.or(where(id: user&.subscribed_courses&.pluck(:id)))
end
}
default_scope { order(year: :desc, name: :asc) }

token_generator :secret, unique: false, length: 5
Expand Down
8 changes: 7 additions & 1 deletion app/views/courses/_courses_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@
<% courses.each do |course| %>
<tr>
<td>
<% if current_user&.admin_of?(course) %>
<% if @membership_status[course.id] == 'course_admin' %>
<span title='<%= t "pages.course_card.course-admin" %>'><i class='mdi mdi-school'></i></span>
<% elsif @membership_status[course.id] == 'student' %>
<span title='<%= t "courses.registration.already_a_member" %>'><i class='mdi mdi-account-check-outline'></i></span>
<% elsif @membership_status[course.id] == 'pending' %>
<span title='<%= t "courses.registration.pending" %>'><i class='mdi mdi-account-clock-outline'></i></span>
<% elsif current_user.present? && !course.open_for_user?(current_user) %>
<span title='<%= t("courses.show.registration-#{course.registration}-info", institution: course.institution&.name) %>'><i class='mdi mdi-account-remove-outline'></i></span>
<% end %>
<% if course.featured %>
<span title='<%= Course.human_attribute_name("featured") %>'><i class='mdi mdi-star-outline'></i></span>
Expand Down
2 changes: 1 addition & 1 deletion app/views/courses/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<% end %>
</ul>
</div>
<%= render partial: 'layouts/searchbar', locals: {institutions: Institution.all, eager: false, no_dropdown_for: ["institutions"] } %>
<%= render partial: 'layouts/searchbar', locals: {institutions: Institution.all, eager: false, no_dropdown_for: ["institutions"], actions: [{search: {can_register: true}, type: 'can-register', text: "#{t(".only_show_can_register")}"}] } %>
<div id="courses-table-wrapper"></div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/pages/_getting_started_card.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<div class="card-title col-6">
<h1 class="card-title-text getting-started-title"><%= t ".title" %></h1>
<p class="getting-started-text"><%= t ".text" %></p>
<%= link_to t(".action"), courses_path, class: "btn btn-text getting-started-action" %>
<%= link_to t(".action"), courses_path(:can_register => :true), class: "btn btn-text getting-started-action" %>
</div>
</div>
</div>
2 changes: 1 addition & 1 deletion app/views/pages/home.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<% end %>
<div class="card home-summary-card">
<div class="card-supporting-text">
<%= link_to (t('.more-courses') + ' …'), courses_path, class: "btn btn-text" %>
<%= link_to (t('.more-courses') + ' …'), courses_path(:can_register => :true), class: "btn btn-text" %>
</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/courses/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ en:
featured_courses: Featured courses
all_courses: "All courses"
my_courses: "My courses"
only_show_can_register: "Only show courses for which you can register"
new:
title: Create course
empty: "Empty course"
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/courses/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ nl:
featured_courses: Uitgelichte cursussen
all_courses: "Alle cursussen"
my_courses: "Mijn cursussen"
only_show_can_register: "Toon enkel cursusen waarvoor je kan registreren"
new:
title: Cursus aanmaken
empty: "Lege cursus"
Expand Down
27 changes: 27 additions & 0 deletions test/models/course_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -259,4 +259,31 @@ class CourseTest < ActiveSupport::TestCase
course.usable_repositories << ex.repository
assert course.all_activities_accessible?
end

test 'can_register scope should always contain own courses' do
i = create :institution
[create(:institution), i, nil].each do |institution|
u = create :user, institution: institution
CourseMembership.statuses.each do |s|
Course.registrations.each do |r|
c = create :course, registration: r[1], institution: i
CourseMembership.create user: u, course: c, status: s[1]
end
end
assert_equal u.subscribed_courses.count, u.subscribed_courses.can_register(u).count
end
end

test 'can_register should only return course if the user can register for it' do
i = create :institution
Course.registrations.each do |r|
create :course, registration: r[1], institution: i
end
[create(:institution), i, nil].each do |institution|
u = create :user, institution: institution
Course.all.each do |c|
assert_equal c.open_for_user?(u), Course.can_register(u).exists?(c.id)
end
end
end
end