Skip to content

Commit 6fddd54

Browse files
author
Felix Milea-Ciobanu
committed
cleanup modules view logic
test plan: - modules functionality should NOT have changed at all to the user refs CYOE-234 Change-Id: I02dee3ba4c0d9f8712c920e04cefe05b43434f7c Reviewed-on: https://gerrit.instructure.com/84327 Tested-by: Jenkins Reviewed-by: Jeremy Stanley <jeremy@instructure.com> QA-Review: Alex Morris <amorris@instructure.com> Product-Review: Felix Milea-Ciobanu <fmileaciobanu@instructure.com>
1 parent 3983e8d commit 6fddd54

6 files changed

Lines changed: 143 additions & 102 deletions

File tree

‎app/controllers/context_modules_controller.rb‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,37 @@ class ContextModulesController < ApplicationController
2424
before_filter { |c| c.active_tab = "modules" }
2525

2626
module ModuleIndexHelper
27+
include ContextModulesHelper
28+
2729
def load_module_file_details
2830
attachment_tags = @context.module_items_visible_to(@current_user).where(content_type: 'Attachment').preload(:content => :folder)
2931
attachment_tags.inject({}) do |items, file_tag|
3032
items[file_tag.id] = {
31-
id: file_tag.id,
32-
content_id: file_tag.content_id,
33-
content_details: content_details(file_tag, @current_user, :for_admin => true)
33+
id: file_tag.id,
34+
content_id: file_tag.content_id,
35+
content_details: content_details(file_tag, @current_user, :for_admin => true)
3436
}
3537
items
3638
end
3739
end
3840

41+
def modules_cache_key
42+
@modules_cache_key ||= begin
43+
visible_assignments = @current_user.try(:assignment_and_quiz_visibilities, @context)
44+
cache_key_items = [@context.cache_key, @can_edit, 'all_context_modules_draft_8', collection_cache_key(@modules), Time.zone, Digest::MD5.hexdigest(visible_assignments.to_s)]
45+
cache_key = cache_key_items.join('/')
46+
cache_key = add_menu_tools_to_cache_key(cache_key)
47+
end
48+
end
49+
3950
def load_modules
4051
@modules = @context.modules_visible_to(@current_user)
4152
@collapsed_modules = ContextModuleProgression.for_user(@current_user).for_modules(@modules).select([:context_module_id, :collapsed]).select{|p| p.collapsed? }.map(&:context_module_id)
4253

