Skip to content

Commit e43ac81

Browse files
jstanley0bracken
jstanley0
authored and
bracken
committed
allow moving module items between modules via api
test plan: - make sure the list/show module item api actions now include the module_id in the module_item - consult the Update Module Item API documentation to see how to move a module item by specifying module_item[module_id] - move an item to another module in this way - set the module_item[module_id] and module_item[position] to move and place an item in the same API call - move an item with a completion requirement and verify that the requirement still exists in the new module (note: "must view" requirements will need to be completed again in the new module) - confirm that the API does not let you move an item to a module in a different course fixes CNVS-7068 Change-Id: I926e7bd242af7fe1743ad5da39daf96457983129 Reviewed-on: https://gerrit.instructure.com/22768 Reviewed-by: Bracken Mosbacker <bracken@instructure.com> Product-Review: Bracken Mosbacker <bracken@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> QA-Review: August Thornton <august@instructure.com>
1 parent b117b39 commit e43ac81

3 files changed

Lines changed: 82 additions & 9 deletions

File tree

‎app/controllers/context_module_items_api_controller.rb‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ def redirect
154154
# @argument module_item[type] [Required] The type of content linked to the item
155155
# one of "File", "Page", "Discussion", "Assignment", "Quiz", "SubHeader", "ExternalUrl", "ExternalTool"
156156
# @argument module_item[content_id] [Required, except for 'ExternalUrl', 'Page', and 'SubHeader' types] The id of the content to link to the module item
157-
# @argument module_item[position] [Optional] The position of this module in the course (1-based)
157+
# @argument module_item[position] [Optional] The position of this item in the module (1-based)
158158
# @argument module_item[indent] [Optional] 0-based indent level; module items may be indented to show a hierarchy
159159
# @argument module_item[page_url] [Required for 'Page' type] Suffix for the linked wiki page (e.g. 'front-page')
160160
# @argument module_item[external_url] [Required for 'ExternalUrl' and 'ExternalTool' types] External url that the item points to
@@ -220,7 +220,7 @@ def create
220220
# Update and return an existing module item
221221
#
222222
# @argument module_item[title] [Optional] The name of the module item
223-
# @argument module_item[position] [Optional] The position of this module in the course (1-based)
223+
# @argument module_item[position] [Optional] The position of this item in the module (1-based)
224224
# @argument module_item[indent] [Optional] 0-based indent level; module items may be indented to show a hierarchy
225225
# @argument module_item[external_url] [Optional, only applies to 'ExternalUrl' type] External url that the item points to
226226
# @argument module_item[new_tab] [Optional, only applies to 'ExternalTool' type] Whether the external tool opens in a new tab
@@ -231,6 +231,7 @@ def create
231231
# Inapplicable types will be ignored
232232
# @argument module_item[completion_requirement][min_score] [Required for completion_requirement type 'min_score'] minimum score required to complete
233233
# @argument module_item[published] [Optional] Whether the module item is published and visible to students
234+
# @argument module_item[module_id] [Optional] Move this item to another module by specifying the target module id here. The target module must be in the same course.
234235
#
235236
# @example_request
236237
#
@@ -252,6 +253,19 @@ def update
252253
@tag.url = params[:module_item][:external_url] if %w(ExternalUrl ContextExternalTool).include?(@tag.content_type) && params[:module_item][:external_url]
253254
@tag.indent = params[:module_item][:indent] if params[:module_item][:indent]
254255
@tag.new_tab = value_to_boolean(params[:module_item][:new_tab]) if params[:module_item][:new_tab]
256+
if target_module_id = params[:module_item][:module_id]
257+
return render :json => {:message => "invalid module_id"}, :status => :bad_request unless @context.context_modules.where(:id => target_module_id).exists?
258+
old_module = @context.context_modules.find(@tag.context_module_id)
259+
@tag.context_module_id = target_module_id
260+
if req_index = old_module.completion_requirements.find_index { |req| req[:id] == @tag.id }
261+
old_module.completion_requirements_will_change!
262+
req = old_module.completion_requirements.delete_at(req_index)
263+
old_module.save!
264+
params[:module_item][:completion_requirement] = req
265+
else
266+
ContentTag.touch_context_modules([old_module.id, target_module_id])
267+
end
268+
end
255269

256270
if params[:module_item].has_key?(:published)
257271
if value_to_boolean(params[:module_item][:published])

