refactor specs and add module_releases shared context

app/assets/stylesheets/application.scss

@@ -195,6 +195,6 @@ ul>li>ul>li {
   white-space: pre-wrap;
 }
 
-#available .govuk-tag {
+#available h2 .govuk-tag {
   margin-left: govuk-spacing(1);
 }

app/models/user.rb

@@ -388,8 +388,8 @@ def continue_training_recipient?
   end
 
   # @return [VisitChanges] changes since last visit
-  def visit_changes
-    @visit_changes ||= VisitChanges.new(user: self)
+  def content_changes
+    @content_changes ||= ContentChanges.new(user: self)
   end
 
 private

app/services/visit_changes.rb → app/services/content_changes.rb

@@ -1,27 +1,20 @@
-# Changes since the user's last visit
+# Content changes since the user's last visit, powered by Ahoy::Visit
 #
-# ContentChanges (powered by visits) might be more appropriate?
-class VisitChanges
+class ContentChanges
   extend Dry::Initializer
 
   option :user, required: true
 
   # @return [Boolean]
   def new_modules?
-    # for consistency change me too
+    return false if previous_visit.nil?
+
     previous_visit && new_modules.any?
   end
 
   # @param mod [Training::Module]
   # @return [Boolean]
   def new_module?(mod)
-    # chained conditions
-    previous_visit &&
-      previous_visit.started_at.to_i < mod.first_published_at.to_i &&
-      !user.course.started?(mod)
-
-    # or
-    # primary condition with two guards
     return false if previous_visit.nil? || user.course.started?(mod)
 
     previous_visit.started_at < mod.first_published_at

app/views/learning/_card.html.slim

@@ -2,7 +2,7 @@
   .card-container
     = training_module_image(mod)
 
-    - if current_user.visit_changes.new_module?(mod)
+    - if current_user.content_changes.new_module?(mod)
       span.govuk-tag.govuk-tag--ey
         = t('my_learning.new_tag.card')
 

app/views/learning/show.html.slim

@@ -29,8 +29,8 @@
     .govuk-grid-column-full
       h2.govuk-heading-m
         | Available modules
-        - if current_user.visit_changes.new_modules?
-          span.govuk-tag.section-tag
+        - if current_user.content_changes.new_modules?
+          span.govuk-tag
             = t('my_learning.new_tag.section')
       - if current_user.course.available_modules.any?
         .grid-container

spec/models/data_analysis/user_overview_spec.rb

@@ -2,6 +2,7 @@
 
 RSpec.describe DataAnalysis::UserOverview do
   include_context 'with progress'
+  include_context 'with module releases'
   let(:headers) do
     [
       'Registration Complete',
@@ -73,11 +74,6 @@
   let(:release_1) { create(:release) }
 
   before do
-    # create records for the previously released modules completed by the `new_module_mail_recipients`
-    create(:module_release, release_id: release_1.id, module_position: 1, name: 'alpha')
-    create(:module_release, release_id: release_1.id, module_position: 2, name: 'bravo')
-    create(:module_release, release_id: release_1.id, module_position: 3, name: 'charlie')
-
     # create notes for the `with_notes` and `without_notes` users
     create(:note, user: user_1)
     create(:note, user: user_2)

spec/models/training/module_spec.rb

@@ -67,4 +67,12 @@
       expect(mod.topic_count).to eq 8 # 4, 1, 3
     end
   end
+
+  describe '#first_published_at' do
+    include_context 'with module releases'
+
+    it 'returns the first published date' do
+      expect(mod.first_published_at).to be_within(1.second).of 2.days.ago
+    end
+  end
 end

spec/services/content_changes_spec.rb

@@ -0,0 +1,81 @@
+require 'rails_helper'
+
+RSpec.describe ContentChanges do
+  subject(:changes) { described_class.new(user: user) }
+
+  let(:user) { create(:user) }
+
+  include_context 'with module releases'
+
+  describe '#new_modules?' do
+    context 'without previous visits' do
+      it 'returns false' do
+        expect(changes.new_modules?).to be false
+      end
+    end
+
+    context 'with visits predating a modules release' do
+      before do
+        create :visit,
+               id: 1,
+               visitor_token: '123',
+               user_id: user.id,
+               started_at: 1.day.ago
+
+        create :visit,
+               id: 2,
+               visitor_token: '456',
+               user_id: user.id,
+               started_at: 1.minute.ago
+      end
+
+      it 'returns true' do
+        expect(changes.new_modules?).to be true
+      end
+    end
+  end
+
+  describe '#new_module?' do
+    let(:alpha) { Training::Module.by_name(:alpha) }
+
+    context 'without previous visits' do
+      it 'returns false' do
+        expect(changes.new_module?(alpha)).to be false
+      end
+    end
+
+    context 'with visits since the modules release' do
+      before do
+        create :visit,
+               id: 1,
+               visitor_token: '123',
+               user_id: user.id,
+               started_at: 1.minute.ago
+      end
+
+      it 'returns false' do
+        expect(changes.new_module?(alpha)).to be false
+      end
+    end
+
+    context 'with visits predating a modules release' do
+      before do
+        create :visit,
+               id: 1,
+               visitor_token: '123',
+               user_id: user.id,
+               started_at: 5.days.ago
+
+        create :visit,
+               id: 2,
+               visitor_token: '456',
+               user_id: user.id,
+               started_at: 5.days.ago
+      end
+
+      it 'returns true' do
+        expect(changes.new_module?(alpha)).to be true
+      end
+    end
+  end
+end

spec/support/shared/with_module_releases.rb

@@ -0,0 +1,12 @@
+RSpec.shared_context 'with module releases' do
+  before do
+    create_module_release(1, 'alpha', 2.days.ago)
+    create_module_release(2, 'bravo', 3.days.ago)
+    create_module_release(3, 'charlie', 2.minutes.ago)
+  end
+
+  def create_module_release(id, name, first_published_at)
+    create(:release, id: id)
+    create(:module_release, release_id: id, module_position: id, name: name, first_published_at: first_published_at)
+  end
+end