Remove caching of CSV responses

.github/workflows/build.yml

@@ -496,6 +496,24 @@ jobs:
       - name: Setup
         uses: ./.github/actions/setup
 
+      - name: Save screenshots
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v4
+        with:
+          name: selenium-screenshots-admin
+          path: ${{ github.workspace }}/tmp/capybara/*.png
+          retention-days: 7
+          if-no-files-found: ignore
+
+      - name: Save rails logs
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v4
+        with:
+          name: rails-logs-admin
+          path: ${{ github.workspace }}/log/test.log
+          retention-days: 7
+          if-no-files-found: ignore
+
       - name: Run cucumber specs
         env:
           CUCUMBER_FORMAT: pretty
@@ -543,6 +561,24 @@ jobs:
       - name: Setup
         uses: ./.github/actions/setup
 
+      - name: Save screenshots
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v4
+        with:
+          name: selenium-screenshots-public
+          path: ${{ github.workspace }}/tmp/capybara/*.png
+          retention-days: 7
+          if-no-files-found: ignore
+
+      - name: Save rails logs
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v4
+        with:
+          name: rails-logs-public
+          path: ${{ github.workspace }}/log/test.log
+          retention-days: 7
+          if-no-files-found: ignore
+
       - name: Run cucumber specs
         env:
           CUCUMBER_FORMAT: pretty

app/helpers/cache_helper.rb

@@ -216,13 +216,4 @@ def last_government_response_updated_at
   def last_debate_outcome_updated_at
     @_last_debate_outcome_updated_at ||= DebateOutcome.maximum(:updated_at)
   end
-
-  def csv_cache(name, options = nil, &block)
-    if controller.respond_to?(:perform_caching) && controller.perform_caching
-      key = ActiveSupport::Cache.expand_cache_key(name, :csv)
-      Rails.cache.fetch(key, options, &block)
-    else
-      yield
-    end
-  end
 end

app/views/archived/petitions/index.csv.ruby

@@ -1,6 +1,4 @@
 csv_builder = lambda do |csv|
-  csv << ['Petition', 'URL', 'State', 'Signatures Count']
-
   @petitions.find_each do |petition|
     csv << [
       csv_escape(petition.action),
@@ -11,10 +9,6 @@ csv_builder = lambda do |csv|
   end
 end
 
-if @petitions.query.present?
-  CSV.generate(&csv_builder)
-else
-  csv_cache [:archived_petitions, @parliament, @petitions.scope], expires_in: 5.minutes do
-    CSV.generate(&csv_builder)
-  end
-end
+headers = %["Petition","URL","State","Signatures Count"\n]
+
+CSV.generate(headers, force_quotes: [0, 1, 2], &csv_builder)

app/views/local_petitions/all.csv.ruby

@@ -1,15 +1,15 @@
-csv_cache [:all_local_petitions, @constituency], expires_in: 5.minutes do
-  CSV.generate do |csv|
-    csv << ['Petition', 'URL', 'State', 'Local Signatures', 'Total Signatures']
-
-    @petitions.each do |petition|
-      csv << [
-        csv_escape(petition.action),
-        petition_url(petition),
-        petition.state,
-        petition.constituency_signature_count,
-        petition.signature_count
-      ]
-    end
+csv_builder = lambda do |csv|
+  @petitions.each do |petition|
+    csv << [
+      csv_escape(petition.action),
+      petition_url(petition),
+      petition.state,
+      petition.constituency_signature_count,
+      petition.signature_count
+    ]
   end
 end
+
+headers = %["Petition","URL","State","Local Signatures","Total Signatures"\n]
+
+CSV.generate(headers, force_quotes: [0, 1, 2], &csv_builder)

app/views/local_petitions/show.csv.ruby

@@ -1,15 +1,15 @@
-csv_cache [:local_petitions, @constituency], expires_in: 5.minutes do
-  CSV.generate do |csv|
-    csv << ['Petition', 'URL', 'State', 'Local Signatures', 'Total Signatures']
-
-    @petitions.each do |petition|
-      csv << [
-        csv_escape(petition.action),
-        petition_url(petition),
-        petition.state,
-        petition.constituency_signature_count,
-        petition.signature_count
-      ]
-    end
+csv_builder = lambda do |csv|
+  @petitions.each do |petition|
+    csv << [
+      csv_escape(petition.action),
+      petition_url(petition),
+      petition.state,
+      petition.constituency_signature_count,
+      petition.signature_count
+    ]
   end
 end
+
+headers = %["Petition","URL","State","Local Signatures","Total Signatures"\n]
+
+CSV.generate(headers, force_quotes: [0, 1, 2], &csv_builder)

app/views/petitions/index.csv.ruby

@@ -1,6 +1,4 @@
 csv_builder = lambda do |csv|
-  csv << ['Petition', 'URL', 'State', 'Signatures Count']
-
   @petitions.find_each do |petition|
     csv << [
       csv_escape(petition.action),
@@ -11,10 +9,6 @@ csv_builder = lambda do |csv|
   end
 end
 
-if @petitions.query.present?
-  CSV.generate(&csv_builder)
-else
-  csv_cache [:petitions, @petitions.scope], expires_in: 5.minutes do
-    CSV.generate(&csv_builder)
-  end
-end
+headers = %["Petition","URL","State","Signatures Count"\n]
+
+CSV.generate(headers, force_quotes: [0, 1, 2], &csv_builder)

config/brakeman.ignore

@@ -102,25 +102,6 @@
       ],
       "note": ""
     },
-    {
-      "warning_type": "Unmaintained Dependency",
-      "warning_code": 123,
-      "fingerprint": "40be17ee84017c91503dab23873e24bdd71665ab665bf6f351cfc09ff83e6582",
-      "check_name": "EOLRuby",
-      "message": "Support for Ruby 3.0.6 ends on 2024-03-31",
-      "file": ".ruby-version",
-      "line": 1,
-      "link": "https://brakemanscanner.org/docs/warning_types/unmaintained_dependency/",
-      "code": null,
-      "render_path": null,
-      "location": null,
-      "user_input": null,
-      "confidence": "Medium",
-      "cwe_id": [
-        1104
-      ],
-      "note": ""
-    },
     {
       "warning_type": "SSL Verification Bypass",
       "warning_code": 71,
@@ -162,8 +143,27 @@
         79
       ],
       "note": ""
+    },
+    {
+      "warning_type": "Unmaintained Dependency",
+      "warning_code": 121,
+      "fingerprint": "edf687f759ec9765bd5db185dbc615c80af77d6e7e19386fc42934e7a80307af",
+      "check_name": "EOLRuby",
+      "message": "Support for Ruby 3.0.7 ended on 2024-03-31",
+      "file": ".ruby-version",
+      "line": 1,
+      "link": "https://brakemanscanner.org/docs/warning_types/unmaintained_dependency/",
+      "code": null,
+      "render_path": null,
+      "location": null,
+      "user_input": null,
+      "confidence": "High",
+      "cwe_id": [
+        1104
+      ],
+      "note": ""
     }
   ],
-  "updated": "2024-03-08 13:30:06 +0000",
+  "updated": "2024-05-10 12:37:54 +0000",
   "brakeman_version": "6.1.2"
 }

features/step_definitions/authentication_steps.rb

@@ -10,20 +10,12 @@
 
 Given(/^I log out and login back in as a sysadmin$/) do
   @user = FactoryBot.build(:sysadmin_sso_user)
-
-  steps %q[
-    the admin user is logged out
-    the admin user is logged in
-  ]
+  steps "the admin user is logged in"
 end
 
 Given(/^I log out and login back in as a moderator$/) do
   @user = FactoryBot.build(:moderator_sso_user)
-
-  steps %q[
-    the admin user is logged out
-    the admin user is logged in
-  ]
+  steps "the admin user is logged in"
 end
 
 Given /^I am logged in as a moderator named "([^\"]*)"$/ do |name|
@@ -44,11 +36,24 @@
 end
 
 Given /^the admin user is logged in$/ do
+  visit admin_logout_url
+
+  expect(page).to have_current_path("/admin/login")
+  expect(page).to have_content("You have been logged out")
+
   OmniAuth.config.mock_auth[:example] = @user
 
   visit admin_login_url
   fill_in("Email", with: @user.uid)
   click_button("Sign in")
+
+  if Capybara.current_driver == :chrome_headless
+    expect(page).to have_current_path("/admin")
+  else
+    expect(page).to have_current_path("/admin/auth/example/callback")
+  end
+
+  expect(page).to have_content("Logged in successfully")
 end
 
 Given /^a sysadmin SSO user exists$/ do

features/support/hooks.rb

@@ -58,6 +58,16 @@
   ENV["INLINE_UPDATES"] = "true"
 end
 
+After do
+   OmniAuth.config.mock_auth[:example] = nil
+end
+
+After do |scenario|
+  if scenario.failed? && page.respond_to?(:save_screenshot)
+    page.save_screenshot("#{scenario.name.gsub(' ', '_').gsub(/[^0-9A-Za-z_]/, '')}.png")
+  end
+end
+
 Before('@admin') do
   Capybara.app_host = 'https://moderate.petition.parliament.uk'
   Capybara.default_host = 'https://moderate.petition.parliament.uk'

spec/helpers/cache_helper_spec.rb

@@ -360,54 +360,4 @@
       end
     end
   end
-
-  describe "#csv_cache" do
-    after do
-      Rails.cache.delete("csv/foo")
-    end
-
-    context "when caching is not enabled" do
-      before do
-        allow(controller).to receive(:perform_caching).and_return(false)
-      end
-
-      context "and the cache key is not set" do
-        it "calls the block" do
-          expect { |b| helper.csv_cache("foo", nil, &b) }.to yield_control
-        end
-      end
-
-      context "and the cache key is set" do
-        before do
-          Rails.cache.write("csv/foo", "bar")
-        end
-
-        it "calls the block" do
-          expect { |b| helper.csv_cache("foo", nil, &b) }.to yield_control
-        end
-      end
-    end
-
-    context "when caching is enabled" do
-      before do
-        allow(controller).to receive(:perform_caching).and_return(true)
-      end
-
-      context "and the cache key is not set" do
-        it "calls the block" do
-          expect { |b| helper.csv_cache("foo", nil, &b) }.to yield_control
-        end
-      end
-
-      context "and the cache key is set" do
-        before do
-          Rails.cache.write("csv/foo", "bar")
-        end
-
-        it "doesn't call the block" do
-          expect { |b| helper.csv_cache("foo", nil, &b) }.not_to yield_control
-        end
-      end
-    end
-  end
 end