Skip to content

Commit

Permalink
Revert migration and content update (#1285)
Browse files Browse the repository at this point in the history
* Revert "Revert "ER-1271 Delete old user_answers and user_assessments tables (#1267)" (#1284)"

This reverts commit f265902.

* My Account update and feedback controller spec update

* Ensure fallback to guest is always possible

web crawlers or curl requests do not instantiate new visit records

* Update lying spec!

---------

Co-authored-by: Peter David Hamilton <[email protected]>
  • Loading branch information
martikat and peterdavidhamilton authored Jul 26, 2024
1 parent 3b3be30 commit 65f0c3f
Show file tree
Hide file tree
Showing 15 changed files with 123 additions and 125 deletions.
6 changes: 3 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ def current_user
super
end

# @return [Guest, nil]
# @return [Guest]
def guest
visit = Visit.find_by(visit_token: cookies[:course_feedback]) || current_visit
Guest.new(visit: visit) if visit.present?
visit = Visit.find_by(visit_token: cookies[:course_feedback]) || current_visit || Visit.new
Guest.new(visit: visit)
end

# @see Auditing
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/feedback_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def content
mod.page_by_name(content_name)
end

# @return [User, Guest, nil]
# @return [User, Guest]
def current_user
super || guest
end
Expand Down
10 changes: 5 additions & 5 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ def navigation
render(HeaderComponent.new(service_name: service_name, classes: 'dfe-header noprint', navigation_label: 'Primary navigation')) do |header|
header.with_navigation_item(text: 'Home', href: root_path, classes: %w[dfe-header__navigation-item])
if user_signed_in?
header.with_action_link(text: 'My Account', href: user_path, options: { inverse: true })
header.with_action_link(text: t('my_account.title'), href: user_path, options: { inverse: true })
header.with_action_link(text: 'Sign out', href: destroy_user_session_path, options: { id: 'sign-out-desktop', data: { turbo_method: :get }, inverse: true })
header.with_navigation_item(text: 'My modules', href: my_modules_path, classes: %w[dfe-header__navigation-item])
header.with_navigation_item(text: 'Learning log', href: user_notes_path, classes: %w[dfe-header__navigation-items]) if current_user.course_started?
header.with_navigation_item(text: t('my_learning.title'), href: my_modules_path, classes: %w[dfe-header__navigation-item])
header.with_navigation_item(text: t('my_learning_log.title'), href: user_notes_path, classes: %w[dfe-header__navigation-items]) if current_user.course_started?
else
header.with_action_link(text: 'Sign in', href: new_user_session_path, options: { inverse: true })
header.with_navigation_item(text: 'Sign in', href: new_user_session_path, classes: %w[dfe-header__navigation-item dfe-header-f-mob])
header.with_action_link(text: t('account_login.title'), href: new_user_session_path, options: { inverse: true })
header.with_navigation_item(text: t('account_login.title'), href: new_user_session_path, classes: %w[dfe-header__navigation-item dfe-header-f-mob])
end
end
end
Expand Down
7 changes: 0 additions & 7 deletions app/views/devise/shared/_form_wrap.html.slim

This file was deleted.

19 changes: 0 additions & 19 deletions app/views/devise/shared/_links.html.slim

This file was deleted.

11 changes: 0 additions & 11 deletions app/views/devise/unlocks/new.html.slim

This file was deleted.

4 changes: 2 additions & 2 deletions app/views/learning/show.html.slim
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
- content_for :page_title do
= html_title t(:title, scope: :my_learning)
= html_title t('my_learning.title')

- content_for :hero do
.govuk-grid-row class='govuk-!-padding-top-9 govuk-!-padding-bottom-0'
.govuk-grid-column-three-quarters
h1.dfe-heading-xl class='govuk-!-margin-bottom-4'
| My modules
= t('my_learning.title')
p You can complete the Early years child development training in any order. However, to support your understanding of the training, you may find it helpful to complete the modules in order.

- content_for :cta do
Expand Down
2 changes: 1 addition & 1 deletion app/views/training/assessments/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ hr.govuk-section-break.govuk-section-break--l.govuk-section-break--visible class
= link_to_next

- else
= govuk_button_link_to 'Go to My modules', my_modules_path, secondary: true
= govuk_button_link_to t('links.my_modules'), my_modules_path, secondary: true

= govuk_button_link_to 'Retake test', training_module_page_path(mod.name, mod.assessment_intro_page.name)
4 changes: 2 additions & 2 deletions app/views/training/pages/certificate.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

p.govuk_body
| If the name on your certificate is not what you expect, you can update it in
=< govuk_link_to 'My account', user_path
=< govuk_link_to t('my_account.title'), user_path
| .

- content_for :cta do
Expand Down Expand Up @@ -72,4 +72,4 @@

hr.govuk-section-break.govuk-section-break--l.govuk-section-break--visible class='govuk-!-margin-top-4'

= govuk_button_link_to 'Go to My modules', my_modules_path
= govuk_button_link_to t('links.my_modules'), my_modules_path
23 changes: 23 additions & 0 deletions db/migrate/20240717072502_drop_user_answers_table.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
class DropUserAnswersTable < ActiveRecord::Migration[7.1]
def up
drop_table :user_answers
end

def down
create_table :user_answers do |t|
t.references :user, null: false, foreign_key: true
t.integer :questionnaire_id, null: false, index: true
t.string :question
t.string :answer
t.boolean :correct
t.boolean :archived
t.index %i[questionnaire_id user_id]
t.timestamps
t.string :module
t.string :name
t.string :assessments_type
t.references :user_assessment, null: true, foreign_key: true
t.string :state
end
end
end
19 changes: 19 additions & 0 deletions db/migrate/20240717072554_drop_user_assessments_table.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
class DropUserAssessmentsTable < ActiveRecord::Migration[7.1]
def up
drop_table :user_assessments
end

def down
create_table :user_assessments do |t|
t.references :user, null: false, foreign_key: true
t.string :score
t.string :status
t.string :module
t.string :assessments_type
t.boolean :archived
t.datetime :completed
t.index %i[score status]
t.timestamps
end
end
end
39 changes: 1 addition & 38 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 25 additions & 13 deletions spec/controllers/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,35 @@
describe '#guest' do
subject(:guest) { controller.send(:guest) }

let(:cookie_token) { 'some-token' }

it 'is nil without a current_visit' do
expect(guest).to be_nil
context 'with a bot' do
it 'instantiates a new visit' do
expect(guest.visit.id).to be_nil
expect(guest).to be_a Guest
end
end

it 'is a Guest with a current_visit' do
allow(controller).to receive(:current_visit).and_return(create(:visit))
expect(guest).to be_a Guest
context 'with a browser' do
let(:current_visit) { create(:visit) }

it 'uses the current visit' do
allow(controller).to receive(:current_visit).and_return(current_visit)
expect(guest.visit.id).to eq current_visit.id
expect(guest).to be_a Guest
end
end

it 'restores a previous visit from a cookie' do
allow(controller).to receive(:current_visit).and_return(create(:visit))
create(:visit, visit_token: cookie_token)
request.cookies[:course_feedback] = cookie_token
expect(guest).to be_a Guest
expect(guest.visit_token).to eq cookie_token
context 'with a cookie' do
let(:cookie_token) { 'some-token' }

before do
create(:visit, visit_token: cookie_token)
request.cookies[:course_feedback] = cookie_token
end

it 'fetches a previous visit' do
expect(guest).to be_a Guest
expect(guest.visit_token).to eq cookie_token
end
end
end
end
9 changes: 9 additions & 0 deletions spec/controllers/feedback_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
require 'rails_helper'

RSpec.describe FeedbackController, type: :controller do
context 'when user is a guest' do
describe 'GET #show' do
it 'returns a success response' do
get :show, params: { id: 'feedback-radio-only' }
expect(response).to have_http_status(:success)
end
end
end

context 'when user is signed in' do
let(:user) { create :user, :registered }

Expand Down
55 changes: 32 additions & 23 deletions spec/system/feedback_spec.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
require 'rails_helper'

RSpec.describe 'Feedback' do
before do
visit '/'
end

describe 'in beta banner' do
specify do
visit '/'
within '.govuk-phase-banner' do
expect(page).to have_text 'This is a new service, your feedback will help us improve it.'
expect(page).to have_link 'feedback', href: '/feedback'
Expand All @@ -16,35 +13,47 @@

describe 'in footer' do
specify do
visit '/'
within '.govuk-footer' do
expect(page).to have_link 'Feedback', href: '/feedback'
end
end
end

describe 'questions' do
include_context 'with user'

it 'have previous/next buttons' do
visit '/my-account'
visit '/feedback'
click_on 'Next'
expect(page).to have_current_path '/feedback/feedback-radio-only'
expect(Event.last.name).to eq 'feedback_intro'
expect(page).to have_link 'Previous', href: '/feedback'
expect(page).to have_button 'Next'
expect(page).not_to have_button 'Save'
context 'with guest' do
it 'has previous/next buttons' do
visit '/feedback/feedback-radio-only'
expect(page).to have_text 'Feedback radio buttons only'
expect(page).to have_link 'Previous', href: '/feedback'
expect(page).to have_button 'Next'
end
end

context 'when updated from account profile' do
it 'have a save button' do
context 'with user' do
include_context 'with user'

it 'has previous/next buttons' do
visit '/my-account'
click_on 'Change research preferences'
expect(page).to have_current_path '/feedback/feedback-skippable'
expect(Event.last.name).to eq 'profile_page'
expect(page).not_to have_link 'Previous'
expect(page).not_to have_button 'Next'
expect(page).to have_button 'Save'
visit '/feedback'
click_on 'Next'
expect(page).to have_current_path '/feedback/feedback-radio-only'
expect(Event.last.name).to eq 'feedback_intro'
expect(page).to have_link 'Previous', href: '/feedback'
expect(page).to have_button 'Next'
expect(page).not_to have_button 'Save'
end

context 'when updated from account profile' do
it 'has a save button' do
visit '/my-account'
click_on 'Change research preferences'
expect(page).to have_current_path '/feedback/feedback-skippable'
expect(Event.last.name).to eq 'profile_page'
expect(page).not_to have_link 'Previous'
expect(page).not_to have_button 'Next'
expect(page).to have_button 'Save'
end
end
end
end
Expand Down

0 comments on commit 65f0c3f

Please sign in to comment.