diff --git a/app/models/course.rb b/app/models/course.rb index 8055816a13..4b8393e9f6 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -18,6 +18,10 @@ def progress_for(user) end def next_lesson(lesson) - lessons.where('lessons.position > ?', lesson.position).first + lessons.where(lessons: { position: lesson.position.next.. }).first + end + + def previous_lesson(lesson) + lessons.where(lessons: { position: ...lesson.position }).last end end diff --git a/app/views/lessons/_lesson_buttons.html.erb b/app/views/lessons/_lesson_buttons.html.erb index 3c1bb7b570..7cb880bb2f 100644 --- a/app/views/lessons/_lesson_buttons.html.erb +++ b/app/views/lessons/_lesson_buttons.html.erb @@ -1,29 +1,43 @@ -
- <%= link_to path_course_path(course.path, course), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'view-course-btn' } do %> - <%= inline_svg_tag 'icons/academic-cap.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'View course', desc: 'Go to course page' %> - View Course - <% end %> - - <% if user_signed_in? %> - <%= render Complete::ButtonComponent.new(lesson:) %> +
+ <% if course.previous_lesson(lesson).present? %> +
+ <%= link_to lesson_path(course.previous_lesson(lesson)), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'previous-lesson-btn' } do %> + <%= inline_svg_tag 'icons/arrow-left-circle.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Previous lesson', desc: 'Go to previous lesson' %> + Previous Lesson + <% end %> +
<% else %> - <%= link_to( - 'Sign in to track progress', - sign_in_path, - class: 'button button--primary', - data: { test_id: 'sign_in_button' } - ) %> + <% end %> - <% if course.next_lesson(lesson).present? %> - <%= link_to lesson_path(course.next_lesson(lesson)), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'next-lesson-btn' } do %> - <%= inline_svg_tag 'icons/arrow-right-circle.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Next lesson', desc: 'Go to next lesson' %> - Next Lesson +
+ <% if user_signed_in? %> + <%= render Complete::ButtonComponent.new(lesson:) %> + <% else %> + <%= link_to( + 'Sign in to track progress', + sign_in_path, + class: 'button button--primary md:w-60', + data: { test_id: 'sign_in_button' } + ) %> <% end %> +
+ + <% if course.next_lesson(lesson).present? %> +
+ <%= link_to lesson_path(course.next_lesson(lesson)), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'next-lesson-btn' } do %> + <%= inline_svg_tag 'icons/arrow-right-circle.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Next lesson', desc: 'Go to next lesson' %> + Next Lesson + <% end %> +
<% elsif lesson.choose_path_lesson? %> - <%= link_to paths_url, class: 'button button--secondary px-4 font-medium', data: { test_id: 'choose-path-lesson-btn' } do %> - <%= inline_svg_tag 'icons/map.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Choose path', desc: 'Go to paths page' %> - Choose Path - <% end %> +
+ <%= link_to paths_url, class: 'button button--secondary px-4 font-medium', data: { test_id: 'choose-path-lesson-btn' } do %> + <%= inline_svg_tag 'icons/map.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Choose path', desc: 'Go to paths page' %> + Choose Path + <% end %> +
+ <% else %> + <% end %>
diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index 39379b2bed..d6a90510c5 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -37,10 +37,24 @@ it 'returns the next lesson' do course = create(:course) section = create(:section, course:) - lesson_one = create(:lesson, position: 1, section:) - lesson_two = create(:lesson, position: 2, section:) + next_lesson = create(:lesson, position: 3, section:) + current_lesson = create(:lesson, position: 2, section:) + _previous_lesson = create(:lesson, position: 1, section:) - expect(course.next_lesson(lesson_one)).to eql(lesson_two) + expect(course.next_lesson(current_lesson)).to eql(next_lesson) + end + end + + context 'when the next lesson is in a later section' do + it 'returns the lowest-position lesson after the current one' do + course = create(:course) + section = create(:section, position: 1, course:) + next_section = create(:section, position: 2, course:) + current_lesson = create(:lesson, position: 1, section:) + first_lesson_in_next_section = create(:lesson, position: 2, section: next_section) + _second_lesson_in_next_section = create(:lesson, position: 3, section: next_section) + + expect(course.next_lesson(current_lesson)).to eql(first_lesson_in_next_section) end end @@ -48,9 +62,46 @@ it 'returns nothing' do course = create(:course) section = create(:section, course:) - lesson_one = create(:lesson, position: 1, section:) + current_lesson = create(:lesson, position: 1, section:) + + expect(course.next_lesson(current_lesson)).to be_nil + end + end + end + + describe '#previous_lesson' do + context 'when there is a previous lesson' do + it 'returns the previous lesson' do + course = create(:course) + section = create(:section, course:) + current_lesson = create(:lesson, position: 2, section:) + previous_lesson = create(:lesson, position: 1, section:) + _next_lesson = create(:lesson, position: 3, section:) + + expect(course.previous_lesson(current_lesson)).to eql(previous_lesson) + end + end + + context 'when the previous lesson is in an earlier section' do + it 'returns the highest-position lesson before the current one' do + course = create(:course) + section = create(:section, position: 1, course:) + next_section = create(:section, position: 2, course:) + _first_lesson_in_previous_section = create(:lesson, position: 1, section:) + last_lesson_in_previous_section = create(:lesson, position: 2, section:) + current_lesson = create(:lesson, position: 3, section: next_section) + + expect(course.previous_lesson(current_lesson)).to eql(last_lesson_in_previous_section) + end + end + + context 'when there is no previous lesson' do + it 'returns nothing' do + course = create(:course) + section = create(:section, course:) + current_lesson = create(:lesson, position: 1, section:) - expect(course.next_lesson(lesson_one)).to be_nil + expect(course.previous_lesson(current_lesson)).to be_nil end end end diff --git a/spec/system/lesson_navigation_spec.rb b/spec/system/lesson_navigation_spec.rb index 5334f30d15..e93191bc1f 100644 --- a/spec/system/lesson_navigation_spec.rb +++ b/spec/system/lesson_navigation_spec.rb @@ -23,13 +23,15 @@ context 'when on the last lesson of a section' do let!(:next_section) { create(:section, position: 2, course:) } - let!(:next_section_lesson) { create(:lesson, position: 2, section: next_section) } + let!(:first_lesson_in_next_section) { create(:lesson, position: 2, section: next_section) } + + before { create(:lesson, position: 3, section: next_section) } it 'moves to the first lesson in the next section when clicked' do visit lesson_path(lesson) find(:test_id, 'next-lesson-btn').click - expect(find(:test_id, 'lesson-title-header')).to have_text(/#{next_section_lesson.title}/i) + expect(find(:test_id, 'lesson-title-header')).to have_text(/#{first_lesson_in_next_section.title}/i) end end @@ -44,15 +46,39 @@ end end - describe 'the View Course button' do - it 'directs to the course view' do - visit lesson_path(lesson) - find(:test_id, 'view-course-btn').click + describe 'the previous lesson button' do + context 'when the previous lesson is within the same section' do + let!(:current_lesson) { create(:lesson, position: 2, section:) } + + it 'moves to the previous lesson in the section when clicked' do + visit lesson_path(current_lesson) + find(:test_id, 'previous-lesson-btn').click + + expect(find(:test_id, 'lesson-title-header')).to have_text(/#{lesson.title}/i) + end + end + + context 'when on the first lesson of a section' do + let!(:next_section) { create(:section, position: 2, course:) } + let!(:next_section_lesson) { create(:lesson, position: 3, section: next_section) } + + it 'moves to the last lesson in the previous section when clicked' do + last_lesson_in_previous_section = create(:lesson, position: 2, section:) - expect(find(:test_id, 'course-title-header')).to have_text(/#{course.title}/i) + visit lesson_path(next_section_lesson) + find(:test_id, 'previous-lesson-btn').click + + expect(find(:test_id, 'lesson-title-header')).to have_text(/#{last_lesson_in_previous_section.title}/i) + end + end + + context 'when on the first lesson in the course' do + it 'is not present' do + visit lesson_path(lesson) + + expect(page).to have_css('[data-test-id]') - within '[data-test-id="lesson-list"]', match: :first do - expect(page).to have_text(/#{lesson.title}/i) + expect(page).to have_no_css('[data-test-id="previous-lesson-btn"]') end end end