ER-467 New Module Indicator (#924)

* check if module has been released since user's last visit

* update account page for one login

* Add new module indicator tag

* revert account page changes

* temporarily remove conditional logic from tags to aid reviewing

* fix merge conflicts

* prevent any available or complete modules from showing new module indicator

* add check for module release record to new mod method

* rubocop

* move styling to stylesheet

* revert schema changes

* move new module logic to new service

* VisitChanges
  #new_modules
    without previous visits
      returns no modules
    with previous visits
      returns newly released modules

* add started guard clause to new tag

* guarding alternatives

* refactor specs and add module_releases shared context

* add module_release spec

* add test for new module indicator on in progress modules

* update account page for one login

* remove unnecessary visits from spec

* revert account changes

* Merge remote-tracking branch 'origin/main' into new-module-indicator

* revert github workflows

* revert erroneous changes

* revert changes to course_progress

* undo revert to course_progress

---------

Co-authored-by: Peter David Hamilton <[email protected]>

app/assets/stylesheets/application.scss

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

app/assets/stylesheets/card.scss

@@ -54,6 +54,11 @@ Progress bar
     max-width: 300px;
   }
 
+.govuk-tag--ey {
+  width: min-content;
+  margin: govuk-spacing(1) 0;
+}
+
   &-container {
     display: flex;
     flex-direction: column;
@@ -108,6 +113,11 @@ Progress bar
     .bar-ball {
       background-color: govuk-colour('yellow');
     }
+
+    .govuk-tag {
+      background-color: govuk-colour('light-grey');
+      color: govuk-colour('blue');
+    }
   }
 
   &:hover &-link--retake {

app/models/training/module.rb

@@ -60,6 +60,11 @@ def debug_summary
       SUMMARY
     end
 
+    # @return [DateTime, nil]
+    def first_published_at
+      ModuleRelease.find_by(module_position: position)&.first_published_at
+    end
+
     # entry references ---------------------------------
 
     # @example

app/models/user.rb

@@ -387,6 +387,11 @@ def continue_training_recipient?
     old_visits.pluck(:user_id).include?(id)
   end
 
+  # @return [VisitChanges] changes since last visit
+  def content_changes
+    @content_changes ||= ContentChanges.new(user: self)
+  end
+
 private
 
   # @return [Hash]

app/services/content_changes.rb

@@ -0,0 +1,34 @@
+# Content changes since the user's last visit, powered by Ahoy::Visit
+#
+class ContentChanges
+  extend Dry::Initializer
+
+  option :user, required: true
+
+  # @return [Boolean]
+  def new_modules?
+    return false if previous_visit.nil?
+
+    new_modules.any?
+  end
+
+  # @param mod [Training::Module]
+  # @return [Boolean]
+  def new_module?(mod)
+    return false if previous_visit.nil? || user.course.started?(mod)
+
+    previous_visit.started_at < mod.first_published_at
+  end
+
+  # @return [Array<Training::Module>]
+  def new_modules
+    user.course.available_modules.select { |mod| new_module?(mod) }
+  end
+
+private
+
+  # @return [Ahoy::Visit]
+  def previous_visit
+    user.visits.order(started_at: :desc).second
+  end
+end

app/services/course_progress.rb

@@ -29,7 +29,7 @@ def completed_modules
 
   # @return [Boolean]
   def course_completed?
-    published_modules.all? { |mod| completed?(mod) }
+    Training::Module.live.all? { |mod| completed?(mod) }
   end
 
   # @return [Array<String>]
@@ -58,8 +58,6 @@ def completed?(mod)
     module_progress(mod).completed?
   end
 
-private
-
   # @param mod [Training::Module]
   # @return [Boolean] module content has been viewed
   def started?(mod)
@@ -68,6 +66,8 @@ def started?(mod)
     module_progress(mod).started?
   end
 
+private
+
   # @param mod [Training::Module]
   # @return [Boolean]
   def active?(mod)
@@ -84,24 +84,14 @@ def upcoming?(mod)
   # @return [Array<Training::Module>] training modules by state
   def by_state(state)
     case state.to_sym
-    when :active    then training_modules.select { |mod| active?(mod) }
-    when :upcoming  then training_modules.select { |mod| upcoming?(mod) }
-    when :completed then training_modules.select { |mod| completed?(mod) }
+    when :active    then Training::Module.ordered.select { |mod| active?(mod) }
+    when :upcoming  then Training::Module.ordered.select { |mod| upcoming?(mod) }
+    when :completed then Training::Module.ordered.select { |mod| completed?(mod) }
     else
       raise 'CourseProgress#by_state can query either :active, :upcoming or :completed modules'
     end
   end
 
-  # @return [Array<Training::Module>] training modules with finalised content
-  def published_modules
-    training_modules.reject(&:draft?)
-  end
-
-  # @return [Array<Training::Module>]
-  def training_modules
-    @training_modules ||= Training::Module.ordered
-  end
-
   # @return [ModuleProgress]
   def module_progress(mod)
     ModuleProgress.new(user: user, mod: mod)

app/views/learning/_card.html.slim

@@ -2,6 +2,10 @@
   .card-container
     = training_module_image(mod)
 
+    - if current_user.content_changes.new_module?(mod)
+      span.govuk-tag.govuk-tag--ey
+        = t('my_learning.new_tag.card')
+
     h3.govuk-heading-s
       = govuk_link_to mod.card_title, training_module_path(mod.name), no_visited_state: true, class: 'card-link--header'
 

app/views/learning/show.html.slim

@@ -27,7 +27,11 @@
   hr.govuk-section-break.govuk-section-break--visible.govuk-section-break--l
   #available.govuk-grid-row
     .govuk-grid-column-full
-      h2.govuk-heading-m Available modules
+      h2.govuk-heading-m
+        | Available modules
+        - 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
           - current_user.course.available_modules.each do |mod|

config/locales/en.yml

@@ -276,6 +276,9 @@ en:
       percentage: "Your progress: %{num}"
       viewed: You have read %{num} pages
       remaining: Only %{num} pages remaining
+    new_tag:
+      card: NEW
+      section: NEW MODULE AVAILABLE
 
   my_learning_log:
     title: Learning log

spec/lib/seed_snippets_spec.rb

@@ -5,7 +5,7 @@
   subject(:locales) { described_class.new.call }
 
   it 'converts all translations' do
-    expect(locales.count).to be 187
+    expect(locales.count).to be 189
   end
 
   it 'dot separated key -> Page::Resource#name' do

spec/models/module_release_spec.rb

@@ -0,0 +1,10 @@
+require 'rails_helper'
+
+RSpec.describe ModuleRelease, type: :model do
+  describe '.ordered' do
+    include_context 'with module releases'
+    it 'returns module releases in order of module position' do
+      expect(described_class.ordered.map(&:module_position)).to eq([1, 2, 3])
+    end
+  end
+end

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,92 @@
+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
+
+      context 'and a module in progress' do
+        include_context 'with progress'
+        before do
+          start_module(alpha)
+        end
+
+        it 'returns false' do
+          expect(changes.new_module?(alpha)).to be false
+        end
+      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