From 58ec2c96bbdbca352d901c93a1efc57816982ee2 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Thu, 29 May 2025 12:16:51 +0300 Subject: [PATCH 1/7] Change questions_with_answer_type page download CSV link to go to self Rather than going to the reports#live_csv_questions action for the CSV of questions with answer type, we should use the same pattern as for the other feature reports and use the same URL but with a `.csv` appended for the CSV download link on the questions with answer type report page. This commit updates the #questions_with_answer_type action to accept format `csv` as well as `html`, same as the other feature report actions, and uses that in the view template. We can then also simplify the live_questions_csv action. This probably should have been done in PR #1976, but it was missed. --- app/controllers/reports_controller.rb | 17 +++++++------- .../questions_with_answer_type.html.erb | 2 +- spec/requests/reports_controller_spec.rb | 22 +++++++++++++++++++ ...uestions_with_answer_type.html.erb_spec.rb | 8 +++++-- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index f083a4ce7..7300771b3 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -16,7 +16,13 @@ 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: } + if params[:format] == "csv" + send_data Reports::QuestionsCsvReportService.new(questions).csv, + type: "text/csv; charset=iso-8859-1", + disposition: "attachment; filename=#{questions_csv_filename(answer_type)}" + else + render template: "reports/questions_with_answer_type", locals: { answer_type:, questions: } + end end def questions_with_add_another_answer @@ -120,17 +126,12 @@ def live_forms_csv end def live_questions_csv - answer_type = params[:answer_type] forms = Reports::FormDocumentsService.live_form_documents - questions = if answer_type - Reports::FeatureReportService.new(forms).questions_with_answer_type(answer_type) - else - Reports::FeatureReportService.new(forms).questions - end + questions = Reports::FeatureReportService.new(forms).questions send_data Reports::QuestionsCsvReportService.new(questions).csv, type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{questions_csv_filename(answer_type)}" + disposition: "attachment; filename=#{csv_filename('live_questions_report')}" end private diff --git a/app/views/reports/questions_with_answer_type.html.erb b/app/views/reports/questions_with_answer_type.html.erb index caf3f82ce..7a962df5c 100644 --- a/app/views/reports/questions_with_answer_type.html.erb +++ b/app/views/reports/questions_with_answer_type.html.erb @@ -4,7 +4,7 @@

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

-

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

+

<%=govuk_link_to(t(".download_csv"), url_for(format: :csv))%>

