From 194aef39ef3c1d5ba6d2861b061839d2dcd2894f Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Fri, 3 Nov 2023 15:58:57 +0000 Subject: [PATCH 1/9] link name pulls through competent authority (teaching_authority_name) --- .../review_verifications_controller.rb | 8 ++++++ .../review_verifications/index.html.erb | 25 +++++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/app/controllers/assessor_interface/review_verifications_controller.rb b/app/controllers/assessor_interface/review_verifications_controller.rb index 786f1c6f56..3d01009321 100644 --- a/app/controllers/assessor_interface/review_verifications_controller.rb +++ b/app/controllers/assessor_interface/review_verifications_controller.rb @@ -11,6 +11,14 @@ def index @professional_standing_request = assessment.professional_standing_request if assessment.professional_standing_request.verify_failed? + @region = @assessment.application_form.region + + if @region.teaching_authority_name.present? + @teaching_authority_name = @region.teaching_authority_name + else + @teaching_authority_name = "Contact relevant competent authority" + end + render layout: "full_from_desktop" end diff --git a/app/views/assessor_interface/review_verifications/index.html.erb b/app/views/assessor_interface/review_verifications/index.html.erb index 413b5db8a8..6baf3b9944 100644 --- a/app/views/assessor_interface/review_verifications/index.html.erb +++ b/app/views/assessor_interface/review_verifications/index.html.erb @@ -7,17 +7,20 @@ The following verification tasks have been flagged for review:

-<%= govuk_table do |table| - if @professional_standing_request.present? - table.with_caption(text: "LoPS") - table.with_body do |body| - body.with_row do |row| - row.with_cell { govuk_link_to region_certificate_name(@application_form.region), [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request] } - row.with_cell { render(StatusTag::Component.new(@professional_standing_request.review_status)) } - end - end - end -end %> +<%= render(TaskList::Component.new( + [ + { + title: "LoPS", + items: [ + { + name: @teaching_authority_name, + link: [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request], + status: @professional_standing_request.review_status, + } + ], + } + ] +)) %>
<%= govuk_button_link_to "Back to overview", [:assessor_interface, @application_form] %> From 2eed99f2bfeee2ec4f9e5e47aedc5bace7c5234a Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Fri, 3 Nov 2023 18:12:57 +0000 Subject: [PATCH 2/9] fixed failing tests --- .../review_verifications_controller.rb | 10 +++++----- .../reviewing_professional_standing_spec.rb | 13 ++++++------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/controllers/assessor_interface/review_verifications_controller.rb b/app/controllers/assessor_interface/review_verifications_controller.rb index 3d01009321..c42b1e0125 100644 --- a/app/controllers/assessor_interface/review_verifications_controller.rb +++ b/app/controllers/assessor_interface/review_verifications_controller.rb @@ -13,11 +13,11 @@ def index @region = @assessment.application_form.region - if @region.teaching_authority_name.present? - @teaching_authority_name = @region.teaching_authority_name - else - @teaching_authority_name = "Contact relevant competent authority" - end + @teaching_authority_name = + ( + @region.teaching_authority_name.presence || + "Contact relevant competent authority" + ) render layout: "full_from_desktop" end diff --git a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb index f086b04ad5..66c6a5545f 100644 --- a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb +++ b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb @@ -94,8 +94,9 @@ def when_i_click_on_assessment_decision end def when_i_click_on_lops - assessor_review_verifications_page.rows.first.link.click + find(".govuk-link").click end + def when_i_fill_in_the_review_lops_form form = assessor_review_professional_standing_request_page.form @@ -109,15 +110,13 @@ def when_i_click_on_back_to_overview end def and_i_see_the_lops_not_started - expect(assessor_review_verifications_page.rows.first.tag.text).to eq( - "NOT STARTED", - ) + status_text = find(".govuk-tag.govuk-tag--grey.app-task-list__tag").text + expect(status_text).to eq("NOT STARTED") end def and_i_see_the_lops_accepted - expect(assessor_review_verifications_page.rows.first.tag.text).to eq( - "ACCEPTED", - ) + status_text = find('strong.govuk-tag.govuk-tag--green.app-task-list__tag').text + expect(status_text).to eq("ACCEPTED") end def application_form From 4d22373411895a7007fbaccd2ccd415def31938a Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Fri, 3 Nov 2023 18:16:28 +0000 Subject: [PATCH 3/9] fixed linting --- .../reviewing_professional_standing_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb index 66c6a5545f..4c3c73dadf 100644 --- a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb +++ b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb @@ -96,7 +96,6 @@ def when_i_click_on_assessment_decision def when_i_click_on_lops find(".govuk-link").click end - def when_i_fill_in_the_review_lops_form form = assessor_review_professional_standing_request_page.form @@ -115,7 +114,8 @@ def and_i_see_the_lops_not_started end def and_i_see_the_lops_accepted - status_text = find('strong.govuk-tag.govuk-tag--green.app-task-list__tag').text + status_text = + find("strong.govuk-tag.govuk-tag--green.app-task-list__tag").text expect(status_text).to eq("ACCEPTED") end From 00037748924415415cae5c50a3fcd5b982da8d4a Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 6 Nov 2023 03:14:38 +0000 Subject: [PATCH 4/9] Change to make use of helper --- .../review_verifications_controller.rb | 9 ++----- app/helpers/region_helper.rb | 4 +-- .../review_verifications/index.html.erb | 26 ++++++++++--------- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/app/controllers/assessor_interface/review_verifications_controller.rb b/app/controllers/assessor_interface/review_verifications_controller.rb index c42b1e0125..fbd40dcb1c 100644 --- a/app/controllers/assessor_interface/review_verifications_controller.rb +++ b/app/controllers/assessor_interface/review_verifications_controller.rb @@ -2,6 +2,7 @@ module AssessorInterface class ReviewVerificationsController < BaseController + include RegionHelper before_action :authorize_assessor def index @@ -11,13 +12,7 @@ def index @professional_standing_request = assessment.professional_standing_request if assessment.professional_standing_request.verify_failed? - @region = @assessment.application_form.region - - @teaching_authority_name = - ( - @region.teaching_authority_name.presence || - "Contact relevant competent authority" - ) + @teaching_authority_name = region_teaching_authority_name(@assessment.application_form.region, "Contact relevant competent authority") render layout: "full_from_desktop" end diff --git a/app/helpers/region_helper.rb b/app/helpers/region_helper.rb index 6368cef49e..b0e0029e5f 100644 --- a/app/helpers/region_helper.rb +++ b/app/helpers/region_helper.rb @@ -11,8 +11,8 @@ def region_certificate_phrase(region) "#{certificate.indefinite_article} #{tag.span(certificate, lang: region.country.code)}".html_safe end - def region_teaching_authority_name(region) - region.teaching_authority_name.presence || "teaching authority" + def region_teaching_authority_name(region, default = nil) + region.teaching_authority_name.presence || default || "teaching authority" end def region_teaching_authority_name_phrase(region) diff --git a/app/views/assessor_interface/review_verifications/index.html.erb b/app/views/assessor_interface/review_verifications/index.html.erb index 6baf3b9944..042b6be2d5 100644 --- a/app/views/assessor_interface/review_verifications/index.html.erb +++ b/app/views/assessor_interface/review_verifications/index.html.erb @@ -8,18 +8,20 @@

