Merge pull request #7534 from donny-wong/v2.7.1

V2.7.1

Author: donny-wong

Changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [v2.7.1]
+
+### 🐛 Bug fixes
+
+- Fix bugs when assigning sections to starter file groups across multiple assignments (#7523)
+
 ## [v2.7.0]
 
 ### 🚨 Breaking changes

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.7.0,PATCH_LEVEL=DEV
+VERSION=v2.7.1,PATCH_LEVEL=DEV

app/controllers/assignments_controller.rb

@@ -438,13 +438,26 @@ def populate_starter_file_manager
         use_rename: g.use_rename,
         files: starter_file_group_file_data(g) }
     end
+    sections = current_course.sections.pluck(:id, :name)
     section_data = current_course.sections
-                                 .left_outer_joins(:starter_file_groups)
-                                 .order(:id)
+                                 .joins(:starter_file_groups)
+                                 .where('starter_file_groups.assessment_id': assignment.id)
                                  .pluck_to_hash('sections.id as section_id',
                                                 'sections.name as section_name',
                                                 'starter_file_groups.id as group_id',
                                                 'starter_file_groups.name as group_name')
+    section_data_ids = section_data.pluck(:section_id)
+    sections.each do |id, name|
+      unless section_data_ids.include? id
+        section_data << {
+          section_id: id,
+          section_name: name,
+          group_id: nil,
+          group_name: nil
+        }
+      end
+    end
+    section_data.sort_by! { |x| x[:section_id] }
     data = { files: file_data,
              sections: section_data,
              available_after_due: assignment.starter_files_after_due,

app/models/section.rb

@@ -24,10 +24,11 @@ def starter_file_group_for(assessment)
   end
 
   def update_starter_file_group(assessment_id, starter_file_group_id)
-    return if starter_file_groups.where(assessment_id: assessment_id).first&.id == starter_file_group_id
+    starter_files_groups_for_assignment = starter_file_groups.where(assessment_id: assessment_id)
+    return if starter_files_groups_for_assignment.first&.id == starter_file_group_id
 
     # delete all old section starter file groups
-    section_starter_file_groups.where.not(starter_file_group_id: starter_file_group_id).find_each(&:destroy)
+    section_starter_file_groups.where(starter_file_group_id: starter_files_groups_for_assignment).find_each(&:destroy)
 
     unless starter_file_group_id.nil?
       SectionStarterFileGroup.find_or_create_by(section_id: self.id, starter_file_group_id: starter_file_group_id)

spec/controllers/assignments_controller_spec.rb

@@ -1033,13 +1033,38 @@
         let(:params) { { course_id: assignment.course.id, id: ssfg.starter_file_group.assignment.id } }
 
         it 'should contain the right values' do
+          expect(response.parsed_body['sections'].size).to eq 1
           file_data = response.parsed_body['sections'].first
           expected = { section_id: section.id,
                        section_name: section.name,
                        group_id: starter_file_group.id,
                        group_name: starter_file_group.name }
           expect(file_data).to eq(expected.transform_keys(&:to_s))
         end
+
+        context 'when there is another assignment with sections' do
+          let(:a2) { create(:assignment) }
+          let(:other_starter_file_group) { create(:starter_file_group, assignment: a2) }
+          let(:ssfg2) do
+            create(:section_starter_file_group, starter_file_group: other_starter_file_group, section: section)
+          end
+          let(:params) do
+            # Ensure creation of both assignments and starter file groups
+            ssfg
+            ssfg2
+            { course_id: assignment.course.id, id: ssfg.starter_file_group.assignment.id }
+          end
+
+          it 'should only contain the section data for the requested assignment' do
+            expect(response.parsed_body['sections'].size).to eq 1
+            file_data = response.parsed_body['sections'].first
+            expected = { section_id: section.id,
+                         section_name: section.name,
+                         group_id: starter_file_group.id,
+                         group_name: starter_file_group.name }
+            expect(file_data).to eq(expected.transform_keys(&:to_s))
+          end
+        end
       end
     end
 

spec/models/section_spec.rb

@@ -102,7 +102,7 @@
       end
     end
 
-    context 'when a starter file group is already assigned' do
+    context 'when a starter file group is already assigned for that assignment' do
       let(:section) { sections.second }
 
       before { section.update_starter_file_group(assignment.id, starter_file_groups.first.id) }
@@ -117,5 +117,26 @@
         expect(ids).not_to include starter_file_groups.second.id
       end
     end
+
+    context 'when a starter file group is assigned for a different assignment' do
+      let(:assignment2) { create(:assignment) }
+      let!(:starter_file_groups2) { create_list(:starter_file_group_with_entries, 2, assignment: assignment2) }
+      let(:section) { sections.second }
+
+      before do
+        create(:section_starter_file_group, starter_file_group: starter_file_groups2.second, section: section)
+        section.update_starter_file_group(assignment.id, starter_file_groups.first.id)
+      end
+
+      it 'should assign the new starter file group' do
+        ids = section.reload.section_starter_file_groups.pluck(:starter_file_group_id)
+        expect(ids).to include starter_file_groups.first.id
+      end
+
+      it 'should not remove the starter file group for the other assignment' do
+        ids = section.reload.section_starter_file_groups.pluck(:starter_file_group_id)
+        expect(ids).to include starter_file_groups2.second.id
+      end
+    end
   end
 end