Merge pull request #4228 from alphagov/improve-coverage

Improve coverage

app/controllers/help_controller.rb

@@ -32,8 +32,4 @@ def sign_in
   def content_item_slug
     request.path
   end
-
-  def slug_param
-    params[:slug] || "help"
-  end
 end

app/controllers/sessions_controller.rb

@@ -42,16 +42,17 @@ def http_referer_path
         http_referer = request.headers["HTTP_REFERER"]
 
         if http_referer&.start_with?(Plek.new.website_root)
-          http_referer.delete_prefix Plek.new.website_root
+          http_referer = http_referer.delete_prefix Plek.new.website_root
         end
+
+        http_referer
       end
   end
 
   def is_valid_redirect_path?(value)
     return true if value.blank?
     return false if value.starts_with? "//"
     return true if value.starts_with? "/"
-    return true if value.starts_with?("http://") && Rails.env.development?
 
     false
   end

app/models/election_postcode.rb

@@ -23,7 +23,9 @@ def postcode_for_api
   end
 
   def valid?
-    sanitized_postcode.match?(UK_POSTCODE_PATTERN) unless sanitized_postcode.nil?
+    return false unless sanitized_postcode
+
+    sanitized_postcode.match?(UK_POSTCODE_PATTERN)
   end
 
   def error

app/models/locations_api_postcode_response.rb

@@ -7,10 +7,6 @@ def initialize(postcode, local_custodian_codes, error)
     @error = error
   end
 
-  def location_found?
-    local_custodian_codes.present?
-  end
-
   def location_not_found?
     postcode && local_custodian_codes.empty? && error.nil?
   end

app/presenters/licence_details_presenter.rb

@@ -72,10 +72,6 @@ def action
     interaction
   end
 
-  def offered_by_county?
-    licence_details["is_offered_by_county"]
-  end
-
   def single_licence_authority_present?
     licence_authority_specific? && authority.present?
   end

app/presenters/place_presenter.rb

@@ -8,7 +8,7 @@ class PlacePresenter < ContentItemPresenter
 
   PASS_THROUGH_DETAILS_KEYS.each do |key|
     define_method key do
-      details[key.to_s] if details
+      details[key.to_s]
     end
   end
 
@@ -20,7 +20,7 @@ def initialize(content_item, places = [])
   end
 
   def preposition
-    return "near" unless @places&.any?
+    return "near" unless @places.any?
 
     @places.first["gss"].blank? ? "near" : "for"
   end

app/presenters/travel_advice_index_presenter.rb

@@ -28,9 +28,7 @@ def countries_recently_updated
 
   def countries_grouped_by_initial_letter
     groups = countries.group_by do |country|
-      if country&.name
-        ordered_country_name(country.name)[0]
-      end
+      ordered_country_name(country.name)[0]
     end
 
     groups.sort_by { |initial, _| initial }

spec/models/election_postcode_spec.rb

@@ -8,6 +8,12 @@
   end
 
   describe "#valid?" do
+    it "rejects nil postcodes" do
+      subject = described_class.new(nil)
+
+      expect(subject.valid?).to be false
+    end
+
     it "rejects invalid postcodes" do
       subject = described_class.new("NOTVALID")
 
@@ -37,7 +43,7 @@
     end
   end
 
-  describe "#errors" do
+  describe "#error" do
     it "returns an errors for an empty postcode after validation" do
       subject = described_class.new("  +  ")
 
@@ -49,5 +55,11 @@
 
       expect(subject.error).to eq("invalidPostcodeFormat")
     end
+
+    it "returns nil errors for a valid postcode" do
+      subject = described_class.new("SW1A 1AA")
+
+      expect(subject.error).to be_nil
+    end
   end
 end

spec/models/uprn_spec.rb

@@ -40,6 +40,12 @@
   end
 
   describe "#error" do
+    it "returns nil for a valid Uprn" do
+      subject = described_class.new("012345678910")
+
+      expect(subject.error).to be_nil
+    end
+
     it "detects empty uprn as invalid" do
       subject = described_class.new("    ")
 

spec/requests/sessions_spec.rb

@@ -44,7 +44,7 @@
         end
 
         it "does not take the redirect_path from the referer" do
-          get "/account"
+          get "/account", headers: { "Referer" => @referer }
 
           expect(@stub_with_redirect_path).not_to have_been_requested
           expect(response).to redirect_to("https://home.account.gov.uk")
@@ -59,7 +59,7 @@
         end
 
         it "does not take the redirect_path from the referer" do
-          get "/account"
+          get "/account", headers: { "Referer" => @referer }
 
           expect(@stub_with_redirect_path).not_to have_been_requested
           expect(response).to redirect_to("https://home.account.gov.uk")

spec/system/developer_root_spec.rb

@@ -0,0 +1,9 @@
+RSpec.describe "Developer Root" do
+  context "when visiting /development" do
+    it "includes a link to at least one example" do
+      visit "/development"
+
+      expect(page).to have_link("/", href: "/")
+    end
+  end
+end

spec/system/sign_in_spec.rb

@@ -0,0 +1,31 @@
+RSpec.describe "Sign in" do
+  context "rendering the sign-in help page" do
+    before do
+      payload = {
+        base_path: "/sign-in",
+        format: "special_route",
+        title: "Sign in to a service",
+        description: "",
+        details: {
+          body: "",
+        },
+      }
+      stub_content_store_has_item("/sign-in", payload)
+    end
+
+    it "renders the sign-in help page correctly" do
+      visit "/sign-in"
+
+      expect(page).to have_title("Sign in to a service")
+
+      expect(page).to have_text("Search GOV.UK for a service")
+      expect(page).to have_button("Search")
+    end
+
+    it "includes search facets as hidden elements" do
+      visit "/sign-in"
+
+      expect(page).to have_field("content_purpose_supergroup[]", type: :hidden, with: "services")
+    end
+  end
+end

spec/unit/presenters/content_item_presenter_spec.rb

@@ -42,6 +42,10 @@ def subject(content_item)
       datetime = 1.minute.ago
       expect(subject(public_updated_at: datetime.to_s).updated_at.to_i).to eq(datetime.to_i)
     end
+
+    it "returns the subject public_updated_at" do
+      expect(subject({}).updated_at).to be_nil
+    end
   end
 
   describe "#locale" do

spec/unit/presenters/place_presenter_spec.rb

@@ -35,4 +35,20 @@ def subject(content_item)
       expect(described_class.new({}, places).places).to eq(expected)
     end
   end
+
+  describe "#preposition" do
+    it "returns near if there are no places" do
+      expect(described_class.new({}, []).preposition).to eq("near")
+    end
+
+    it "returns near if places are addresses" do
+      places = [{ "address1" => "44", "address2" => "Foo Foo Forest", "url" => "http://www.example.com/foo_foo_foorentinafoo" }]
+      expect(described_class.new({}, places).preposition).to eq("near")
+    end
+
+    it "returns far if places are counties" do
+      places = [{ "gss" => "123" }]
+      expect(described_class.new({}, places).preposition).to eq("for")
+    end
+  end
 end