Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ER-467 New Module Indicator #924

Merged
merged 30 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
108e0d1
check if module has been released since user's last visit
jack-coggin Sep 11, 2023
342a89f
Merge branch 'main' into new-module-indicator
jack-coggin Nov 2, 2023
4ccb818
update account page for one login
jack-coggin Nov 14, 2023
2aeffca
Merge branch 'main' into new-module-indicator
jack-coggin Nov 21, 2023
737addb
Add new module indicator tag
jack-coggin Nov 23, 2023
4126c1e
revert account page changes
jack-coggin Nov 23, 2023
43197c7
temporarily remove conditional logic from tags to aid reviewing
jack-coggin Nov 23, 2023
1e529c5
fix merge conflicts
jack-coggin Nov 27, 2023
2b4fe85
prevent any available or complete modules from showing new module ind…
jack-coggin Nov 24, 2023
2d3fa5b
add check for module release record to new mod method
jack-coggin Nov 27, 2023
08121ee
rubocop
jack-coggin Nov 27, 2023
723cbd0
move styling to stylesheet
jack-coggin Nov 28, 2023
95c0f04
revert schema changes
jack-coggin Nov 28, 2023
bd535b9
move new module logic to new service
jack-coggin Nov 29, 2023
5e3c7c7
VisitChanges
peterdavidhamilton Nov 30, 2023
01f8d17
add started guard clause to new tag
jack-coggin Dec 1, 2023
379ebe6
guarding alternatives
peterdavidhamilton Dec 1, 2023
61bf0e5
refactor specs and add module_releases shared context
jack-coggin Dec 1, 2023
375318f
add module_release spec
jack-coggin Dec 1, 2023
5855be9
add test for new module indicator on in progress modules
jack-coggin Dec 5, 2023
2cd7e5f
update account page for one login
jack-coggin Nov 14, 2023
2349224
remove unnecessary visits from spec
jack-coggin Dec 5, 2023
8b38174
resolve merge conflicts
jack-coggin Dec 5, 2023
b2a205d
revert account changes
jack-coggin Dec 5, 2023
22528c1
Merge remote-tracking branch 'origin/main' into new-module-indicator
jack-coggin Dec 5, 2023
cd83ebc
Merge remote-tracking branch 'origin/main' into new-module-indicator
jack-coggin Dec 5, 2023
2acff4a
revert github workflows
jack-coggin Dec 5, 2023
f0b4133
revert erroneous changes
jack-coggin Dec 5, 2023
4840469
revert changes to course_progress
jack-coggin Dec 5, 2023
a8300f5
undo revert to course_progress
jack-coggin Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/assets/stylesheets/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
10 changes: 10 additions & 0 deletions app/assets/stylesheets/card.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions app/models/training/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
34 changes: 34 additions & 0 deletions app/services/content_changes.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions app/views/learning/_card.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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
peterdavidhamilton marked this conversation as resolved.
Show resolved Hide resolved
= govuk_link_to mod.card_title, training_module_path(mod.name), no_visited_state: true, class: 'card-link--header'

Expand Down
6 changes: 5 additions & 1 deletion app/views/learning/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
3 changes: 3 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/seed_snippets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions spec/models/module_release_spec.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions spec/models/training/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
92 changes: 92 additions & 0 deletions spec/services/content_changes_spec.rb
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions spec/support/shared/with_module_releases.rb
Original file line number Diff line number Diff line change
@@ -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
Loading