From 8fe1258fadb591fe49fb51d2a7032bc432e28a63 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Wed, 21 May 2025 11:45:09 +0300 Subject: [PATCH 1/4] Change feature report views to work with either live or draft forms --- app/controllers/reports_controller.rb | 6 ++-- app/views/reports/feature_report.html.erb | 6 ++-- app/views/reports/features.html.erb | 22 ++++++------ app/views/reports/index.html.erb | 2 +- .../questions_with_answer_type.html.erb | 6 ++-- config/locales/en.yml | 36 +++++++++---------- .../reports/feature_report.html.erb_spec.rb | 6 +++- spec/views/reports/features.html.erb_spec.rb | 13 ++++++- ...uestions_with_answer_type.html.erb_spec.rb | 4 ++- 9 files changed, 60 insertions(+), 41 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 22a4332e7..49d9d007f 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -8,7 +8,7 @@ def features forms = Reports::FormDocumentsService.live_form_documents data = Reports::FeatureReportService.new(forms).report - render template: "reports/features", locals: { data: } + render template: "reports/features", locals: { tag: "live", data: } end def questions_with_answer_type @@ -16,7 +16,7 @@ def questions_with_answer_type forms = Reports::FormDocumentsService.live_form_documents questions = Reports::FeatureReportService.new(forms).questions_with_answer_type(answer_type) - render template: "reports/questions_with_answer_type", locals: { answer_type:, questions: } + render template: "reports/questions_with_answer_type", locals: { tag: "live", answer_type:, questions: } end def feature_report @@ -30,7 +30,7 @@ def feature_report type: "text/csv; charset=iso-8859-1", disposition: "attachment; filename=#{csv_filename("live_#{report}_report")}" else - render template: "reports/feature_report", locals: { report:, records: } + render template: "reports/feature_report", locals: { tag: "live", report:, records: } end end diff --git a/app/views/reports/feature_report.html.erb b/app/views/reports/feature_report.html.erb index f71186f24..ceddf1390 100644 --- a/app/views/reports/feature_report.html.erb +++ b/app/views/reports/feature_report.html.erb @@ -1,10 +1,10 @@ -<% set_page_title(t("reports.#{report}.heading"))%> +<% set_page_title(t("reports.#{report}.heading", tag:).upcase_first)%> <% content_for :back_link, govuk_back_link_to(report_features_path, t("reports.back_to_feature_usage")) %>
-

<%= t("reports.#{report}.heading")%>

+

<%= t("reports.#{report}.heading", tag:).upcase_first %>

-

<%=govuk_link_to(t("reports.#{report}.download_csv"), url_for(format: :csv))%>

+

<%=govuk_link_to(t("reports.#{report}.download_csv", tag:).upcase_first, url_for(format: :csv))%>

diff --git a/app/views/reports/features.html.erb b/app/views/reports/features.html.erb index ee7368fda..6439a360e 100644 --- a/app/views/reports/features.html.erb +++ b/app/views/reports/features.html.erb @@ -1,29 +1,29 @@ -<% set_page_title(t(".title")) %> +<% set_page_title(t(".title", tag:)) %> <% content_for :back_link, govuk_back_link_to(reports_path, t("reports.back_link")) %>
-

<%= t(".title") %>

+

<%= t(".title", tag:) %>

<%= t(".features.heading") %>

<%= govuk_summary_list do |summary_list| %> <%= summary_list.with_row do |row| %> - <%= row.with_key(text: t(".features.total_live_forms")) %> + <%= row.with_key(text: t(".features.total_forms", tag:).upcase_first) %> <%= row.with_value(text: data[:total_forms]) %> <% end %> <%= summary_list.with_row do |row| %> - <%= row.with_key(text: t(".features.live_forms_with_routes")) %> + <%= row.with_key(text: t(".features.forms_with_routes", tag:).upcase_first) %> <%= row.with_value(text: govuk_link_to(data[:forms_with_routing], feature_report_path(report: "forms-with-routes"), no_visited_state: true)) %> <% end %> <%= summary_list.with_row do |row| %> - <%= row.with_key(text: t(".features.live_forms_with_payments")) %> + <%= row.with_key(text: t(".features.forms_with_payments", tag:).upcase_first) %> <%= row.with_value(text: govuk_link_to(data[:forms_with_payment], feature_report_path(report: "forms-with-payments"), no_visited_state: true)) %> <% end %> <%= summary_list.with_row do |row| %> - <%= row.with_key(text: t(".features.live_forms_with_add_another_answer")) %> + <%= row.with_key(text: t(".features.forms_with_add_another_answer", tag:).upcase_first) %> <%= row.with_value(text: govuk_link_to(data[:forms_with_add_another_answer], feature_report_path(report: "questions-with-add-another-answer"), no_visited_state: true)) %> <% end %> <%= summary_list.with_row do |row| %> - <%= row.with_key(text: t(".features.live_forms_with_csv_submission_enabled")) %> + <%= row.with_key(text: t(".features.forms_with_csv_submission_enabled", tag:).upcase_first) %> <%= row.with_value(text: govuk_link_to(data[:forms_with_csv_submission_enabled], feature_report_path(report: "forms-with-csv-submission-enabled"), no_visited_state: true)) %> <% end %> <% end %> @@ -35,12 +35,14 @@ <%= table.with_head do |head| %> <%= head.with_row do |row| %> <%= row.with_cell(text: t(".answer_types.table_headings.answer_type")) %> - <%= row.with_cell(text: t(".answer_types.table_headings.number_of_forms"), numeric: true) %> - <%= row.with_cell(text: t(".answer_types.table_headings.number_of_pages"), numeric: true) %> + <%= row.with_cell(text: t(".answer_types.table_headings.number_of_forms", tag:), numeric: true) %> + <%= row.with_cell(text: t(".answer_types.table_headings.number_of_pages", tag:), numeric: true) %> <% end %> <% end %> - <% answer_type_links = { "selection" => report_selection_questions_summary_path } %> + <% answer_type_links = { + "selection" => tag == "live" ? report_selection_questions_summary_path : nil, + } %> <%= table.with_body do |body| %> <% Page::ANSWER_TYPES.each do |answer_type| %> <%= body.with_row do |row| %> diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb index 4a6cadd08..1634e731a 100644 --- a/app/views/reports/index.html.erb +++ b/app/views/reports/index.html.erb @@ -4,7 +4,7 @@