<%= render(TaskList::Component.new( - [ - { - title: "LoPS", - items: [ - { - name: @teaching_authority_name, - link: [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request], - status: @professional_standing_request.review_status, - } - ], - } - ] + if @professional_standing_request.present? + [ + { + title: "LoPS", + items: [ + { + name: @teaching_authority_name, + link: [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request], + status: @professional_standing_request.review_status, + } + ], + } + ] + end )) %>
From 7e712ecb16923bef244a4ca15c98c10f271f6c49 Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 6 Nov 2023 07:28:01 +0000 Subject: [PATCH 5/9] used task list page object for tests --- .../review_verifications_controller.rb | 8 ++++++-- .../assessor_interface/review_verifications.rb | 2 ++ spec/support/autoload/page_objects/task_list.rb | 3 +++ .../reviewing_professional_standing_spec.rb | 13 +++++++------ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/app/controllers/assessor_interface/review_verifications_controller.rb b/app/controllers/assessor_interface/review_verifications_controller.rb index fbd40dcb1c..67379ea938 100644 --- a/app/controllers/assessor_interface/review_verifications_controller.rb +++ b/app/controllers/assessor_interface/review_verifications_controller.rb @@ -2,7 +2,7 @@ module AssessorInterface class ReviewVerificationsController < BaseController - include RegionHelper + include RegionHelper before_action :authorize_assessor def index @@ -12,7 +12,11 @@ def index @professional_standing_request = assessment.professional_standing_request if assessment.professional_standing_request.verify_failed? - @teaching_authority_name = region_teaching_authority_name(@assessment.application_form.region, "Contact relevant competent authority") + @teaching_authority_name = + region_teaching_authority_name( + @assessment.application_form.region, + "Contact relevant competent authority", + ) render layout: "full_from_desktop" end diff --git a/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb b/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb index 9543f0947d..5fb4437b77 100644 --- a/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb +++ b/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb @@ -11,6 +11,8 @@ class ReviewVerifications < SitePrism::Page element :tag, ".govuk-tag" end + section :task_list, TaskList, ".app-task-list" + element :back_to_overview_button, ".govuk-button" end end diff --git a/spec/support/autoload/page_objects/task_list.rb b/spec/support/autoload/page_objects/task_list.rb index 1a342923ca..15d7624492 100644 --- a/spec/support/autoload/page_objects/task_list.rb +++ b/spec/support/autoload/page_objects/task_list.rb @@ -1,5 +1,8 @@ module PageObjects class TaskList < SitePrism::Section + element :not_started_status, ".govuk-tag.govuk-tag--grey" + element :accepted_status, ".govuk-tag.govuk-tag--green" + sections :sections, TaskListSection, ".app-task-list > li" def find_item(text) diff --git a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb index 4c3c73dadf..2104cd7059 100644 --- a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb +++ b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb @@ -94,7 +94,9 @@ def when_i_click_on_assessment_decision end def when_i_click_on_lops - find(".govuk-link").click + assessor_application_page.task_list.click_item( + "Contact relevant competent authority", + ) end def when_i_fill_in_the_review_lops_form @@ -109,14 +111,13 @@ def when_i_click_on_back_to_overview end def and_i_see_the_lops_not_started - status_text = find(".govuk-tag.govuk-tag--grey.app-task-list__tag").text - expect(status_text).to eq("NOT STARTED") + status_tag = assessor_application_page.task_list.not_started_status + expect(status_tag.text).to eq("NOT STARTED") end def and_i_see_the_lops_accepted - status_text = - find("strong.govuk-tag.govuk-tag--green.app-task-list__tag").text - expect(status_text).to eq("ACCEPTED") + status_tag = assessor_application_page.task_list.accepted_status + expect(status_tag.text).to eq("ACCEPTED") end def application_form From a87b81ed2620c4291466570a74309103e5685a2c Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 6 Nov 2023 07:51:04 +0000 Subject: [PATCH 6/9] removed code from controller to use helper method in view --- .../assessor_interface/review_verifications_controller.rb | 6 ------ app/helpers/region_helper.rb | 4 ++-- .../assessor_interface/review_verifications/index.html.erb | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/app/controllers/assessor_interface/review_verifications_controller.rb b/app/controllers/assessor_interface/review_verifications_controller.rb index 67379ea938..de12ce5180 100644 --- a/app/controllers/assessor_interface/review_verifications_controller.rb +++ b/app/controllers/assessor_interface/review_verifications_controller.rb @@ -12,12 +12,6 @@ def index @professional_standing_request = assessment.professional_standing_request if assessment.professional_standing_request.verify_failed? - @teaching_authority_name = - region_teaching_authority_name( - @assessment.application_form.region, - "Contact relevant competent authority", - ) - render layout: "full_from_desktop" end diff --git a/app/helpers/region_helper.rb b/app/helpers/region_helper.rb index b0e0029e5f..6368cef49e 100644 --- a/app/helpers/region_helper.rb +++ b/app/helpers/region_helper.rb @@ -11,8 +11,8 @@ def region_certificate_phrase(region) "#{certificate.indefinite_article} #{tag.span(certificate, lang: region.country.code)}".html_safe end - def region_teaching_authority_name(region, default = nil) - region.teaching_authority_name.presence || default || "teaching authority" + def region_teaching_authority_name(region) + region.teaching_authority_name.presence || "teaching authority" end def region_teaching_authority_name_phrase(region) diff --git a/app/views/assessor_interface/review_verifications/index.html.erb b/app/views/assessor_interface/review_verifications/index.html.erb index 042b6be2d5..0cac294122 100644 --- a/app/views/assessor_interface/review_verifications/index.html.erb +++ b/app/views/assessor_interface/review_verifications/index.html.erb @@ -14,7 +14,7 @@ title: "LoPS", items: [ { - name: @teaching_authority_name, + name: region_teaching_authority_name(@application_form.region), link: [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request], status: @professional_standing_request.review_status, } From d377a8398bfd4d81a636fb658361647a6aa0f7c6 Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 6 Nov 2023 14:52:16 +0000 Subject: [PATCH 7/9] changed tests according to comments --- .../review_verifications/index.html.erb | 2 +- .../reviewing_professional_standing_spec.rb | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/views/assessor_interface/review_verifications/index.html.erb b/app/views/assessor_interface/review_verifications/index.html.erb index 0cac294122..798cdf1859 100644 --- a/app/views/assessor_interface/review_verifications/index.html.erb +++ b/app/views/assessor_interface/review_verifications/index.html.erb @@ -14,7 +14,7 @@ title: "LoPS", items: [ { - name: region_teaching_authority_name(@application_form.region), + name: (region_teaching_authority_name(@application_form.region) == "teaching authority") ? "Contact relevant competent authority" : region_teaching_authority_name(@application_form.region), link: [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request], status: @professional_standing_request.review_status, } diff --git a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb index 2104cd7059..490d2b54cd 100644 --- a/spec/system/assessor_interface/reviewing_professional_standing_spec.rb +++ b/spec/system/assessor_interface/reviewing_professional_standing_spec.rb @@ -111,13 +111,19 @@ def when_i_click_on_back_to_overview end def and_i_see_the_lops_not_started - status_tag = assessor_application_page.task_list.not_started_status - expect(status_tag.text).to eq("NOT STARTED") + item = + assessor_review_verifications_page.task_list.find_item( + "Contact relevant competent authority", + ) + expect(item.status_tag.text).to eq("NOT STARTED") end def and_i_see_the_lops_accepted - status_tag = assessor_application_page.task_list.accepted_status - expect(status_tag.text).to eq("ACCEPTED") + item = + assessor_review_verifications_page.task_list.find_item( + "Contact relevant competent authority", + ) + expect(item.status_tag.text).to eq("ACCEPTED") end def application_form From 3b0cef469c3ba02ea80a23d2cd1e0bda50123057 Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 6 Nov 2023 16:12:08 +0000 Subject: [PATCH 8/9] removed redundent code and added default to helper method --- .../assessor_interface/review_verifications_controller.rb | 1 - app/helpers/region_helper.rb | 4 ++-- .../assessor_interface/review_verifications/index.html.erb | 2 +- spec/support/autoload/page_objects/task_list.rb | 3 --- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/controllers/assessor_interface/review_verifications_controller.rb b/app/controllers/assessor_interface/review_verifications_controller.rb index de12ce5180..786f1c6f56 100644 --- a/app/controllers/assessor_interface/review_verifications_controller.rb +++ b/app/controllers/assessor_interface/review_verifications_controller.rb @@ -2,7 +2,6 @@ module AssessorInterface class ReviewVerificationsController < BaseController - include RegionHelper before_action :authorize_assessor def index diff --git a/app/helpers/region_helper.rb b/app/helpers/region_helper.rb index 6368cef49e..b0e0029e5f 100644 --- a/app/helpers/region_helper.rb +++ b/app/helpers/region_helper.rb @@ -11,8 +11,8 @@ def region_certificate_phrase(region) "#{certificate.indefinite_article} #{tag.span(certificate, lang: region.country.code)}".html_safe end - def region_teaching_authority_name(region) - region.teaching_authority_name.presence || "teaching authority" + def region_teaching_authority_name(region, default = nil) + region.teaching_authority_name.presence || default || "teaching authority" end def region_teaching_authority_name_phrase(region) diff --git a/app/views/assessor_interface/review_verifications/index.html.erb b/app/views/assessor_interface/review_verifications/index.html.erb index 798cdf1859..38002180fb 100644 --- a/app/views/assessor_interface/review_verifications/index.html.erb +++ b/app/views/assessor_interface/review_verifications/index.html.erb @@ -14,7 +14,7 @@ title: "LoPS", items: [ { - name: (region_teaching_authority_name(@application_form.region) == "teaching authority") ? "Contact relevant competent authority" : region_teaching_authority_name(@application_form.region), + name: region_teaching_authority_name(@application_form.region, "Contact relevant competent authority"), link: [:review, :assessor_interface, @application_form, @assessment, :professional_standing_request], status: @professional_standing_request.review_status, } diff --git a/spec/support/autoload/page_objects/task_list.rb b/spec/support/autoload/page_objects/task_list.rb index 15d7624492..1a342923ca 100644 --- a/spec/support/autoload/page_objects/task_list.rb +++ b/spec/support/autoload/page_objects/task_list.rb @@ -1,8 +1,5 @@ module PageObjects class TaskList < SitePrism::Section - element :not_started_status, ".govuk-tag.govuk-tag--grey" - element :accepted_status, ".govuk-tag.govuk-tag--green" - sections :sections, TaskListSection, ".app-task-list > li" def find_item(text) From 6d6adda54786cab8d4dc4e1af03dde44e850f7d3 Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 6 Nov 2023 16:18:23 +0000 Subject: [PATCH 9/9] removed some more redundant code --- .../page_objects/assessor_interface/review_verifications.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb b/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb index 5fb4437b77..18f75cd782 100644 --- a/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb +++ b/spec/support/autoload/page_objects/assessor_interface/review_verifications.rb @@ -6,11 +6,6 @@ class ReviewVerifications < SitePrism::Page set_url "/assessor/applications/{application_form_id}/assessments/{assessment_id}" \ "/review-verifications" - sections :rows, ".govuk-table__row" do - element :link, ".govuk-link" - element :tag, ".govuk-tag" - end - section :task_list, TaskList, ".app-task-list" element :back_to_overview_button, ".govuk-button"