diff --git a/app/assets/javascripts/local_authorities.js b/app/assets/javascripts/local_authorities.js new file mode 100644 index 000000000..3dd300871 --- /dev/null +++ b/app/assets/javascripts/local_authorities.js @@ -0,0 +1,15 @@ +document.addEventListener("DOMContentLoaded", function(event) { + var links_download_button = document.getElementById('links_download_button'); + var href = links_download_button.href; + + links_download_button.addEventListener('mouseenter', function() { + var checkboxes = document.getElementsByClassName('link_status_checkbox'); + var params = []; + for (var i = 0; i < checkboxes.length; i++) { + if (checkboxes[i].checked) { + params.push(checkboxes[i].name + '=' + checkboxes[i].value + '&') + } + } + links_download_button.href = href + '?' + params.join('') + }); +}); diff --git a/app/controllers/local_authorities_controller.rb b/app/controllers/local_authorities_controller.rb index 30e1076e2..7d5b63c09 100644 --- a/app/controllers/local_authorities_controller.rb +++ b/app/controllers/local_authorities_controller.rb @@ -17,11 +17,11 @@ def show @link_count = links_for_authority.count end - def broken_links_csv + def links_csv @authority = LocalAuthority.find_by_slug!(params[:local_authority_slug]) authority_name = @authority.name.parameterize.underscore - data = LocalLinksManager::Export::LinksExporter.new.export_broken_links(@authority) - send_data data, filename: "#{authority_name}_broken_links.csv" + data = LocalLinksManager::Export::LinksExporter.new.export_links(@authority.id, params) + send_data data, filename: "#{authority_name}_links.csv" end def bad_homepage_url_and_status_csv diff --git a/app/views/shared/_local_authority_details.html.erb b/app/views/shared/_local_authority_details.html.erb index 05a68522b..dee4e4443 100644 --- a/app/views/shared/_local_authority_details.html.erb +++ b/app/views/shared/_local_authority_details.html.erb @@ -1,9 +1,28 @@ +<%= javascript_include_tag params[:controller] %> +

<%= authority.name %>

Homepage <%= link_to_if(authority.homepage_url, nil, authority.homepage_url) %>
<%= authority.homepage_status %> <%= authority.homepage_link_last_checked %> -

<%= link_to('Download broken links', broken_links_csv_local_authority_path, class: "btn btn-default btn-s") %>

+

+ <%= check_box_tag(:ok, :ok, true, class: 'link_status_checkbox') %> + <%= label_tag(:ok, 'OK') %> + <%= check_box_tag(:broken, :broken, true, class: 'link_status_checkbox') %> + <%= label_tag(:broken, 'Broken') %> + <%= check_box_tag(:caution, :caution, true, class: 'link_status_checkbox') %> + <%= label_tag(:caution, 'Caution') %> + <%= check_box_tag(:missing, :missing, true, class: 'link_status_checkbox') %> + <%= label_tag(:missing, 'Missing') %> + <%= check_box_tag(:pending, :pending, true, class: 'link_status_checkbox') %> + <%= label_tag(:pending, 'Pending') %> + <%= link_to( + 'Download links', + links_csv_local_authority_path, + class: "btn btn-default btn-s", + id: "links_download_button" + ) %> +

diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 01ef3e663..b36a7ab3c 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -9,3 +9,4 @@ # Precompile additional assets. # application.js, application.css, and all non-JS/CSS in app/assets folder are already added. # Rails.application.config.assets.precompile += %w( search.js ) +Rails.application.config.assets.precompile += %w( local_authorities.js ) diff --git a/config/routes.rb b/config/routes.rb index 052ec412d..6c55e3859 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,7 +5,7 @@ resources 'local_authorities', only: [:index, :show], param: :local_authority_slug do member do - get 'broken_links_csv' + get 'links_csv' end end diff --git a/lib/local-links-manager/export/links_exporter.rb b/lib/local-links-manager/export/links_exporter.rb index ddfd5b344..920ccb132 100644 --- a/lib/local-links-manager/export/links_exporter.rb +++ b/lib/local-links-manager/export/links_exporter.rb @@ -3,9 +3,29 @@ module LocalLinksManager module Export class LinksExporter - HEADINGS = ["Authority Name", "SNAC", "GSS", "Description", "LGSL", "LGIL", "URL"].freeze - ALL_LINKS_HEADINGS = ["Supported by GOV.UK"].freeze - BROKEN_LINKS_HEADINGS = ["New URL"].freeze + SELECTION = [ + "local_authorities.name", + :snac, + :gss, + "services.label as service_label", + "interactions.label as interaction_label", + "links.status as status", + :lgsl_code, + :lgil_code, + :url, + :enabled + ].freeze + COMMON_HEADINGS = [ + "Authority Name", + "SNAC", + "GSS", + "Description", + "LGSL", + "LGIL", + "URL", + "Supported by GOV.UK" + ].freeze + EXTRA_HEADINGS = ["Status", "New URL"].freeze def self.export_links path = Rails.root.join("public", "data", 'links_to_services_provided_by_local_authorities.csv') @@ -17,54 +37,40 @@ def self.export_links def export(io) output = CSV.generate do |csv| - csv << HEADINGS + ALL_LINKS_HEADINGS + csv << COMMON_HEADINGS records.each do |record| - csv << format(record).push(record.enabled) + csv << format(record) end end io.write(output) end - def export_broken_links(local_authority_id) + def export_links(local_authority_id, params) + statuses = params.slice('ok', 'broken', 'caution', 'missing', 'pending').keys CSV.generate do |csv| - csv << HEADINGS + BROKEN_LINKS_HEADINGS - broken_links(local_authority_id).each do |link| - csv << format(link) + csv << COMMON_HEADINGS + EXTRA_HEADINGS + statuses.each do |status| + links(local_authority_id, status).each do |link| + csv << format(link).push(link.status) + end end end end def records Link.with_url.joins(:local_authority, :service, :interaction) - .select( - "local_authorities.name", - :snac, - :gss, - "services.label as service_label", - "interactions.label as interaction_label", - :lgsl_code, - :lgil_code, - :url, - :enabled - ).order("local_authorities.name", "services.lgsl_code", "interactions.lgil_code").all + .select(*SELECTION) + .order("local_authorities.name", "services.lgsl_code", "interactions.lgil_code").all end private - def broken_links(local_authority_id) - Link.enabled_links.broken + def links(local_authority_id, status) + Link.enabled_links.public_send(status) .where(local_authority_id: local_authority_id) .joins(:local_authority, :service, :interaction) - .select( - "local_authorities.name", - :snac, - :gss, - "services.label as service_label", - "interactions.label as interaction_label", - :lgsl_code, - :lgil_code, - :url, - ).order("services.lgsl_code", "interactions.lgil_code").all + .select(*SELECTION) + .order("services.lgsl_code", "interactions.lgil_code").all end def format(record) @@ -76,6 +82,7 @@ def format(record) record.lgsl_code, record.lgil_code, record.url, + record.enabled ] end diff --git a/spec/controllers/local_authorities_controller_spec.rb b/spec/controllers/local_authorities_controller_spec.rb index 9d4fd0bae..ab5e434e1 100644 --- a/spec/controllers/local_authorities_controller_spec.rb +++ b/spec/controllers/local_authorities_controller_spec.rb @@ -41,14 +41,24 @@ end end - describe "GET broken_links_csv" do + describe "GET links_csv" do before do @local_authority = create(:local_authority) end it "retrieves HTTP success" do login_as_stub_user - get :broken_links_csv, params: { local_authority_slug: @local_authority.slug } + get( + :links_csv, + params: { + local_authority_slug: @local_authority.slug, + ok: 'ok', + broken: 'broken', + caution: 'caution', + missing: 'missing', + pending: 'pending' + } + ) expect(response).to have_http_status(200) expect(response.headers["Content-Type"]).to eq("text/csv") end diff --git a/spec/features/local_authorities/local_authority_show_spec.rb b/spec/features/local_authorities/local_authority_show_spec.rb index 269d0c39f..13b390318 100644 --- a/spec/features/local_authorities/local_authority_show_spec.rb +++ b/spec/features/local_authorities/local_authority_show_spec.rb @@ -1,16 +1,17 @@ require 'rails_helper' feature "The local authority show page" do + let!(:local_authority) { create(:district_council) } + before do User.create(email: 'user@example.com', name: 'Test User', permissions: %w[signin]) - @local_authority = create(:district_council) - visit local_authority_path(local_authority_slug: @local_authority.slug) + visit local_authority_path(local_authority_slug: local_authority.slug) end it 'has a list of breadcrumbs pointing back to the authority that lead us here' do within '.breadcrumb' do expect(page).to have_link 'Local links', href: root_path - expect(page).to have_text @local_authority.name + expect(page).to have_text local_authority.name end end @@ -26,18 +27,18 @@ end it "displays 'No link'" do - @local_authority.homepage_url = nil - @local_authority.save - visit local_authority_path(local_authority_slug: @local_authority.slug) + local_authority.homepage_url = nil + local_authority.save + visit local_authority_path(local_authority_slug: local_authority.slug) within(:css, ".page-title") do expect(page).to have_content('No link') end end it "does not display 'Link not checked'" do - @local_authority.homepage_url = nil - @local_authority.save - visit local_authority_path(local_authority_slug: @local_authority.slug) + local_authority.homepage_url = nil + local_authority.save + visit local_authority_path(local_authority_slug: local_authority.slug) within(:css, ".page-title") do expect(page).not_to have_content('Link not checked') end @@ -45,14 +46,15 @@ end describe "with services present" do + let(:service) { create(:service, :all_tiers) } + let(:disabled_service) { create(:disabled_service) } + let!(:ok_link) { create_service_interaction_link(service, status: :ok) } + let!(:disabled_link) { create_service_interaction_link(disabled_service, status: :ok) } + let!(:broken_link) { create_service_interaction_link(service, status: :broken) } + let!(:missing_link) { create_missing_link(service) } + before do - @service = create(:service, :all_tiers) - @disabled_service = create(:disabled_service) - @good_link = create_service_interaction_link(@service, status: :ok) - @disabled_link = create_service_interaction_link(@disabled_service, status: :ok) - @broken_link = create_service_interaction_link(@service, status: :broken) - @missing_link = create_missing_link(@service) - visit local_authority_path(@local_authority) + visit local_authority_path(local_authority) end let(:http_status) { 200 } @@ -77,11 +79,11 @@ it "shows only the enabled services provided by the authority according to its tier with links to their individual pages" do expect(page).to have_content 'Services and links' - expect(page).to have_text(@good_link.service.label) + expect(page).to have_text(ok_link.service.label) end it "does not show the disabled service interaction" do - expect(page).not_to have_content(@disabled_service.label) + expect(page).not_to have_content(disabled_service.label) end it "shows missing links" do @@ -90,28 +92,68 @@ it "shows each service's LGSL codes in the table" do expect(page).to have_content 'Code' - expect(page).to have_css('td.lgsl', text: @good_link.service.lgsl_code) + expect(page).to have_css('td.lgsl', text: ok_link.service.lgsl_code) end it 'shows the link status as Good Link when the status is 200' do - within(:css, "tr[data-interaction-id=\"#{@good_link.interaction.id}\"]") do + within(:css, "tr[data-interaction-id=\"#{ok_link.interaction.id}\"]") do expect(page).to have_text 'Good' end end it 'shows the link last checked details' do - expect(page).to have_text @good_link.link_last_checked + expect(page).to have_text ok_link.link_last_checked end it 'should have a link to Edit Link' do - expect(page).to have_link 'Edit link', href: edit_link_path(@local_authority, @service, @good_link.interaction) + expect(page).to have_link 'Edit link', href: edit_link_path(local_authority, service, ok_link.interaction) end - it "allows user to download a CSV of broken links for the local authority" do - click_on "Download broken links" - expect(page.response_headers["Content-Type"]).to eq("text/csv") - csv = page.body.chomp.split(",") - expect(csv).to include(@broken_link.url) + describe "CSV download" do + let(:status_checkboxes) { %w(ok broken caution missing pending) } + let(:url_regex) { /http:\/\/.+\/local_authorities\/.+\/links_csv/ } + + it "downloads a CSV" do + click_on "Download links" + + expect(page.response_headers["Content-Type"]).to eq("text/csv") + end + + context "when user leaves all link status checkboxes selected (by default)", js: true do + it "passes all statuses in params" do + submit_button = find("a", text: "Download links") + expect(submit_button['href']).to match(url_regex) + + submit_button.hover + params = submit_button['href'].split('?')[-1].split('&') + + status_checkboxes.each do |status| + expect(params).to include("#{status}=#{status}") + end + end + end + + context "when user deselects some link status checkboxes", js: true do + let(:unchecked_status_checkboxes) { %w(ok caution pending) } + let(:checked_status_checkboxes) { status_checkboxes - unchecked_status_checkboxes } + + it "passes all statuses in params, except the unchecked ones" do + submit_button = find("a", text: "Download links") + expect(submit_button['href']).to match(url_regex) + + unchecked_status_checkboxes.each { |status_checkbox| uncheck status_checkbox } + submit_button.hover + params = submit_button['href'].split('?')[-1].split('&') + + checked_status_checkboxes.each do |status| + expect(params).to include("#{status}=#{status}") + end + + unchecked_status_checkboxes.each do |status| + expect(params).to_not include("#{status}=#{status}") + end + end + end end context 'editing a link' do @@ -120,14 +162,14 @@ fill_in('link_url', with: 'http://angus.example.com/link-to-change') click_on('Update') - expect(page.current_path).to eq(local_authority_path(@local_authority)) + expect(page.current_path).to eq(local_authority_path(local_authority)) end it 'returns you to the correct page after cancelling the editing of a link' do within('.table') { click_on('Edit link', match: :first) } click_on('Cancel') - expect(page.current_path).to eq(local_authority_path(@local_authority)) + expect(page.current_path).to eq(local_authority_path(local_authority)) end end @@ -141,11 +183,11 @@ end it 'shows non-200 status links' do - expect(page).to have_link @broken_link.url + expect(page).to have_link broken_link.url end it 'doesn\'t show 200 status links' do - expect(page).not_to have_link @good_link.url + expect(page).not_to have_link ok_link.url end it 'shows missing links' do @@ -159,7 +201,7 @@ def create_service_interaction_link(service, status:) create( :link, - local_authority: @local_authority, + local_authority: local_authority, service_interaction: service_interaction, status: status ) @@ -170,7 +212,7 @@ def create_missing_link(service) create( :missing_link, - local_authority: @local_authority, + local_authority: local_authority, service_interaction: service_interaction, status: "missing" ) diff --git a/spec/lib/local-links-manager/export/fixtures/bad_links_url_status.csv b/spec/lib/local-links-manager/export/fixtures/bad_links_url_status.csv index 91d5a93fe..1c3d3f4d8 100644 --- a/spec/lib/local-links-manager/export/fixtures/bad_links_url_status.csv +++ b/spec/lib/local-links-manager/export/fixtures/bad_links_url_status.csv @@ -1,5 +1,5 @@ ga:dimension36,ga:dimension37 http://www.carmarthenshire.gov.uk/Cymraeg/addysg/childrens-services/Pages/fostering.aspx,Page not found -http://www.warwickshire.gov.uk/azrecycling,Website unavailable http://www.southoxon.gov.uk/dogwardens,Page requires login +http://www.warwickshire.gov.uk/azrecycling,Website unavailable https://portal.southtyneside.info/eservices/frmHomepage.aspx?FunctionId=79&ignore=0,Security Error diff --git a/spec/lib/local-links-manager/export/links_exporter_spec.rb b/spec/lib/local-links-manager/export/links_exporter_spec.rb index 90d035210..a12e723af 100644 --- a/spec/lib/local-links-manager/export/links_exporter_spec.rb +++ b/spec/lib/local-links-manager/export/links_exporter_spec.rb @@ -8,7 +8,7 @@ def fixture_file(file) let(:exporter) { LocalLinksManager::Export::LinksExporter.new } - describe '#export_links' do + describe '#export' do def test_url(local_authority, interaction) base_url = "http://www.example.com" "#{base_url}/#{local_authority.gss}/#{interaction.lgil_code}" @@ -61,30 +61,50 @@ def test_url(local_authority, interaction) end end - describe "#export_broken_links" do + describe "#export_links" do + let(:headings) { (LocalLinksManager::Export::LinksExporter::COMMON_HEADINGS + LocalLinksManager::Export::LinksExporter::EXTRA_HEADINGS).join(",") } let(:la) { create(:local_authority) } - let(:service) { create(:service) } - let(:disabled_service) { create(:disabled_service) } - let(:interaction_1) { create(:interaction) } - let(:interaction_2) { create(:interaction) } - let(:service_interaction_1) { create(:service_interaction, service: service, interaction: interaction_1) } - let(:service_interaction_2) { create(:service_interaction, service: service, interaction: interaction_2) } - let(:service_interaction_3) { create(:service_interaction, service: disabled_service, interaction: interaction_1) } - let(:broken_link) { create(:broken_link, local_authority: la, url: "http://www.diagonalley.gov.uk/broken-link", service_interaction: service_interaction_1) } - let(:ok_link) { create(:link, local_authority: la, url: "http://www.diagonalley.gov.uk/ok-link", status: "ok", service_interaction: service_interaction_2) } - let(:disabled_link) { create(:broken_link, local_authority: la, url: "http://www.diagonalley.gov.uk/ok-link", service_interaction: service_interaction_3) } - - it "exports broken links for enabled services for a given local authority to CSV format with headings" do - broken_link_data = "#{la.name},#{la.snac},#{la.gss},#{service.label}: #{interaction_1.label},#{service.lgsl_code},#{interaction_1.lgil_code},#{broken_link.url}" - ok_link_data = "#{la.name},#{la.snac},#{la.gss},#{service.label}: #{interaction_2.label},#{service.lgsl_code},#{interaction_2.lgil_code},#{ok_link.url}" - disabled_link_data = "#{la.name},#{la.snac},#{la.gss},#{disabled_service.label}: #{interaction_1.label},#{disabled_service.lgsl_code},#{interaction_1.lgil_code},#{disabled_link.url}" - headings = (LocalLinksManager::Export::LinksExporter::HEADINGS + LocalLinksManager::Export::LinksExporter::BROKEN_LINKS_HEADINGS).join(",") - csv = exporter.export_broken_links(la.id).split("\n") - - expect(csv).to include(broken_link_data) - expect(csv).not_to include(ok_link_data) - expect(csv).not_to include(disabled_link_data) - expect(csv).to include(headings) + let(:ok_link) { create(:ok_link, local_authority: la) } + let(:broken_link) { create(:broken_link, local_authority: la) } + let(:caution_link) { create(:caution_link, local_authority: la) } + let(:missing_link) { create(:missing_link, local_authority: la) } + let(:pending_link) { create(:pending_link, local_authority: la) } + let(:disabled_link) { create(:link_for_disabled_service, local_authority: la) } + let!(:links) do + { + 'ok' => ok_link, + 'broken' => broken_link, + 'caution' => caution_link, + 'missing' => missing_link, + 'pending' => pending_link, + 'disabled' => disabled_link + } + end + + %w(ok broken caution missing pending).each do |status_in_params| + context "when params is {'#{status_in_params}' => '#{status_in_params}'}" do + let(:params) { { status_in_params => status_in_params } } + let(:csv) { exporter.export_links(la.id, params) } + + it "exports #{status_in_params} links for enabled services for a given local authority to CSV format with headings" do + expect(csv).to include(headings) + links.slice(status_in_params).values.each do |link| + expect(csv).to include("#{la.name},#{la.snac},#{la.gss},#{link.service.label}: #{link.interaction.label},#{link.service.lgsl_code},#{link.interaction.lgil_code},#{link.url},#{link.service.enabled},#{link.status}") + end + end + + it "does not export links for disabled services" do + expect(csv).to_not include("#{la.name},#{la.snac},#{la.gss},#{disabled_link.service.label}: #{disabled_link.interaction.label},#{disabled_link.service.lgsl_code},#{disabled_link.interaction.lgil_code},#{disabled_link.url},#{disabled_link.service.enabled},#{disabled_link.status}") + end + + (%w(ok broken caution missing pending) - [status_in_params]).each do |status_not_in_params| + it "does not export #{status_not_in_params} links" do + links.except(status_in_params).values.each do |link| + expect(csv).to_not include("#{la.name},#{la.snac},#{la.gss},#{link.service.label}: #{link.interaction.label},#{link.service.lgsl_code},#{link.interaction.lgil_code},#{link.url},#{link.service.enabled},#{link.status}") + end + end + end + end end end end