<%= t(".title") %>

  • <%= govuk_link_to t("reports.add_another_answer.title"), report_add_another_answer_path %>
    • -
  • <%= govuk_link_to t("reports.features.title"), report_features_path %>
    • +
  • <%= govuk_link_to t("reports.features.title", tag: "live"), report_features_path %>
  • <%= govuk_link_to t("reports.features.long_list_usage.title"), report_selection_questions_summary_path %>
  • <%= govuk_link_to t("reports.users.title"), report_users_path %>
  • <%= govuk_link_to t("reports.last_signed_in_at.title"), report_last_signed_in_at_path %>
    • diff --git a/app/views/reports/questions_with_answer_type.html.erb b/app/views/reports/questions_with_answer_type.html.erb index caf3f82ce..306f64b38 100644 --- a/app/views/reports/questions_with_answer_type.html.erb +++ b/app/views/reports/questions_with_answer_type.html.erb @@ -1,10 +1,10 @@ -<% set_page_title(t(".heading", answer_type: t("helpers.label.page.answer_type_options.names.#{answer_type}").downcase)) %> +<% set_page_title(t(".heading", answer_type: t("helpers.label.page.answer_type_options.names.#{answer_type}").downcase, tag:).upcase_first) %> <% content_for :back_link, govuk_back_link_to(report_features_path, t("reports.back_to_feature_usage")) %>
    -

    <%= t(".heading", answer_type: t("helpers.label.page.answer_type_options.names.#{answer_type}").downcase) %>

    +

    <%= t(".heading", answer_type: t("helpers.label.page.answer_type_options.names.#{answer_type}").downcase, tag:).upcase_first %>

    -

    <%=govuk_link_to(t(".download_csv"), report_live_questions_csv_path(answer_type:))%>

    +

    <%=govuk_link_to(t(".download_csv", tag:).upcase_first, report_live_questions_csv_path(answer_type:))%>

    diff --git a/config/locales/en.yml b/config/locales/en.yml index 61178485a..d4a9dcb7b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1377,18 +1377,18 @@ en: heading: Answer type usage table_headings: answer_type: Answer type - number_of_forms: Number of live forms with this answer type - number_of_pages: Number of uses of this answer type in live forms + number_of_forms: Number of %{tag} forms with this answer type + number_of_pages: Number of uses of this answer type in %{tag} forms features: + forms_with_add_another_answer: "%{tag} forms with add another answer" + forms_with_csv_submission_enabled: "%{tag} forms with CSV submission enabled" + forms_with_payments: "%{tag} forms with payments" + forms_with_routes: "%{tag} forms with routes" heading: Feature usage - live_forms_with_add_another_answer: Live forms with add another answer - live_forms_with_csv_submission_enabled: Live forms with CSV submission enabled - live_forms_with_payments: Live forms with payments - live_forms_with_routes: Live forms with routes - total_live_forms: Total live forms + total_forms: Total %{tag} forms long_list_usage: title: Selection from a list of options feature usage in live forms - title: Feature and answer type usage in live forms + title: Feature and answer type usage in %{tag} forms form_or_questions_list_table: headings: form_name: Form name @@ -1396,14 +1396,14 @@ en: organisation: Organisation question_text: Question text forms_with_csv_submission_enabled: - download_csv: Download data about all live forms with CSV submission enabled as a CSV file - heading: Live forms with CSV submission enabled + download_csv: Download data about all %{tag} forms with CSV submission enabled as a CSV file + heading: "%{tag} forms with CSV submission enabled" forms_with_payments: - download_csv: Download data about all live forms with payments as a CSV file - heading: Live forms with payments + download_csv: Download data about all %{tag} forms with payments as a CSV file + heading: "%{tag} forms with payments" forms_with_routes: - download_csv: Download data about all live forms with routes as a CSV file - heading: Live forms with routes + download_csv: Download data about all %{tag} forms with routes as a CSV file + heading: "%{tag} forms with routes" index: title: Reports last_signed_in_at: @@ -1413,11 +1413,11 @@ en: not_since_last_signed_in_at_added: People who last signed in sometime between 3 November 2023 and 4 November 2024 title: When users last signed in questions_with_add_another_answer: - download_csv: Download all questions with add another answer in live forms as a CSV file - heading: Questions with add another answer in live forms + download_csv: Download all questions with add another answer in %{tag} forms as a CSV file + heading: Questions with add another answer in %{tag} forms questions_with_answer_type: - download_csv: Download all questions in live forms with this answer type as a CSV file - heading: Live questions with %{answer_type} answer type + download_csv: Download all questions in %{tag} forms with this answer type as a CSV file + heading: "%{tag} questions with %{answer_type} answer type" selection_questions: autocomplete: title: Questions where you can select one from over 30 options diff --git a/spec/views/reports/feature_report.html.erb_spec.rb b/spec/views/reports/feature_report.html.erb_spec.rb index 095b9b3f4..c6fb65837 100644 --- a/spec/views/reports/feature_report.html.erb_spec.rb +++ b/spec/views/reports/feature_report.html.erb_spec.rb @@ -7,7 +7,7 @@ before do controller.request.path_parameters[:report] = report.dasherize - render locals: { report:, records: } + render locals: { tag:, report:, records: } end context "with forms_with_csv_submission_enabled report" do @@ -22,6 +22,7 @@ describe "page title" do it "matches the heading" do expect(view.content_for(:title)).to eq "Live forms with CSV submission enabled" + expect(rendered).to have_css "h1", text: view.content_for(:title) end end @@ -68,6 +69,7 @@ describe "page title" do it "matches the heading" do expect(view.content_for(:title)).to eq "Live forms with payments" + expect(rendered).to have_css "h1", text: view.content_for(:title) end end @@ -114,6 +116,7 @@ describe "page title" do it "matches the heading" do expect(view.content_for(:title)).to eq "Live forms with routes" + expect(rendered).to have_css "h1", text: view.content_for(:title) end end @@ -163,6 +166,7 @@ describe "page title" do it "matches the heading" do expect(view.content_for(:title)).to eq "Questions with add another answer in live forms" + expect(rendered).to have_css "h1", text: view.content_for(:title) end end diff --git a/spec/views/reports/features.html.erb_spec.rb b/spec/views/reports/features.html.erb_spec.rb index e1976b111..f133f6839 100644 --- a/spec/views/reports/features.html.erb_spec.rb +++ b/spec/views/reports/features.html.erb_spec.rb @@ -34,9 +34,10 @@ forms_with_csv_submission_enabled: 2, } end + let(:tag) { "live" } before do - render template: "reports/features", locals: { data: report } + render template: "reports/features", locals: { tag:, data: report } end describe "page title" do @@ -57,6 +58,16 @@ expect(rendered).to have_css(".govuk-summary-list__row", text: "Total live forms#{report[:total_forms]}") end + it "has a table of answer type usage" do + expect(rendered).to have_table "Answer type usage" do |table| + expect(table.find_all("thead th").map(&:text)).to eq [ + "Answer type", + "Number of live forms with this answer type", + "Number of uses of this answer type in live forms", + ] + end + end + Page::ANSWER_TYPES.map(&:to_sym).each do |answer_type| it "contains a heading for #{answer_type}" do expect(rendered).to have_css("th", text: I18n.t("helpers.label.page.answer_type_options.names.#{answer_type}")) diff --git a/spec/views/reports/questions_with_answer_type.html.erb_spec.rb b/spec/views/reports/questions_with_answer_type.html.erb_spec.rb index 1c72a4add..c961631d4 100644 --- a/spec/views/reports/questions_with_answer_type.html.erb_spec.rb +++ b/spec/views/reports/questions_with_answer_type.html.erb_spec.rb @@ -7,14 +7,16 @@ { "data" => { "question_text" => "What’s your email address?" }, "form" => { "form_id" => 3, "content" => { "name" => "Branch route form" }, "group" => { "organisation" => { "name" => "Government Digital Service" } } } }, ] end + let(:tag) { "live" } before do - render locals: { answer_type: "email", questions: } + render locals: { tag:, answer_type: "email", questions: } end describe "page title" do it "matches the heading" do expect(view.content_for(:title)).to eq "Live questions with email address answer type" + expect(rendered).to have_css "h1", text: view.content_for(:title) end end From a46eddc50b5af327ca0997b72a6397c3c1a5fed7 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 20 May 2025 15:13:52 +0300 Subject: [PATCH 2/4] Add FormDocumentsService#draft_form_documents --- .../reports/form_documents_service.rb | 14 +++- .../reports/form_documents_service_spec.rb | 79 +++++++++++++++---- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/app/services/reports/form_documents_service.rb b/app/services/reports/form_documents_service.rb index 557c47531..b99f67447 100644 --- a/app/services/reports/form_documents_service.rb +++ b/app/services/reports/form_documents_service.rb @@ -8,11 +8,19 @@ class << self FormDocumentsResponse = Data.define(:forms, :has_more_results?) + def draft_form_documents + form_documents(tag: "draft") + end + def live_form_documents + form_documents(tag: "live") + end + + def form_documents(tag:) Enumerator.new do |yielder| page = 1 loop do - form_documents_response = live_form_documents_page(page) + form_documents_response = form_documents_page(tag:, page:) form_documents_response.forms.each { |f| yielder << f unless is_in_internal_organisation?(f) } break unless form_documents_response.has_more_results? @@ -36,9 +44,9 @@ def has_csv_submission_enabled?(form_document) private - def live_form_documents_page(page) + def form_documents_page(tag:, page:) uri = URI(FORM_DOCUMENTS_URL) - params = { tag: "live", page:, per_page: Settings.reports.forms_api_forms_per_request_page } + params = { tag:, page:, per_page: Settings.reports.forms_api_forms_per_request_page } uri.query = URI.encode_www_form(params) response = Net::HTTP.get_response(uri, REQUEST_HEADERS) diff --git a/spec/services/reports/form_documents_service_spec.rb b/spec/services/reports/form_documents_service_spec.rb index c4639a0fd..d64ac5227 100644 --- a/spec/services/reports/form_documents_service_spec.rb +++ b/spec/services/reports/form_documents_service_spec.rb @@ -1,39 +1,84 @@ require "rails_helper" RSpec.describe Reports::FormDocumentsService do - describe ".live_form_documents" do + describe ".form_documents" do let(:form_documents_url) { "#{Settings.forms_api.base_url}/api/v2/form-documents".freeze } # This response JSON was generated by making a real API request to forms-api with the data from the database seeds. # Once we have transitioned to using the V2 API in forms-admin, it might make more sense to use factories to generate # the response. let(:form_documents_response_json) { file_fixture("form_documents_response.json").read } + let(:tag) { "live" } before do allow(Settings.reports).to receive(:forms_api_forms_per_request_page).and_return 4 stub_request(:get, form_documents_url) - .with(query: { page: "1", per_page: "4", tag: "live" }) + .with(query: { page: "1", per_page: "4", tag: }) .to_return(body: form_documents_response_json, headers: response_headers(12, 0, 4)) stub_request(:get, form_documents_url) - .with(query: { page: "2", per_page: "4", tag: "live" }) + .with(query: { page: "2", per_page: "4", tag: }) .to_return(body: form_documents_response_json, headers: response_headers(12, 4, 4)) stub_request(:get, form_documents_url) - .with(query: { page: "3", per_page: "4", tag: "live" }) + .with(query: { page: "3", per_page: "4", tag: }) .to_return(body: form_documents_response_json, headers: response_headers(12, 8, 4)) end - it "makes request to forms-api for each page of results" do - form_documents = described_class.live_form_documents.to_a - expect(form_documents.size).to eq(12) - assert_requested(:get, form_documents_url, query: { page: "1", per_page: "4", tag: "live" }, headers:, times: 1) - assert_requested(:get, form_documents_url, query: { page: "2", per_page: "4", tag: "live" }, headers:, times: 1) - assert_requested(:get, form_documents_url, query: { page: "3", per_page: "4", tag: "live" }, headers:, times: 1) + context "with draft tag" do + let(:tag) { "draft" } + + it "makes request to forms-api for each page of results" do + form_documents = described_class.form_documents(tag:).to_a + expect(form_documents.size).to eq(12) + assert_requested(:get, form_documents_url, query: { page: "1", per_page: "4", tag: "draft" }, headers:, times: 1) + assert_requested(:get, form_documents_url, query: { page: "2", per_page: "4", tag: "draft" }, headers:, times: 1) + assert_requested(:get, form_documents_url, query: { page: "3", per_page: "4", tag: "draft" }, headers:, times: 1) + end + + it "returns form documents" do + form_document = described_class.form_documents(tag:).first + expect(form_document).to be_a(Hash) + expect(form_document).to have_key("form_id") + end end - it "returns form documents" do - form_document = described_class.live_form_documents.first - expect(form_document).to be_a(Hash) - expect(form_document).to have_key("form_id") + context "with live tag" do + let(:tag) { :live } + + it "makes request to forms-api for each page of results" do + form_documents = described_class.form_documents(tag:).to_a + expect(form_documents.size).to eq(12) + assert_requested(:get, form_documents_url, query: { page: "1", per_page: "4", tag: "live" }, headers:, times: 1) + assert_requested(:get, form_documents_url, query: { page: "2", per_page: "4", tag: "live" }, headers:, times: 1) + assert_requested(:get, form_documents_url, query: { page: "3", per_page: "4", tag: "live" }, headers:, times: 1) + end + + it "returns form documents" do + form_document = described_class.form_documents(tag:).first + expect(form_document).to be_a(Hash) + expect(form_document).to have_key("form_id") + end + + context "when there are forms from internal organisations" do + let(:organisation) { build :organisation, id: 1, internal: false } + let(:internal_organisation) { build :organisation, id: 1, internal: true } + let(:group) { build :group, id: 1, organisation: } + let(:internal_group) { build :group, id: 1, organisation: internal_organisation } + let(:group_form) { GroupForm.new(group:) } + let(:internal_group_form) { GroupForm.new(group: internal_group) } + + before do + allow(GroupForm).to receive(:find_by_form_id).and_return(group_form) + allow(GroupForm).to receive(:find_by_form_id).with(3).and_return(internal_group_form) + end + + it "does not include these forms in the live_form_documents output" do + form_documents = described_class.live_form_documents.to_a + expect(form_documents.size).to eq(9) + + form_documents_with_internal_id = form_documents.filter { it["id"] == 3 } + expect(form_documents_with_internal_id).to be_empty + end + end end context "when forms-api responds with a non-success status code" do @@ -44,7 +89,7 @@ end it "raises a StandardError" do - expect { described_class.live_form_documents.first }.to raise_error( + expect { described_class.form_documents(tag: "live").first }.to raise_error( StandardError, "Forms API responded with a non-success HTTP code when retrieving form documents: status 400" ) end @@ -63,8 +108,8 @@ allow(GroupForm).to receive(:find_by_form_id).with(3).and_return(internal_group_form) end - it "does not include these forms in the live_form_documents output" do - form_documents = described_class.live_form_documents.to_a + it "does not include these forms in the draft_form_documents output" do + form_documents = described_class.form_documents(tag: "live").to_a expect(form_documents.size).to eq(9) form_documents_with_internal_id = form_documents.filter { it["id"] == 3 } From 289b6769bf1a6ee8f54e069db004640409ad09a0 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Wed, 21 May 2025 09:49:50 +0300 Subject: [PATCH 3/4] Change feature report routes to have parameter for live or draft forms --- app/controllers/reports_controller.rb | 17 ++-- app/views/reports/index.html.erb | 2 +- .../selection_questions/summary.html.erb | 2 +- config/routes.rb | 2 +- spec/requests/reports_controller_spec.rb | 44 +++++----- spec/routing/reports_routing_spec.rb | 88 +++++++++++-------- .../reports/feature_report.html.erb_spec.rb | 2 + spec/views/reports/features.html.erb_spec.rb | 16 ++++ spec/views/reports/index.html.erb_spec.rb | 2 +- ...uestions_with_answer_type.html.erb_spec.rb | 2 + .../summary.html.erb_spec.rb | 2 +- 11 files changed, 109 insertions(+), 70 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 49d9d007f..d2dd55b6b 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -5,32 +5,35 @@ class ReportsController < ApplicationController def index; end def features - forms = Reports::FormDocumentsService.live_form_documents + tag = params[:tag] + forms = Reports::FormDocumentsService.form_documents(tag:) data = Reports::FeatureReportService.new(forms).report - render template: "reports/features", locals: { tag: "live", data: } + render template: "reports/features", locals: { tag:, data: } end def questions_with_answer_type + tag = params[:tag] answer_type = params.require(:answer_type) - forms = Reports::FormDocumentsService.live_form_documents + forms = Reports::FormDocumentsService.form_documents(tag:) questions = Reports::FeatureReportService.new(forms).questions_with_answer_type(answer_type) - render template: "reports/questions_with_answer_type", locals: { tag: "live", answer_type:, questions: } + render template: "reports/questions_with_answer_type", locals: { tag:, answer_type:, questions: } end def feature_report report = params[:report].underscore + tag = params[:tag] - forms = Reports::FormDocumentsService.live_form_documents + forms = Reports::FormDocumentsService.form_documents(tag:) records = Reports::FeatureReportService.new(forms).report report if params[:format] == "csv" send_data Reports::CsvReportService.new(records).csv, type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{csv_filename("live_#{report}_report")}" + disposition: "attachment; filename=#{csv_filename("#{tag}_#{report}_report")}" else - render template: "reports/feature_report", locals: { tag: "live", report:, records: } + render template: "reports/feature_report", locals: { tag:, report:, records: } end end diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb index 1634e731a..12c94ad46 100644 --- a/app/views/reports/index.html.erb +++ b/app/views/reports/index.html.erb @@ -4,7 +4,7 @@

    <%= t(".title") %>

    • <%= govuk_link_to t("reports.add_another_answer.title"), report_add_another_answer_path %>
      • -
    • <%= govuk_link_to t("reports.features.title", tag: "live"), report_features_path %>
      • +
    • <%= govuk_link_to t("reports.features.title", tag: "live"), report_features_path(tag: :live) %>
    • <%= govuk_link_to t("reports.features.long_list_usage.title"), report_selection_questions_summary_path %>
    • <%= govuk_link_to t("reports.users.title"), report_users_path %>
    • <%= govuk_link_to t("reports.last_signed_in_at.title"), report_last_signed_in_at_path %>
      • diff --git a/app/views/reports/selection_questions/summary.html.erb b/app/views/reports/selection_questions/summary.html.erb index 93f287c6e..1343c0f93 100644 --- a/app/views/reports/selection_questions/summary.html.erb +++ b/app/views/reports/selection_questions/summary.html.erb @@ -1,5 +1,5 @@ <% set_page_title(t(".title")) %> -<% content_for :back_link, govuk_back_link_to(report_features_path, t(".back_link")) %> +<% content_for :back_link, govuk_back_link_to(report_features_path(tag: :live), t(".back_link")) %>

      <%= t(".title") %>

      diff --git a/config/routes.rb b/config/routes.rb index 554cf79c7..5cdb36664 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -195,7 +195,7 @@ scope "/reports" do get "/", to: "reports#index", as: :reports - scope "/features" do + scope "/features/:tag", constraints: { tag: /(draft|live)/ } do get "/", to: "reports#features", as: :report_features get "/questions-with-answer-type/:answer_type", to: "reports#questions_with_answer_type", as: :report_questions_with_answer_type get "/:report", constraints: Reports::FeatureReportService::Constraint, to: "reports#feature_report", as: :feature_report diff --git a/spec/requests/reports_controller_spec.rb b/spec/requests/reports_controller_spec.rb index 7fcdce372..71df3c13b 100644 --- a/spec/requests/reports_controller_spec.rb +++ b/spec/requests/reports_controller_spec.rb @@ -77,7 +77,7 @@ before do login_as_standard_user - get report_features_path + get report_features_path(tag: :live) end it "returns http code 403" do @@ -93,7 +93,7 @@ before do login_as_organisation_admin_user - get report_features_path + get report_features_path(tag: :live) end it "returns http code 403" do @@ -109,7 +109,7 @@ before do login_as_super_admin_user - get report_features_path + get report_features_path(tag: :live) end it "returns http code 200" do @@ -135,7 +135,7 @@ before do login_as_standard_user - get report_questions_with_answer_type_path(answer_type: "email") + get report_questions_with_answer_type_path(tag: :live, answer_type: "email") end it "returns http code 403" do @@ -151,7 +151,7 @@ before do login_as_organisation_admin_user - get report_questions_with_answer_type_path(answer_type: "email") + get report_questions_with_answer_type_path(tag: :live, answer_type: "email") end it "returns http code 403" do @@ -167,7 +167,7 @@ before do login_as_super_admin_user - get report_questions_with_answer_type_path(answer_type: "email") + get report_questions_with_answer_type_path(tag: :live, answer_type: "email") end it "returns http code 200" do @@ -193,7 +193,7 @@ before do login_as_standard_user - get feature_report_path(report: "questions-with-add-another-answer") + get feature_report_path(tag: :live, report: "questions-with-add-another-answer") end it "returns http code 403" do @@ -209,7 +209,7 @@ before do login_as_organisation_admin_user - get feature_report_path(report: "questions-with-add-another-answer") + get feature_report_path(tag: :live, report: "questions-with-add-another-answer") end it "returns http code 403" do @@ -225,7 +225,7 @@ before do login_as_super_admin_user - get feature_report_path(report: "questions-with-add-another-answer") + get feature_report_path(tag: :live, report: "questions-with-add-another-answer") end it "returns http code 200" do @@ -251,7 +251,7 @@ before do login_as_standard_user - get feature_report_path(report: "forms-with-routes") + get feature_report_path(tag: :live, report: "forms-with-routes") end it "returns http code 403" do @@ -267,7 +267,7 @@ before do login_as_organisation_admin_user - get feature_report_path(report: "forms-with-routes") + get feature_report_path(tag: :live, report: "forms-with-routes") end it "returns http code 403" do @@ -283,7 +283,7 @@ before do login_as_super_admin_user - get feature_report_path(report: "forms-with-routes") + get feature_report_path(tag: :live, report: "forms-with-routes") end it "returns http code 200" do @@ -309,7 +309,7 @@ before do login_as_standard_user - get feature_report_path(report: "forms-with-payments") + get feature_report_path(tag: :live, report: "forms-with-payments") end it "returns http code 403" do @@ -325,7 +325,7 @@ before do login_as_organisation_admin_user - get feature_report_path(report: "forms-with-payments") + get feature_report_path(tag: :live, report: "forms-with-payments") end it "returns http code 403" do @@ -341,7 +341,7 @@ before do login_as_super_admin_user - get feature_report_path(report: "forms-with-payments") + get feature_report_path(tag: :live, report: "forms-with-payments") end it "returns http code 200" do @@ -367,7 +367,7 @@ before do login_as_standard_user - get feature_report_path(report: "forms-with-csv-submission-enabled") + get feature_report_path(tag: :live, report: "forms-with-csv-submission-enabled") end it "returns http code 403" do @@ -383,7 +383,7 @@ before do login_as_organisation_admin_user - get feature_report_path(report: "forms-with-csv-submission-enabled") + get feature_report_path(tag: :live, report: "forms-with-csv-submission-enabled") end it "returns http code 403" do @@ -399,7 +399,7 @@ before do login_as_super_admin_user - get feature_report_path(report: "forms-with-csv-submission-enabled") + get feature_report_path(tag: :live, report: "forms-with-csv-submission-enabled") end it "returns http code 200" do @@ -736,7 +736,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get feature_report_path(report: "forms-with-routes", format: :csv) + get feature_report_path(tag: :live, report: "forms-with-routes", format: :csv) end it_behaves_like "csv response" @@ -762,7 +762,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get feature_report_path(report: "forms-with-payments", format: :csv) + get feature_report_path(tag: :live, report: "forms-with-payments", format: :csv) end it_behaves_like "csv response" @@ -787,7 +787,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get feature_report_path(report: "forms-with-csv-submission-enabled", format: :csv) + get feature_report_path(tag: :live, report: "forms-with-csv-submission-enabled", format: :csv) end it_behaves_like "csv response" @@ -834,7 +834,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get feature_report_path(report: "questions-with-add-another-answer", format: :csv) + get feature_report_path(tag: :live, report: "questions-with-add-another-answer", format: :csv) end it_behaves_like "csv response" diff --git a/spec/routing/reports_routing_spec.rb b/spec/routing/reports_routing_spec.rb index 77ab3808d..be7b06a77 100644 --- a/spec/routing/reports_routing_spec.rb +++ b/spec/routing/reports_routing_spec.rb @@ -3,66 +3,82 @@ RSpec.describe ReportsController, type: :routing do describe "feature reports" do describe "routing" do - it "routes to#features" do - expect(get: "/reports/features").to route_to("reports#features") + %w[draft live].each do |tag| + context "with #{tag} tag" do + it "routes to#features for #{tag} forms" do + expect(get: "/reports/features/#{tag}").to route_to("reports#features", tag:) + end + end end - it "routes to questions_with_answer_type" do - expect(get: "/reports/features/questions-with-answer-type/text").to route_to("reports#questions_with_answer_type", answer_type: "text") + it "routes to questions_with_answer_type for draft forms" do + expect(get: "/reports/features/draft/questions-with-answer-type/text").to route_to("reports#questions_with_answer_type", answer_type: "text", tag: "draft") end - it "routes to #feature_report with questions_with_add_another_answer" do - expect(get: "/reports/features/questions-with-add-another-answer").to route_to("reports#feature_report", report: "questions-with-add-another-answer") + it "routes to questions_with_answer_type for live forms" do + expect(get: "/reports/features/live/questions-with-answer-type/text").to route_to("reports#questions_with_answer_type", answer_type: "text", tag: "live") end - it "routes to #feature_report with questions_with_add_another_answer and csv format" do - expect(get: "/reports/features/questions-with-add-another-answer.csv").to route_to("reports#feature_report", report: "questions-with-add-another-answer", format: "csv") + it "does not route to questions_with_answer_type for invalid tag" do + expect(get: "/reports/features/foo/questions-with-answer-type/text").not_to route_to("reports#questions_with_answer_type", answer_type: "text", tag: "foo") end - it "routes to #feature_report with forms_with_routes" do - expect(get: "/reports/features/forms-with-routes").to route_to("reports#feature_report", report: "forms-with-routes") - end + %w[draft live].each do |tag| + context "with #{tag} tag" do + it "routes to #feature_report with questions_with_add_another_answer for #{tag} forms" do + expect(get: "/reports/features/#{tag}/questions-with-add-another-answer").to route_to("reports#feature_report", report: "questions-with-add-another-answer", tag:) + end - it "routes to #feature_report with forms_with_routes and csv format" do - expect(get: "/reports/features/forms-with-routes.csv").to route_to("reports#feature_report", report: "forms-with-routes", format: "csv") - end + it "routes to #feature_report with questions_with_add_another_answer for #{tag} and csv format" do + expect(get: "/reports/features/#{tag}/questions-with-add-another-answer.csv").to route_to("reports#feature_report", report: "questions-with-add-another-answer", tag:, format: "csv") + end - it "routes to #feature_report with forms_with_payments" do - expect(get: "/reports/features/forms-with-payments").to route_to("reports#feature_report", report: "forms-with-payments") - end + it "routes to #feature_report with forms_with_routes for #{tag} forms" do + expect(get: "/reports/features/#{tag}/forms-with-routes").to route_to("reports#feature_report", report: "forms-with-routes", tag:) + end - it "routes to #feature_report with forms_with_payments and csv format" do - expect(get: "/reports/features/forms-with-payments.csv").to route_to("reports#feature_report", report: "forms-with-payments", format: "csv") - end + it "routes to #feature_report with forms_with_routes for #{tag} forms and csv format" do + expect(get: "/reports/features/#{tag}/forms-with-routes.csv").to route_to("reports#feature_report", report: "forms-with-routes", tag:, format: "csv") + end - it "routes to #feature_report with forms_with_csv_submission_enabled" do - expect(get: "/reports/features/forms-with-csv-submission-enabled").to route_to("reports#feature_report", report: "forms-with-csv-submission-enabled") - end + it "routes to #feature_report with forms_with_payments for #{tag} forms" do + expect(get: "/reports/features/#{tag}/forms-with-payments").to route_to("reports#feature_report", report: "forms-with-payments", tag:) + end - it "routes to #feature_report with forms_with_csv_submission_enabled and csv format" do - expect(get: "/reports/features/forms-with-csv-submission-enabled.csv").to route_to("reports#feature_report", report: "forms-with-csv-submission-enabled", format: "csv") - end + it "routes to #feature_report with forms_with_payments for #{tag} forms and csv format" do + expect(get: "/reports/features/#{tag}/forms-with-payments.csv").to route_to("reports#feature_report", report: "forms-with-payments", tag:, format: "csv") + end + + it "routes to #feature_report with forms_with_csv_submission_enabled for #{tag} forms" do + expect(get: "/reports/features/#{tag}/forms-with-csv-submission-enabled").to route_to("reports#feature_report", report: "forms-with-csv-submission-enabled", tag:) + end + + it "routes to #feature_report with forms_with_csv_submission_enabled for #{tag} forms and csv format" do + expect(get: "/reports/features/#{tag}/forms-with-csv-submission-enabled.csv").to route_to("reports#feature_report", report: "forms-with-csv-submission-enabled", tag:, format: "csv") + end + end - it "does not route to #feature_report if param does not match defined report" do - expect(get: "/reports/features/foobar").not_to route_to("reports#feature_report", report: "foobar") + it "does not route to #feature_report if param does not match defined report" do + expect(get: "/reports/features/foobar").not_to route_to("reports#feature_report", report: "foobar") + end end end describe "path helpers" do - it "routes to #questions_with_add_another_answer as csv" do - expect(get: feature_report_path(report: "questions-with-add-another-answer", format: :csv)).to route_to("reports#feature_report", report: "questions-with-add-another-answer", format: "csv") + it "routes to #questions_with_add_another_answer for live forms as csv" do + expect(get: feature_report_path(report: "questions-with-add-another-answer", tag: :live, format: :csv)).to route_to("reports#feature_report", report: "questions-with-add-another-answer", tag: "live", format: "csv") end - it "routes to #forms_with_routes as csv" do - expect(get: feature_report_path(report: "forms-with-routes", format: :csv)).to route_to("reports#feature_report", report: "forms-with-routes", format: "csv") + it "routes to #forms_with_routes for live forms as csv" do + expect(get: feature_report_path(report: "forms-with-routes", tag: :live, format: :csv)).to route_to("reports#feature_report", report: "forms-with-routes", tag: "live", format: "csv") end - it "routes to #forms_with_payments as csv" do - expect(get: feature_report_path(report: "forms-with-payments", format: :csv)).to route_to("reports#feature_report", report: "forms-with-payments", format: "csv") + it "routes to #forms_with_payments for live forms as csv" do + expect(get: feature_report_path(report: "forms-with-payments", tag: :live, format: :csv)).to route_to("reports#feature_report", report: "forms-with-payments", tag: "live", format: "csv") end - it "routes to #forms_with_csv_submission_enabled as csv" do - expect(get: feature_report_path(report: "forms-with-csv-submission-enabled", format: :csv)).to route_to("reports#feature_report", report: "forms-with-csv-submission-enabled", format: "csv") + it "routes to #forms_with_csv_submission_enabled for live forms as csv" do + expect(get: feature_report_path(report: "forms-with-csv-submission-enabled", tag: :live, format: :csv)).to route_to("reports#feature_report", report: "forms-with-csv-submission-enabled", tag: "live", format: "csv") end end end diff --git a/spec/views/reports/feature_report.html.erb_spec.rb b/spec/views/reports/feature_report.html.erb_spec.rb index c6fb65837..4afc71453 100644 --- a/spec/views/reports/feature_report.html.erb_spec.rb +++ b/spec/views/reports/feature_report.html.erb_spec.rb @@ -3,9 +3,11 @@ describe "reports/feature_report" do let(:report) {} let(:records) { [] } + let(:tag) { "live" } before do controller.request.path_parameters[:report] = report.dasherize + controller.request.path_parameters[:tag] = tag render locals: { tag:, report:, records: } end diff --git a/spec/views/reports/features.html.erb_spec.rb b/spec/views/reports/features.html.erb_spec.rb index f133f6839..6b426e469 100644 --- a/spec/views/reports/features.html.erb_spec.rb +++ b/spec/views/reports/features.html.erb_spec.rb @@ -37,6 +37,8 @@ let(:tag) { "live" } before do + controller.request.path_parameters[:tag] = tag + render template: "reports/features", locals: { tag:, data: report } end @@ -119,4 +121,18 @@ it "includes the number of live forms with CSV submission enabled" do expect(rendered).to have_css(".govuk-summary-list__row", text: "Live forms with CSV submission enabled#{report[:forms_with_csv_submission_enabled]}") end + + context "with live tag" do + it "has a link to the selection questions summary report" do + expect(rendered).to have_link href: report_selection_questions_summary_path + end + end + + context "with draft tag" do + let(:tag) { "draft" } + + it "does not have a link to the selection questions summary report" do + expect(rendered).not_to have_link href: report_selection_questions_summary_path + end + end end diff --git a/spec/views/reports/index.html.erb_spec.rb b/spec/views/reports/index.html.erb_spec.rb index 0d8edd0f4..52f94be33 100644 --- a/spec/views/reports/index.html.erb_spec.rb +++ b/spec/views/reports/index.html.erb_spec.rb @@ -16,6 +16,6 @@ end it "includes a link to the features report" do - expect(rendered).to have_link("Feature and answer type usage in live forms", href: report_features_path) + expect(rendered).to have_link("Feature and answer type usage in live forms", href: report_features_path(tag: :live)) end end diff --git a/spec/views/reports/questions_with_answer_type.html.erb_spec.rb b/spec/views/reports/questions_with_answer_type.html.erb_spec.rb index c961631d4..def3ea1f3 100644 --- a/spec/views/reports/questions_with_answer_type.html.erb_spec.rb +++ b/spec/views/reports/questions_with_answer_type.html.erb_spec.rb @@ -10,6 +10,8 @@ let(:tag) { "live" } before do + controller.request.path_parameters[:tag] = tag + render locals: { tag:, answer_type: "email", questions: } end diff --git a/spec/views/reports/selection_questions/summary.html.erb_spec.rb b/spec/views/reports/selection_questions/summary.html.erb_spec.rb index b3a440c91..7010a291a 100644 --- a/spec/views/reports/selection_questions/summary.html.erb_spec.rb +++ b/spec/views/reports/selection_questions/summary.html.erb_spec.rb @@ -30,7 +30,7 @@ end it "has a back link to the selection from a list of options usage report" do - expect(view.content_for(:back_link)).to have_link("Back to feature usage", href: report_features_path) + expect(view.content_for(:back_link)).to have_link("Back to feature usage", href: report_features_path(tag: :live)) end it "has statistics about questions with autocomplete" do From 4ac2d8711af6ffda8f8897844fea2cb702054f52 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Wed, 21 May 2025 15:55:52 +0300 Subject: [PATCH 4/4] Add link to feature report for draft forms --- app/views/reports/index.html.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb index 12c94ad46..5a1000543 100644 --- a/app/views/reports/index.html.erb +++ b/app/views/reports/index.html.erb @@ -5,6 +5,7 @@
      • <%= govuk_link_to t("reports.add_another_answer.title"), report_add_another_answer_path %>
      • <%= govuk_link_to t("reports.features.title", tag: "live"), report_features_path(tag: :live) %>
        • +
      • <%= govuk_link_to t("reports.features.title", tag: "draft"), report_features_path(tag: :draft) %>
      • <%= govuk_link_to t("reports.features.long_list_usage.title"), report_selection_questions_summary_path %>
      • <%= govuk_link_to t("reports.users.title"), report_users_path %>
      • <%= govuk_link_to t("reports.last_signed_in_at.title"), report_last_signed_in_at_path %>