diff --git a/spec/requests/reports_controller_spec.rb b/spec/requests/reports_controller_spec.rb index 68321ff9e..006d2b160 100644 --- a/spec/requests/reports_controller_spec.rb +++ b/spec/requests/reports_controller_spec.rb @@ -828,6 +828,28 @@ end end + describe "#questions_with_answer_type as csv" do + before do + login_as_super_admin_user + + travel_to Time.utc(2025, 5, 15, 15, 31, 57) + + get report_questions_with_answer_type_path(answer_type: "text", format: :csv) + end + + it_behaves_like "csv response" + + it "responds with an attachment content-disposition header" do + expect(response.headers["content-disposition"]).to match("attachment; filename=live_questions_report_text_answer_type-2025-05-15 15:31:57 UTC.csv") + end + + it "has expected response body" do + csv = CSV.parse(response.body, headers: true) + expect(csv.headers).to eq Reports::QuestionsCsvReportService::QUESTIONS_CSV_HEADERS + expect(csv.length).to eq 5 + end + end + describe "#questions_with_add_another_answer as csv" do before do login_as_super_admin_user 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..d82ab3a35 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 @@ -8,8 +8,12 @@ ] end + let(:answer_type) { "email" } + before do - render locals: { answer_type: "email", questions: } + controller.request.path_parameters[:answer_type] = answer_type + + render locals: { answer_type:, questions: } end describe "page title" do @@ -23,7 +27,7 @@ end it "has a link to download the CSV" do - expect(rendered).to have_link("Download all questions in live forms with this answer type as a CSV file", href: report_live_questions_csv_path(answer_type: "email")) + expect(rendered).to have_link("Download all questions in live forms with this answer type as a CSV file", href: report_questions_with_answer_type_path(answer_type: "email", format: :csv)) end describe "questions table" do From e7873787635502831ff615c2e7871cfd35d207d7 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 23 May 2025 10:21:20 +0300 Subject: [PATCH 2/7] Refactor ReportsController to reduce repetition Co-authored-by: Stephen Daly --- app/controllers/reports_controller.rb | 52 +++++++++++++-------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 7300771b3..ba65922cb 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -29,52 +29,28 @@ def questions_with_add_another_answer forms = Reports::FormDocumentsService.live_form_documents questions = Reports::FeatureReportService.new(forms).questions_with_add_another_answer - if params[:format] == "csv" - send_data Reports::QuestionsCsvReportService.new(questions).csv, - type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{csv_filename('live_questions_with_add_another_answer_report')}" - else - render template: "reports/feature_report", locals: { report: params[:action], records: questions } - end + questions_feature_report(params[:action], questions) end def forms_with_routes forms = Reports::FormDocumentsService.live_form_documents forms = Reports::FeatureReportService.new(forms).forms_with_routes - if params[:format] == "csv" - send_data Reports::FormsCsvReportService.new(forms).csv, - type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{csv_filename('live_forms_with_routes_report')}" - else - render template: "reports/feature_report", locals: { report: params[:action], records: forms } - end + forms_feature_report(params[:action], forms) end def forms_with_payments forms = Reports::FormDocumentsService.live_form_documents forms = Reports::FeatureReportService.new(forms).forms_with_payments - if params[:format] == "csv" - send_data Reports::FormsCsvReportService.new(forms).csv, - type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{csv_filename('live_forms_with_payments_report')}" - else - render template: "reports/feature_report", locals: { report: params[:action], records: forms } - end + forms_feature_report(params[:action], forms) end def forms_with_csv_submission_enabled forms = Reports::FormDocumentsService.live_form_documents forms = Reports::FeatureReportService.new(forms).forms_with_csv_submission_enabled - if params[:format] == "csv" - send_data Reports::FormsCsvReportService.new(forms).csv, - type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{csv_filename('live_forms_with_csv_submission_enabled_report')}" - else - render template: "reports/feature_report", locals: { report: params[:action], records: forms } - end + forms_feature_report(params[:action], forms) end def users @@ -136,6 +112,26 @@ def live_questions_csv private + def questions_feature_report(report, questions) + if params[:format] == "csv" + send_data Reports::QuestionsCsvReportService.new(questions).csv, + type: "text/csv; charset=iso-8859-1", + disposition: "attachment; filename=#{csv_filename("live_#{report}_report")}" + else + render template: "reports/feature_report", locals: { report:, records: questions } + end + end + + def forms_feature_report(report, forms) + if params[:format] == "csv" + send_data Reports::FormsCsvReportService.new(forms).csv, + type: "text/csv; charset=iso-8859-1", + disposition: "attachment; filename=#{csv_filename("live_#{report}_report")}" + else + render template: "reports/feature_report", locals: { report:, records: forms } + end + end + def check_user_has_permission authorize Report, :can_view_reports? end From 02ada1c303527c289fd8bdaf658202e762e80576 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Wed, 21 May 2025 11:45:09 +0300 Subject: [PATCH 3/7] Change feature report views to work with either live or draft forms --- app/controllers/reports_controller.rb | 8 ++--- 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 | 7 +++- spec/views/reports/features.html.erb_spec.rb | 13 ++++++- ...uestions_with_answer_type.html.erb_spec.rb | 4 ++- 9 files changed, 62 insertions(+), 42 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index ba65922cb..af9a2f6e0 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 @@ -21,7 +21,7 @@ def questions_with_answer_type type: "text/csv; charset=iso-8859-1", disposition: "attachment; filename=#{questions_csv_filename(answer_type)}" else - 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 end @@ -118,7 +118,7 @@ def questions_feature_report(report, questions) type: "text/csv; charset=iso-8859-1", disposition: "attachment; filename=#{csv_filename("live_#{report}_report")}" else - render template: "reports/feature_report", locals: { report:, records: questions } + render template: "reports/feature_report", locals: { tag: "live", report:, records: questions } end end @@ -128,7 +128,7 @@ def forms_feature_report(report, forms) type: "text/csv; charset=iso-8859-1", disposition: "attachment; filename=#{csv_filename("live_#{report}_report")}" else - render template: "reports/feature_report", locals: { report:, records: forms } + render template: "reports/feature_report", locals: { tag: "live", report:, records: forms } 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 725646a7c..dcecea0fb 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], report_forms_with_routes_path, 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], report_forms_with_payments_path, 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], report_questions_with_add_another_answer_path, 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], report_forms_with_csv_submission_enabled_path, 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 7a962df5c..0331e035a 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"), url_for(format: :csv))%>

    +

    <%=govuk_link_to(t(".download_csv", tag:).upcase_first, url_for(format: :csv))%>

    diff --git a/config/locales/en.yml b/config/locales/en.yml index e05e81396..5dc07e96c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1385,18 +1385,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 @@ -1404,14 +1404,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: @@ -1421,11 +1421,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 9d8cd48ca..528626f72 100644 --- a/spec/views/reports/feature_report.html.erb_spec.rb +++ b/spec/views/reports/feature_report.html.erb_spec.rb @@ -3,11 +3,12 @@ describe "reports/feature_report" do let(:report) {} let(:records) { [] } + let(:tag) { "live" } before do controller.request.path_parameters[:action] = report - render locals: { report:, records: } + render locals: { tag:, report:, records: } end context "with forms_with_csv_submission_enabled report" do @@ -22,6 +23,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 +70,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 +117,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 +167,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 d82ab3a35..eacececd1 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,18 +7,20 @@ { "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" } let(:answer_type) { "email" } before do controller.request.path_parameters[:answer_type] = answer_type - render locals: { answer_type:, questions: } + render locals: { tag:, answer_type:, 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 d1e09b9e7b276e2b4a365cf966d9f721a382aaae Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 20 May 2025 15:13:52 +0300 Subject: [PATCH 4/7] 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 8e56a71aa6c1f1eab477c2f08730da7e1201955a Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Wed, 21 May 2025 09:49:50 +0300 Subject: [PATCH 5/7] Change feature report routes to have parameter for live or draft forms --- app/controllers/reports_controller.rb | 48 +++---- app/views/reports/index.html.erb | 2 +- .../selection_questions/summary.html.erb | 2 +- config/routes.rb | 2 +- spec/requests/reports_controller_spec.rb | 46 +++---- spec/routing/reports_routing_spec.rb | 118 ++++++++++++------ .../reports/feature_report.html.erb_spec.rb | 1 + 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 | 1 + .../summary.html.erb_spec.rb | 2 +- 11 files changed, 155 insertions(+), 85 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index af9a2f6e0..c03851099 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -5,52 +5,58 @@ 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) if params[:format] == "csv" send_data Reports::QuestionsCsvReportService.new(questions).csv, type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{questions_csv_filename(answer_type)}" + disposition: "attachment; filename=#{questions_csv_filename(tag, answer_type)}" else - 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 end def questions_with_add_another_answer - forms = Reports::FormDocumentsService.live_form_documents + tag = params[:tag] + forms = Reports::FormDocumentsService.form_documents(tag:) questions = Reports::FeatureReportService.new(forms).questions_with_add_another_answer - questions_feature_report(params[:action], questions) + questions_feature_report(tag, params[:action], questions) end def forms_with_routes - forms = Reports::FormDocumentsService.live_form_documents + tag = params[:tag] + forms = Reports::FormDocumentsService.form_documents(tag:) forms = Reports::FeatureReportService.new(forms).forms_with_routes - forms_feature_report(params[:action], forms) + forms_feature_report(tag, params[:action], forms) end def forms_with_payments - forms = Reports::FormDocumentsService.live_form_documents + tag = params[:tag] + forms = Reports::FormDocumentsService.form_documents(tag:) forms = Reports::FeatureReportService.new(forms).forms_with_payments - forms_feature_report(params[:action], forms) + forms_feature_report(tag, params[:action], forms) end def forms_with_csv_submission_enabled - forms = Reports::FormDocumentsService.live_form_documents + tag = params[:tag] + forms = Reports::FormDocumentsService.form_documents(tag:) forms = Reports::FeatureReportService.new(forms).forms_with_csv_submission_enabled - forms_feature_report(params[:action], forms) + forms_feature_report(tag, params[:action], forms) end def users @@ -112,23 +118,23 @@ def live_questions_csv private - def questions_feature_report(report, questions) + def questions_feature_report(tag, report, questions) if params[:format] == "csv" send_data Reports::QuestionsCsvReportService.new(questions).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: questions } + render template: "reports/feature_report", locals: { tag:, report:, records: questions } end end - def forms_feature_report(report, forms) + def forms_feature_report(tag, report, forms) if params[:format] == "csv" send_data Reports::FormsCsvReportService.new(forms).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: forms } + render template: "reports/feature_report", locals: { tag:, report:, records: forms } end end @@ -136,8 +142,8 @@ def check_user_has_permission authorize Report, :can_view_reports? end - def questions_csv_filename(answer_type) - base_name = "live_questions_report" + def questions_csv_filename(tag, answer_type) + base_name = "#{tag}_questions_report" base_name += "_#{answer_type}_answer_type" if answer_type.present? csv_filename(base_name) 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 20e74299b..7ad85d294 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -197,7 +197,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 "questions-with-add-another-answer", to: "reports#questions_with_add_another_answer", as: :report_questions_with_add_another_answer diff --git a/spec/requests/reports_controller_spec.rb b/spec/requests/reports_controller_spec.rb index 006d2b160..09215983b 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 report_questions_with_add_another_answer_path + get report_questions_with_add_another_answer_path(tag: :live) end it "returns http code 403" do @@ -209,7 +209,7 @@ before do login_as_organisation_admin_user - get report_questions_with_add_another_answer_path + get report_questions_with_add_another_answer_path(tag: :live) end it "returns http code 403" do @@ -225,7 +225,7 @@ before do login_as_super_admin_user - get report_questions_with_add_another_answer_path + get report_questions_with_add_another_answer_path(tag: :live) end it "returns http code 200" do @@ -251,7 +251,7 @@ before do login_as_standard_user - get report_forms_with_routes_path + get report_forms_with_routes_path(tag: :live) end it "returns http code 403" do @@ -267,7 +267,7 @@ before do login_as_organisation_admin_user - get report_forms_with_routes_path + get report_forms_with_routes_path(tag: :live) end it "returns http code 403" do @@ -283,7 +283,7 @@ before do login_as_super_admin_user - get report_forms_with_routes_path + get report_forms_with_routes_path(tag: :live) end it "returns http code 200" do @@ -309,7 +309,7 @@ before do login_as_standard_user - get report_forms_with_payments_path + get report_forms_with_payments_path(tag: :live) end it "returns http code 403" do @@ -325,7 +325,7 @@ before do login_as_organisation_admin_user - get report_forms_with_payments_path + get report_forms_with_payments_path(tag: :live) end it "returns http code 403" do @@ -341,7 +341,7 @@ before do login_as_super_admin_user - get report_forms_with_payments_path + get report_forms_with_payments_path(tag: :live) end it "returns http code 200" do @@ -367,7 +367,7 @@ before do login_as_standard_user - get report_forms_with_csv_submission_enabled_path + get report_forms_with_csv_submission_enabled_path(tag: :live) end it "returns http code 403" do @@ -383,7 +383,7 @@ before do login_as_organisation_admin_user - get report_forms_with_csv_submission_enabled_path + get report_forms_with_csv_submission_enabled_path(tag: :live) end it "returns http code 403" do @@ -399,7 +399,7 @@ before do login_as_super_admin_user - get report_forms_with_csv_submission_enabled_path + get report_forms_with_csv_submission_enabled_path(tag: :live) end it "returns http code 200" do @@ -736,7 +736,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get report_forms_with_routes_path(format: :csv) + get report_forms_with_routes_path(tag: :live, format: :csv) end it_behaves_like "csv response" @@ -762,7 +762,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get report_forms_with_payments_path(format: :csv) + get report_forms_with_payments_path(tag: :live, format: :csv) end it_behaves_like "csv response" @@ -787,7 +787,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get report_forms_with_csv_submission_enabled_path(format: :csv) + get report_forms_with_csv_submission_enabled_path(tag: :live, format: :csv) end it_behaves_like "csv response" @@ -834,7 +834,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get report_questions_with_answer_type_path(answer_type: "text", format: :csv) + get report_questions_with_answer_type_path(tag: :live, answer_type: "text", format: :csv) end it_behaves_like "csv response" @@ -856,7 +856,7 @@ travel_to Time.utc(2025, 5, 15, 15, 31, 57) - get report_questions_with_add_another_answer_path(format: :csv) + get report_questions_with_add_another_answer_path(tag: :live, 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 c2aad0ddd..54ee7efe3 100644 --- a/spec/routing/reports_routing_spec.rb +++ b/spec/routing/reports_routing_spec.rb @@ -3,62 +3,108 @@ 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") - end + %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 - 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") - end + it "routes to #questions_with_answer_type for #{tag} forms" do + expect(get: "/reports/features/#{tag}/questions-with-answer-type/text").to route_to("reports#questions_with_answer_type", tag:, answer_type: "text") + end - it "routes to #questions_with_add_another_answer" do - expect(get: "/reports/features/questions-with-add-another-answer").to route_to("reports#questions_with_add_another_answer") - end + it "routes to #questions_with_add_another_answer for #{tag} forms" do + expect(get: "/reports/features/#{tag}/questions-with-add-another-answer").to route_to("reports#questions_with_add_another_answer", tag:) + end - it "routes to #questions_with_add_another_answer with csv format" do - expect(get: "/reports/features/questions-with-add-another-answer.csv").to route_to("reports#questions_with_add_another_answer", format: "csv") - end + it "routes to #questions_with_add_another_answer for #{tag} forms with csv format" do + expect(get: "/reports/features/#{tag}/questions-with-add-another-answer.csv").to route_to("reports#questions_with_add_another_answer", tag:, format: "csv") + end - it "routes to #forms_with_routes" do - expect(get: "/reports/features/forms-with-routes").to route_to("reports#forms_with_routes") - end + it "routes to #forms_with_routes for #{tag} forms" do + expect(get: "/reports/features/#{tag}/forms-with-routes").to route_to("reports#forms_with_routes", tag:) + end - it "routes to #forms_with_routes with csv format" do - expect(get: "/reports/features/forms-with-routes.csv").to route_to("reports#forms_with_routes", format: "csv") - end + it "routes to #forms_with_routes for #{tag} forms with csv format" do + expect(get: "/reports/features/#{tag}/forms-with-routes.csv").to route_to("reports#forms_with_routes", tag:, format: "csv") + end - it "routes to #forms_with_payments" do - expect(get: "/reports/features/forms-with-payments").to route_to("reports#forms_with_payments") - end + it "routes to #forms_with_payments for #{tag} forms" do + expect(get: "/reports/features/#{tag}/forms-with-payments").to route_to("reports#forms_with_payments", tag:) + end - it "routes to #forms_with_payments with csv format" do - expect(get: "/reports/features/forms-with-payments.csv").to route_to("reports#forms_with_payments", format: "csv") - end + it "routes to #forms_with_payments for #{tag} forms with csv format" do + expect(get: "/reports/features/#{tag}/forms-with-payments.csv").to route_to("reports#forms_with_payments", tag:, format: "csv") + end + + it "routes to #forms_with_csv_submission_enabled for #{tag} forms" do + expect(get: "/reports/features/#{tag}/forms-with-csv-submission-enabled").to route_to("reports#forms_with_csv_submission_enabled", tag:) + end - it "routes to #forms_with_csv_submission_enabled" do - expect(get: "/reports/features/forms-with-csv-submission-enabled").to route_to("reports#forms_with_csv_submission_enabled") + it "routes to #forms_with_csv_submission_enabled for #{tag} forms with csv format" do + expect(get: "/reports/features/#{tag}/forms-with-csv-submission-enabled.csv").to route_to("reports#forms_with_csv_submission_enabled", tag:, format: "csv") + end + end end - it "routes to #forms_with_csv_submission_enabled with csv format" do - expect(get: "/reports/features/forms-with-csv-submission-enabled.csv").to route_to("reports#forms_with_csv_submission_enabled", format: "csv") + context "with invalid tag" do + it "does not route to #features" do + expect(get: "/reports/features/foo").not_to route_to("reports#features", tag: "foo") + end + + it "does not route to #questions_with_answer_type" do + expect(get: "/reports/features/bar/questions-with-answer-type/text").not_to route_to("reports#questions_with_answer_type", answer_type: "text", tag: "bar") + end + + it "does not route to #questions_with_add_another_answer" do + expect(get: "/reports/features/baz/questions-with-add-another-answer").not_to route_to("reports#questions_with_add_another_answer", tag: "baz") + end + + it "does not route to #questions_with_add_another_answer with csv format" do + expect(get: "/reports/features/qux/questions-with-add-another-answer.csv").not_to route_to("reports#questions_with_add_another_answer", format: "csv", tag: "qux") + end + + it "does not route to #forms_with_routes" do + expect(get: "/reports/features/thud/forms-with-routes").not_to route_to("reports#forms_with_routes", tag: "thud") + end + + it "does not route to #forms_with_routes with csv format" do + expect(get: "/reports/features/foo/forms-with-routes.csv").not_to route_to("reports#forms_with_routes", format: "csv", tag: "foo") + end + + it "does not route to #forms_with_payments" do + expect(get: "/reports/features/bar/forms-with-payments").not_to route_to("reports#forms_with_payments", tag: "bar") + end + + it "does not route to #forms_with_payments with csv format" do + expect(get: "/reports/features/baz/forms-with-payments.csv").not_to route_to("reports#forms_with_payments", format: "csv", tag: "baz") + end + + it "does not route to #forms_with_csv_submission_enabled" do + expect(get: "/reports/features/qux/forms-with-csv-submission-enabled").not_to route_to("reports#forms_with_csv_submission_enabled", tag: "qux") + end + + it "does not route to #forms_with_csv_submission_enabled with csv format" do + expect(get: "/reports/features/thud/forms-with-csv-submission-enabled.csv").not_to route_to("reports#forms_with_csv_submission_enabled", format: "csv", tag: "thud") + end end end describe "path helpers" do - it "routes to #questions_with_add_another_answer as csv" do - expect(get: report_questions_with_add_another_answer_path(format: :csv)).to route_to("reports#questions_with_add_another_answer", format: "csv") + it "routes to #questions_with_add_another_answer for live forms as csv" do + expect(get: report_questions_with_add_another_answer_path(tag: :live, format: :csv)).to route_to("reports#questions_with_add_another_answer", tag: "live", format: "csv") end - it "routes to #forms_with_routes as csv" do - expect(get: report_forms_with_routes_path(format: :csv)).to route_to("reports#forms_with_routes", format: "csv") + it "routes to #forms_with_routes for live forms as csv" do + expect(get: report_forms_with_routes_path(tag: :live, format: :csv)).to route_to("reports#forms_with_routes", tag: "live", format: "csv") end - it "routes to #forms_with_payments as csv" do - expect(get: report_forms_with_payments_path(format: :csv)).to route_to("reports#forms_with_payments", format: "csv") + it "routes to #forms_with_payments for live forms as csv" do + expect(get: report_forms_with_payments_path(tag: :live, format: :csv)).to route_to("reports#forms_with_payments", tag: "live", format: "csv") end - it "routes to #forms_with_csv_submission_enabled as csv" do - expect(get: report_forms_with_csv_submission_enabled_path(format: :csv)).to route_to("reports#forms_with_csv_submission_enabled", format: "csv") + it "routes to #forms_with_csv_submission_enabled for live forms as csv" do + expect(get: report_forms_with_csv_submission_enabled_path(tag: :live, format: :csv)).to route_to("reports#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 528626f72..1f2d257fd 100644 --- a/spec/views/reports/feature_report.html.erb_spec.rb +++ b/spec/views/reports/feature_report.html.erb_spec.rb @@ -7,6 +7,7 @@ before do controller.request.path_parameters[:action] = report + 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 eacececd1..da79b9f9d 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 @@ -12,6 +12,7 @@ let(:answer_type) { "email" } before do + controller.request.path_parameters[:tag] = tag controller.request.path_parameters[:answer_type] = answer_type render locals: { tag:, answer_type:, questions: } 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 184375eadb7e043f82960e374e8dbf96965aa349 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Wed, 21 May 2025 15:55:52 +0300 Subject: [PATCH 6/7] 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 %>
      • From 84773bbc0c131f745ed2b5db2f405bc1d252e9da Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Thu, 29 May 2025 15:10:22 +0300 Subject: [PATCH 7/7] Fix feature report template error when records is empty list The `report_table` helper can't handle it if `records` is an empty list and raises an exception; this commit fixes that by not calling it if `records` is empty. For friendliness, we instead show the user a message making it clear there are no forms/questions to show. For completeness we also make the same change to the questions_with_answer_type report view, even though it's not strictly necessary. --- app/views/reports/feature_report.html.erb | 10 ++++++++-- .../questions_with_answer_type.html.erb | 16 +++++++++++----- config/locales/en.yml | 5 +++++ .../reports/feature_report.html.erb_spec.rb | 18 ++++++++++++++++++ ...questions_with_answer_type.html.erb_spec.rb | 16 ++++++++++++++++ 5 files changed, 58 insertions(+), 7 deletions(-) diff --git a/app/views/reports/feature_report.html.erb b/app/views/reports/feature_report.html.erb index ceddf1390..a1ce1a80d 100644 --- a/app/views/reports/feature_report.html.erb +++ b/app/views/reports/feature_report.html.erb @@ -4,10 +4,16 @@

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

        -

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

        + <% unless records.empty? %> +

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

        + <% end %>
        - <%= govuk_table(**report_table(records)) %> + <% if records.empty? %> +

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

        + <% else %> + <%= govuk_table(**report_table(records)) %> + <% end %>
      diff --git a/app/views/reports/questions_with_answer_type.html.erb b/app/views/reports/questions_with_answer_type.html.erb index 0331e035a..eafba9b16 100644 --- a/app/views/reports/questions_with_answer_type.html.erb +++ b/app/views/reports/questions_with_answer_type.html.erb @@ -4,13 +4,19 @@

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

      -

      <%=govuk_link_to(t(".download_csv", tag:).upcase_first, url_for(format: :csv))%>

      + <% unless questions.empty? %> +

      <%=govuk_link_to(t(".download_csv", tag:).upcase_first, url_for(format: :csv))%>

      + <% end %>
      - <%= govuk_table( - head: report_questions_table_head, - rows: report_questions_table_rows(questions), - ) %> + <% if questions.empty? %> +

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

      + <% else %> + <%= govuk_table( + head: report_questions_table_head, + rows: report_questions_table_rows(questions), + ) %> + <% end %>
      diff --git a/config/locales/en.yml b/config/locales/en.yml index 5dc07e96c..5962e5e45 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1405,12 +1405,15 @@ en: question_text: Question text forms_with_csv_submission_enabled: download_csv: Download data about all %{tag} forms with CSV submission enabled as a CSV file + empty: There are no %{tag} forms with CSV submission enabled heading: "%{tag} forms with CSV submission enabled" forms_with_payments: download_csv: Download data about all %{tag} forms with payments as a CSV file + empty: There are no %{tag} forms with payments heading: "%{tag} forms with payments" forms_with_routes: download_csv: Download data about all %{tag} forms with routes as a CSV file + empty: There are no %{tag} forms with routes heading: "%{tag} forms with routes" index: title: Reports @@ -1422,9 +1425,11 @@ en: title: When users last signed in questions_with_add_another_answer: download_csv: Download all questions with add another answer in %{tag} forms as a CSV file + empty: There are no questions with add another answer in %{tag} forms heading: Questions with add another answer in %{tag} forms questions_with_answer_type: download_csv: Download all questions in %{tag} forms with this answer type as a CSV file + empty: There are no %{tag} questions with %{answer_type} answer type heading: "%{tag} questions with %{answer_type} answer type" selection_questions: autocomplete: diff --git a/spec/views/reports/feature_report.html.erb_spec.rb b/spec/views/reports/feature_report.html.erb_spec.rb index 1f2d257fd..8f163793d 100644 --- a/spec/views/reports/feature_report.html.erb_spec.rb +++ b/spec/views/reports/feature_report.html.erb_spec.rb @@ -205,4 +205,22 @@ end end end + + context "when there are no records to render" do + let(:report) { "forms_with_csv_submission_enabled" } + let(:records) { [] } + let(:tag) { "live" } + + it "does not have a link to download a CSV" do + expect(rendered).not_to have_link(href: url_for(format: :csv)) + end + + it "does not render a table" do + expect(rendered).not_to have_table + end + + it "renders the empty message" do + expect(rendered).to include I18n.t("reports.#{report}.empty", tag:) + end + 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 da79b9f9d..54cf99e11 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 @@ -57,4 +57,20 @@ end end end + + context "when there are no questions to render" do + let(:questions) { [] } + + it "does not have a link to download a CSV" do + expect(rendered).not_to have_link(href: url_for(format: :csv)) + end + + it "does not render a table" do + expect(rendered).not_to have_table + end + + it "renders the empty message" do + expect(rendered).to include I18n.t("reports.questions_with_answer_type.empty", tag:, answer_type:) + end + end end