‎lib/api/v1/context_module.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ def module_item_json(content_tag, current_user, session, context_module = nil, p
5656
hash = api_json(content_tag, current_user, session, :only => MODULE_ITEM_JSON_ATTRS)
5757
hash['type'] = Api::API_DATA_TYPE[content_tag.content_type] || content_tag.content_type
5858

59+
hash['module_id'] = content_tag.context_module_id
60+
5961
# add canvas web url
6062
unless content_tag.content_type == 'ContextModuleSubHeader'
6163
hash['html_url'] = case content_tag.content_type

‎spec/apis/v1/context_module_items_api_spec.rb‎

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@
7676
"title" => @assignment_tag.title,
7777
"indent" => 0,
7878
"completion_requirement" => { "type" => "must_submit" },
79-
"published" => false
79+
"published" => false,
80+
"module_id" => @module1.id
8081
},
8182
{
8283
"type" => "Quiz",
@@ -88,7 +89,8 @@
8889
"title" => @quiz_tag.title,
8990
"indent" => 0,
9091
"completion_requirement" => { "type" => "min_score", "min_score" => 10 },
91-
"published" => true
92+
"published" => true,
93+
"module_id" => @module1.id
9294
},
9395
{
9496
"type" => "Discussion",
@@ -100,15 +102,17 @@
100102
"title" => @topic_tag.title,
101103
"indent" => 0,
102104
"completion_requirement" => { "type" => "must_contribute" },
103-
"published" => true
105+
"published" => true,
106+
"module_id" => @module1.id
104107
},
105108
{
106109
"type" => "SubHeader",
107110
"id" => @subheader_tag.id,
108111
"position" => 4,
109112
"title" => @subheader_tag.title,
110113
"indent" => 0,
111-
"published" => true
114+
"published" => true,
115+
"module_id" => @module1.id
112116
},
113117
{
114118
"type" => "ExternalUrl",
@@ -119,7 +123,8 @@
119123
"title" => @external_url_tag.title,
120124
"indent" => 1,
121125
"completion_requirement" => { "type" => "must_view" },
122-
"published" => true
126+
"published" => true,
127+
"module_id" => @module1.id
123128
}
124129
]
125130
end
@@ -164,7 +169,8 @@
164169
"indent" => 0,
165170
"url" => "http://www.example.com/api/v1/courses/#{@course.id}/pages/#{@wiki_page.url}",
166171
"page_url" => @wiki_page.url,
167-
"published" => true
172+
"published" => true,
173+
"module_id" => @module2.id
168174
}
169175

170176
@attachment_tag.unpublish
@@ -181,7 +187,8 @@
181187
"title" => @attachment_tag.title,
182188
"indent" => 0,
183189
"url" => "http://www.example.com/api/v1/files/#{@attachment.id}",
184-
"published" => false
190+
"published" => false,
191+
"module_id" => @module2.id
185192
}
186193
end
187194

@@ -502,6 +509,56 @@
502509
@module1.reload
503510
@module1.evaluate_for(@student).workflow_state.should == 'unlocked'
504511
end
512+
513+
describe "moving items between modules" do
514+
it "should move a module item" do
515+
old_updated_ats = []
516+
Timecop.freeze(1.minute.ago) do
517+
@module2.touch; old_updated_ats << @module2.updated_at
518+
@module3.touch; old_updated_ats << @module3.updated_at
519+
end
520+
api_call(:put, "/api/v1/courses/#{@course.id}/modules/#{@module2.id}/items/#{@wiki_page_tag.id}",
521+
{:controller => "context_module_items_api", :action => "update", :format => "json",
522+
:course_id => "#{@course.id}", :module_id => "#{@module2.id}", :id => "#{@wiki_page_tag.id}"},
523+
{:module_item => {:module_id => @module3.id}})
524+
525+
@module2.reload.content_tags.map(&:id).should_not be_include @wiki_page_tag.id
526+
@module2.updated_at.should > old_updated_ats[0]
527+
@module3.reload.content_tags.map(&:id).should == [@wiki_page_tag.id]
528+
@module3.updated_at.should > old_updated_ats[1]
529+
end
530+
531+
it "should move completion requirements" do
532+
old_updated_ats = []
533+
Timecop.freeze(1.minute.ago) do
534+
@module1.touch; old_updated_ats << @module1.updated_at
535+
@module2.touch; old_updated_ats << @module2.updated_at
536+
end
537+
api_call(:put, "/api/v1/courses/#{@course.id}/modules/#{@module1.id}/items/#{@assignment_tag.id}",
538+
{:controller => "context_module_items_api", :action => "update", :format => "json",
539+
:course_id => "#{@course.id}", :module_id => "#{@module1.id}", :id => "#{@assignment_tag.id}"},
540+
{:module_item => {:module_id => @module2.id, :position => 2}})
541+
542+
@module1.reload.content_tags.map(&:id).should_not be_include @assignment_tag.id
543+
@module1.updated_at.should > old_updated_ats[0]
544+
@module1.completion_requirements.size.should == 3
545+
@module1.completion_requirements.detect { |req| req[:id] == @assignment_tag.id }.should be_nil
546+
547+
@module2.reload.content_tags.sort_by(&:position).map(&:id).should == [@wiki_page_tag.id, @assignment_tag.id, @attachment_tag.id]
548+
@module2.updated_at.should > old_updated_ats[1]
549+
@module2.completion_requirements.detect { |req| req[:id] == @assignment_tag.id }.should_not be_nil
550+
end
551+
552+
it "should verify the target module is in the course" do
553+
course_with_teacher
554+
mod = @course.context_modules.create!
555+
item = mod.add_item(:type => 'context_module_sub_header', :title => 'blah')
556+
api_call(:put, "/api/v1/courses/#{@course.id}/modules/#{mod.id}/items/#{item.id}",
557+
{:controller => "context_module_items_api", :action => "update", :format => "json",
558+
:course_id => @course.to_param, :module_id => mod.to_param, :id => item.to_param},
559+
{:module_item => {:module_id => @module1.id}}, {}, {:expected_status => 400})
560+
end
561+
end
505562
end
506563

507564
it "should delete a module item" do

0 commit comments

Comments
 (0)