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.

Sign up for GitHub

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

List own courses first when copying a course #3049

Closed
wants to merge 2 commits into from
Closed

List own courses first when copying a course #3049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b698ef
List own courses first when copying courses
TimonDB Aug 30, 2021
1e3d2af
List featured courses first, own courses second, others third
TimonDB Aug 30, 2021
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
8 changes: 7 additions & 1 deletion app/controllers/courses_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ def index
elsif params[:tab] == 'featured'
@courses = @courses.where(featured: true)
elsif params[:copy_courses]
@courses = @courses.reorder(featured: :desc, year: :desc, name: :asc)
@featured_courses = @courses.where(featured: true)
@own_courses = @courses.where(featured: false)
.reorder(year: :desc, name: :asc)
.select { |course| current_user.course_admin?(course) }
@other_courses = @courses.where(featured: false)
.reorder(year: :desc, name: :asc)
.reject { |course| current_user.course_admin?(course) }
Comment on lines +32 to +38
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@featured_courses = @courses.where(featured: true)
@own_courses = @courses.where(featured: false)
.reorder(year: :desc, name: :asc)
.select { |course| current_user.course_admin?(course) }
@other_courses = @courses.where(featured: false)
.reorder(year: :desc, name: :asc)
.reject { |course| current_user.course_admin?(course) }
featured_courses = @courses.where(featured: true)
own_courses = @courses.where(featured: false)
.reorder(year: :desc, name: :asc)
.select { |course| current_user.course_admin?(course) }
other_courses = @courses.where(featured: false)
.reorder(year: :desc, name: :asc)
.reject { |course| current_user.course_admin?(course) }
@courses = featured_courses + own_courses + other_courses

This way, no changes to the HTML are necessary. (Make sure to check this locally, I'm not entirely sure if the + works if the actual data hasn't been loaded yet.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to result in an error:
Screenshot from 2021-08-30 16-50-11

It would be nice if that was possible because now there is a lot of redundant code in the HTML

Copy link
Member

Choose a reason for hiding this comment

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

You can look at the server log to see the actual error. Probably an explicit load is necessary for the featured_courses.

end

@courses = apply_scopes(@courses)
Expand Down
48 changes: 47 additions & 1 deletion app/views/courses/_copy_courses_table.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,53 @@
</tr>
</thead>
<tbody>
<% courses.each do |course| %>
<% @featured_courses.each do |course| %>
<tr class="copy-course-row" data-course_id="<%= course.id %>" data-answer="<%= course.name %>">
<td>
<input type="radio" name="course-select" class="form-check-input">
</td>
<td>
<span>
<% if course.featured %>
<span title='<%= Course.human_attribute_name("featured") %>'><i class='mdi mdi-18 mdi-star-outline'></i></span>
<% end %>
<%= course.name %> <span class="text-muted">(<%= course.formatted_year %>)</span>
</span>
<br/>
<small class="text-muted"><%= [course.teacher, course.institution&.name].select(&:present?).join(" - ") %></small>
</td>
<td><%= course.subscribed_members_count %></td>
<td class="actions">
<%= link_to course_path(course), target: '_blank', class: 'btn btn-sm btn-secondary nested-link' do %>
<i class="mdi mdi-open-in-new mdi-18"></i>
<% end %>
</td>
</tr>
<% end %>
<% @own_courses.each do |course| %>
<tr class="copy-course-row" data-course_id="<%= course.id %>" data-answer="<%= course.name %>">
<td>
<input type="radio" name="course-select" class="form-check-input">
</td>
<td>
<span>
<% if course.featured %>
<span title='<%= Course.human_attribute_name("featured") %>'><i class='mdi mdi-18 mdi-star-outline'></i></span>
<% end %>
<%= course.name %> <span class="text-muted">(<%= course.formatted_year %>)</span>
</span>
<br/>
<small class="text-muted"><%= [course.teacher, course.institution&.name].select(&:present?).join(" - ") %></small>
</td>
<td><%= course.subscribed_members_count %></td>
<td class="actions">
<%= link_to course_path(course), target: '_blank', class: 'btn btn-sm btn-secondary nested-link' do %>
<i class="mdi mdi-open-in-new mdi-18"></i>
<% end %>
</td>
</tr>
<% end %>
<% @other_courses.each do |course| %>
<tr class="copy-course-row" data-course_id="<%= course.id %>" data-answer="<%= course.name %>">
<td>
<input type="radio" name="course-select" class="form-check-input">
Expand Down