refactor(reference_timelines_controller): remove bulk times creation

Author: purfectliterature

app/controllers/course/reference_timelines_controller.rb

@@ -4,28 +4,24 @@ class Course::ReferenceTimelinesController < Course::ComponentController
 
   def index
     @timelines = @reference_timelines.includes(:reference_times, :course_users)
+
+    # TODO: [PR#5491] Allow timelines management for items other than assessments
     @items = current_course.lesson_plan_items.
              where(actable_type: Course::Assessment.name).
              order(:title).
              includes(:reference_times)
   end
 
-  def show
-  end
-
   def create
     if @reference_timeline.save
-      render partial: 'reference_timeline', locals: {
-        reference_timeline: @reference_timeline,
-        render_times: reference_timeline_params[:reference_times_attributes].present?
-      }
+      render partial: 'reference_timeline', locals: { timeline: @reference_timeline }
     else
       render json: { errors: @reference_timeline.errors.full_messages.to_sentence }, status: :bad_request
     end
   end
 
   def update
-    if @reference_timeline.update(reference_timeline_update_params)
+    if @reference_timeline.update(reference_timeline_params)
       head :ok
     else
       render json: { errors: @reference_timeline.errors.full_messages.to_sentence }, status: :bad_request
@@ -54,12 +50,6 @@ def destroy
   private
 
   def reference_timeline_params
-    params.require(:reference_timeline).permit(:title, :weight, {
-      reference_times_attributes: Course::ReferenceTimesController.base_params
-    })
-  end
-
-  def reference_timeline_update_params
     params.require(:reference_timeline).permit(:title, :weight)
   end
 

app/models/course/reference_timeline.rb

@@ -17,8 +17,6 @@ class Course::ReferenceTimeline < ApplicationRecord
 
   default_scope { order(:weight) }
 
-  accepts_nested_attributes_for :reference_times
-
   def initialize_duplicate(duplicator, _other)
     self.course = duplicator.options[:destination_course]
     self.reference_times = []

app/views/course/reference_timelines/_reference_timeline.json.jbuilder

@@ -1,20 +1,8 @@
 # frozen_string_literal: true
-json.id reference_timeline.id
-json.title reference_timeline.title
-json.weight reference_timeline.weight
+json.id timeline.id
+json.title timeline.title || t('.default_title')
+json.timesCount timeline.reference_times.size
 
-if render_times
-  reference_times = reference_timeline.reference_times.includes(:lesson_plan_item)
-  json.times reference_times do |time|
-    json.id time.id
-    json.startAt time.start_at
-    json.bonusEndAt time.bonus_end_at if time.bonus_end_at.present?
-    json.endAt time.end_at if time.end_at.present?
-
-    item = time.lesson_plan_item
-    json.itemId item.id
-    json.actableId item.actable_id
-    json.type item.actable_type.constantize.model_name.human
-    json.title item.title
-  end
-end
+json.weight timeline.weight if timeline.weight.present?
+json.default true if timeline.default?
+json.assignees timeline.course_users.size unless timeline.default?

app/views/course/reference_timelines/index.json.jbuilder

@@ -3,13 +3,7 @@ json.gamified current_course.gamified
 json.defaultTimeline current_course.default_reference_timeline.id
 
 json.timelines @timelines do |timeline|
-  json.id timeline.id
-  json.title timeline.title || t('.default_title')
-  json.timesCount timeline.reference_times.size
-
-  json.weight timeline.weight if timeline.weight.present?
-  json.default true if timeline.default?
-  json.assignees timeline.course_users.size unless timeline.default?
+  json.partial! 'reference_timeline', timeline: timeline
 end
 
 json.items @items do |item|

app/views/course/reference_timelines/show.json.jbuilder

@@ -1,5 +1,5 @@
 en:
   course:
     reference_timelines:
-      index:
+      reference_timeline:
         default_title: 'Default Timeline'

config/locales/en/course/reference_timelines.yml

@@ -444,7 +444,7 @@
         post 'toggle_satisfiability_type' => 'learning_map#toggle_satisfiability_type'
       end
 
-      resources :reference_timelines, path: 'timelines', except: [:new, :edit] do
+      resources :reference_timelines, path: 'timelines', except: [:new, :edit, :show] do
         resources :reference_times, path: 'times', only: [:create, :update, :destroy]
       end
     end

config/routes.rb

@@ -28,22 +28,6 @@
       end
     end
 
-    describe '#show' do
-      subject { get :show, as: :json, params: { course_id: course, id: timeline } }
-
-      context 'when the user is a manager of the course' do
-        let(:user) { create(:course_manager, course: course).user }
-
-        it { is_expected.to render_template(:show) }
-      end
-
-      context 'when the user is a student' do
-        let(:user) { create(:course_student, course: course).user }
-
-        it { expect { subject }.to raise_error(CanCan::AccessDenied) }
-      end
-    end
-
     describe '#create' do
       subject do
         post :create, as: :json, params: {
@@ -65,51 +49,6 @@
           expect(new_timeline.weight).to eq(2)
         end
 
-        context 'when an array of times are given' do
-          let(:item) { create(:course_lesson_plan_item, course: course) }
-          # Convert Ruby Time to string because of differences in micro/nano-second precision between
-          # database and in-memory time representations. See https://stackoverflow.com/a/20403290.
-          let(:start_time) { 1.day.from_now.to_s }
-          let(:bonus_end_time) { 2.days.from_now.to_s }
-          let(:end_time) { 3.days.from_now.to_s }
-
-          subject do
-            post :create, as: :json, params: {
-              course_id: course,
-              reference_timeline: {
-                title: title,
-                reference_times_attributes: [
-                  {
-                    lesson_plan_item_id: item.id,
-                    start_at: start_time,
-                    bonus_end_at: bonus_end_time,
-                    end_at: end_time
-                  }
-                ]
-              }
-            }
-          end
-
-          it 'creates the timeline, the given times, and their associations' do
-            expect { subject }.
-              to change { course.reference_timelines.size }.by(1).
-              and change { item.reference_times.size }.by(1)
-
-            is_expected.to have_http_status(:ok)
-            is_expected.to render_template(partial: '_reference_timeline')
-
-            new_timeline = assigns(:reference_timeline)
-            expect(new_timeline.title).to eq(title)
-            expect(new_timeline.reference_times.size).to eq(1)
-
-            new_time = new_timeline.reference_times.first
-            expect(new_time.lesson_plan_item.id).to eq(item.id)
-            expect(new_time.start_at).to eq(start_time)
-            expect(new_time.bonus_end_at).to eq(bonus_end_time)
-            expect(new_time.end_at).to eq(end_time)
-          end
-        end
-
         context 'when cannot be saved' do
           before do
             allow(timeline).to receive(:save).and_return(false)

spec/controllers/course/reference_timelines_controller_spec.rb

@@ -16,8 +16,6 @@
         dependent(:restrict_with_error)
     }
 
-    it { should accept_nested_attributes_for(:reference_times) }
-
     describe '#validations' do
       let(:title) { 'Test Timeline Title' }