54+
@can_edit = can_do(@context, @current_user, :manage_content)
55+
56+
modules_cache_key
57+
4358
@menu_tools = {}
4459
placements = [:assignment_menu, :discussion_topic_menu, :file_menu, :module_menu, :quiz_menu, :wiki_page_menu]
4560
tools = ContextExternalTool.all_tools_for(@context, placements: placements,
@@ -65,7 +80,6 @@ def index
6580

6681
if @context.grants_right?(@current_user, session, :participate_as_student)
6782
return unless tab_enabled?(@context.class::TAB_MODULES)
68-
ActiveRecord::Associations::Preloader.new.preload(@modules, :content_tags)
6983
@modules.each{|m| m.evaluate_for(@current_user) }
7084
session[:module_progressions_initialized] = true
7185
end

‎app/helpers/context_modules_helper.rb‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,44 @@ def module_item_unpublishable?(item)
7373
item.content.can_unpublish?
7474
end
7575

76+
def preload_modules_content(modules, can_edit)
77+
ActiveRecord::Associations::Preloader.new.preload(modules, :content_tags => :content)
78+
preload_can_unpublish(@context, @modules) if can_edit
79+
end
80+
81+
def process_module_data(mod, is_cyoe_on = false, is_student = false, cyoe_data = nil)
82+
# pre-calculated module view data can be added here
83+
module_data = {
84+
published_status: mod.published? ? 'published' : 'unpublished',
85+
items: mod.content_tags_visible_to(@current_user).map
86+
}
87+
88+
items_data = {}
89+
module_data[:items].each do |item|
90+
# pre-calculated module item view data can be added here
91+
item_data = {
92+
published_status: item.published? ? 'published' : 'unpublished'
93+
}
94+
95+
if is_cyoe_on && is_student
96+
item_data[:is_trigger_assignment] = cyoe_data ? cyoe_data.has_key?(item.id) : false
97+
item_data[:is_in_conditional] = false # TODO: check against cyoe_data
98+
99+
# FIXME: use CYOE data to figure out if
100+
if item.graded?
101+
item_data[:has_submission] = item.content.submissions.length > 0 if defined? item.content.submissions
102+
item_data[:has_submission] = item.content.quiz_submissions.length > 0 if defined? item.content.quiz_submissions
103+
end
104+
end
105+
106+
items_data[item.id] = item_data
107+
end
108+
109+
module_data[:items_data] = items_data
110+
111+
return module_data
112+
end
113+
76114
def module_item_translated_content_type(item)
77115
return '' unless item
78116

‎app/views/context_modules/_content_next.html.erb‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
1-
<% @body_classes << 'padless-content' %>
2-
<% course_home ||= false %>
31
<%
2+
@body_classes << 'padless-content'
3+
course_home ||= false
44
js_env({
55
:COLLAPSED_MODULES => @collapsed_modules,
6-
:CAN_MANAGE_MODULES => can_do(@context, @current_user, :manage_content),
6+
:CAN_MANAGE_MODULES => @can_edit,
77
:IS_STUDENT => can_do(@context, @current_user, :participate_as_student),
88
:COURSE_ID => @context.id,
99
:NO_MODULE_PROGRESSIONS => @context.large_roster,
1010
})
11+
js_bundle :context_modules
12+
css_bundle :content_next
1113
%>
12-
<% js_bundle :context_modules %>
13-
<% css_bundle :content_next %>
14-
1514
<% if course_home %>
1615
<div class="screenreader-only"><%= @context.name %></div>
1716
<h2 class="context-modules-title screenreader-only"><%= t('headings.course_modules', %{Course Modules}) %></h2>
@@ -20,7 +19,7 @@
2019
<% end %>
2120

2221
<div class="header-bar">
23-
<% if can_do(@context, @current_user, :manage_content) %>
22+
<% if @can_edit %>
2423
<div class="header-bar-right">
2524
<a class="btn module_progressions_link" href="<%= progressions_course_context_modules_path(@context) %>"><%= t('links.student_progress', 'View Progress') %></a>
2625
<button class="btn btn-primary add_module_link">
@@ -32,7 +31,7 @@
3231
<% end %>
3332
</div>
3433

35-
<% if can_do(@context, @current_user, :manage_content) %>
34+
<% if @can_edit %>
3635
<div class="hidden-readable screenreader-only" aria-label="keyboard instructions">
3736
<%= t('modules_keyboard_hint_updated',
3837
'Warning: For improved accessibility in reordering Modules (or Module items), please use the Move To Dialog option found in the menu.') %>
@@ -59,27 +58,21 @@ TEXT
5958
<div
6059
id="context_modules"
6160
aria-label="<%= t('headings.course_modules', %{Course Modules}) %>"
62-
class="ig-list <%= 'editable' if can_do(@context, @current_user, :manage_content) %>"
61+
class="ig-list <%= 'editable' if @can_edit %>"
6362
>
64-
<% editable = can_do(@context, @current_user, :manage_content) %>
65-
<% visible_assignments = @current_user.try(:assignment_and_quiz_visibilities, @context) %>
66-
<% cache_key_items = [@context.cache_key, editable, 'all_context_modules_draft_9', collection_cache_key(@modules), Time.zone, Digest::MD5.hexdigest(visible_assignments.to_s)] %>
67-
<% cache_key = cache_key_items.join('/') %>
68-
<% cache_key = add_menu_tools_to_cache_key(cache_key) %>
69-
<% cache(cache_key) do %>
70-
<% ActiveRecord::Associations::Preloader.new.preload(@modules, :content_tags => :content) %>
71-
<% preload_can_unpublish(@context, @modules) %>
63+
<% cache(@modules_cache_key) do %>
64+
<% preload_modules_content(@modules, @can_edit) %>
7265
<% @modules.each do |m| %>
73-
<%= render :partial => 'context_modules/context_module_next', :object => m, :locals => {:editable => editable} %>
66+
<%= render :partial => 'context_modules/context_module_next', :object => m, :as => :context_module, :locals => { :editable => @can_edit } %>
7467
<% end %>
7568
<% end %>
7669
</div>
7770
</div>
7871

79-
<%= render :partial => 'context_modules/context_module_next', :object => nil %>
80-
<%= render :partial => 'context_modules/module_item_next', :object => nil, :locals => {:editable => editable } %>
72+
<%= render :partial => 'context_modules/context_module_next', :object => nil, :as => :context_module %>
73+
<%= render :partial => 'context_modules/module_item_next', :object => nil, :as => :module_item, :locals => { :editable => @can_edit } %>
8174

82-
<% if can_do(@context, @current_user, :manage_content) %>
75+
<% if @can_edit %>
8376
<form id="move_context_module_form" style="display:none" class="form-dialog" title="Move Module">
8477
<div class="form-dialog-content">
8578
<h2><%= t('move_module.place', "Place ") %> <span id="move_module_name"> </span></h2>

‎app/views/context_modules/_context_module_next.html.erb‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
1-
21
<%
32
context_module_next ||= nil
4-
context_module = context_module_next
3+
context_module ||= context_module_next
4+
module_data = context_module ? process_module_data(context_module) : { published_status: 'unpublished' }
55
editable ||= can_do(@context, @current_user, :manage_content)
66
workflow_state = context_module && context_module.workflow_state
7-
published_status = context_module && context_module.published? ? 'published' : 'unpublished'
87
@modules ||= []
98
%>
10-
<% cache_if_module(context_module, editable, @current_user, @context) do %>
119

10+
<% cache_if_module(context_module, editable, @current_user, @context) do %>
1211
<div
1312
class="item-group-condensed context_module
1413
<%= 'editable_context_module' if editable %>
@@ -112,15 +111,15 @@
112111
data-module-type="module"
113112
data-id="<%= context_module && context_module.id %>"
114113
data-course-id="<%= context_module && context_module.context_id %>"
115-
data-published="<%= published_status == 'published' %>"
114+
data-published="<%= module_data[:published_status] == 'published' %>"
116115
data-publishable="<%= true %>"
117116
data-publish-message="<%= t('Unpublished. Click to publish %{module_name}.', {module_name: context_module ? context_module.name : 'module'}) %>"
118117
data-unpublish-message="<%= t('Published. Click to unpublish %{module_name}.', {module_name: context_module ? context_module.name : 'module'}) %>"
119118
title=""
120119
data-tooltip
121-
class="publish-icon module <%= published_status %>"
120+
class="publish-icon module <%= module_data[:published_status] %>"
122121
>
123-
<i class="icon-<%= published_status %>" alt="<%= published_status == 'published' ? t('published') : t('unpublished') %>"></i>
122+
<i class="icon-<%= module_data[:published_status] %>" alt="<%= module_data[:published_status] == 'published' ? t('published') : t('unpublished') %>"></i>
124123
</span>
125124

126125
<button
@@ -163,9 +162,9 @@
163162

164163
<div class="content" id="context_module_content_<%= context_module && context_module.id %>">
165164
<ul class="ig-list items context_module_items <%= 'manageable' if editable %>">
166-
<% if context_module %>
167-
<% context_module.content_tags_visible_to(@current_user).each do |tag| %>
168-
<%= render :partial => 'context_modules/module_item_next', :object => tag, :locals => {:completion_criteria => context_module.completion_requirements, :editable => editable} %>
165+
<% if context_module && module_data[:items] %>
166+
<% module_data[:items].each do |item| %>
167+
<%= render :partial => 'context_modules/module_item_next', :object => item, :as => :module_item, :locals => {:completion_criteria => context_module.completion_requirements, :item_data => module_data[:items_data][item.id], :editable => editable} %>
169168
<% end %>
170169
<% end %>
171170
</ul>

0 commit comments

Comments
 (0)