Merge pull request alphagov#941 from alphagov/update-for-new-constitu…

…encies Support the new constituencies for the July 2024 election
unboxed · Jun 4, 2024 · 956a853 · 956a853
2 parents fd00e07 + f27f287
commit 956a853
Show file tree

Hide file tree

Showing 19 changed files with 749 additions and 55 deletions.
diff --git a/app/controllers/local_petitions_controller.rb b/app/controllers/local_petitions_controller.rb
@@ -46,11 +46,11 @@ def postcode?
   end
 
   def find_by_postcode
-    @constituency = Constituency.find_by_postcode(@postcode)
+    @constituency = Constituency.current.find_by_postcode(@postcode)
   end
 
   def find_by_slug
-    @constituency = Constituency.find_by_slug!(params[:id])
+    @constituency = Constituency.current.find_by_slug!(params[:id])
   end
 
   def constituency?

diff --git a/app/jobs/fetch_constituencies_job.rb b/app/jobs/fetch_constituencies_job.rb
@@ -4,13 +4,17 @@ class FetchConstituenciesJob < ApplicationJob
   end
 
   def perform
-    constituencies.each do |external_id, name, ons_code|
+    constituencies.each do |external_id, name, ons_code, start_date, end_date|
       begin
+        retried = false
+
         Constituency.for(external_id) do |constituency|
           constituency.name = name
           constituency.ons_code = ons_code
-          constituency.example_postcode = example_postcodes[ons_code]
-          constituency.region_id = regions[external_id]
+          constituency.example_postcode = example_postcodes.fetch(ons_code)
+          constituency.region_id = regions.fetch(external_id)
+          constituency.start_date = start_date
+          constituency.end_date = end_date
 
           if mp = mps[external_id]
             constituency.mp_id = mp.id
@@ -27,7 +31,8 @@ def perform
           constituency.save!
         end
       rescue ActiveRecord::RecordNotUnique => e
-        retry
+        retry unless retried
+        retried = true
       end
     end
   end

diff --git a/app/lib/feed.rb b/app/lib/feed.rb
@@ -6,7 +6,7 @@ class Base
 
     with_options instance_writer: false do
       class_attribute :host, :path, :model
-      class_attribute :columns, :filter
+      class_attribute :columns, :filter, :orderby
       class_attribute :open_timeout, :timeout
       class_attribute :xpath, :klass
     end
@@ -22,7 +22,7 @@ def url
     end
 
     def endpoint
-      "#{path}/#{model}?$select=#{columns}&$filter=#{filter}"
+      "#{path}/#{model}?$select=#{columns}&$filter=#{escape(filter)}&$orderby=#{orderby||columns}"
     end
 
     def each(&block)
@@ -39,6 +39,10 @@ def size
 
     private
 
+    def escape(filter)
+      CGI.escape(filter)
+    end
+
     def entries
       @entries ||= fetch_entries
     end

diff --git a/app/lib/feed/constituencies.rb b/app/lib/feed/constituencies.rb
@@ -4,11 +4,14 @@ class Constituency < Entry
       attribute :id, :string, ".//d:Constituency_Id"
       attribute :name, :string, ".//d:Name"
       attribute :ons_code, :string, ".//d:ONSCode"
+      attribute :start_date, :date, ".//d:StartDate"
+      attribute :end_date, :date, ".//d:EndDate"
     end
 
     self.model   = "Constituencies"
-    self.columns = "Constituency_Id,Name,ONSCode"
-    self.filter  = "EndDate%20eq%20null"
+    self.columns = "Constituency_Id,Name,ONSCode,StartDate,EndDate"
+    self.filter  = "(EndDate gt datetime'2015-05-07') or (EndDate eq null)"
+    self.orderby = "ONSCode"
     self.klass   = Constituency
   end
 end
diff --git a/app/lib/feed/constituency_regions.rb b/app/lib/feed/constituency_regions.rb
@@ -7,7 +7,7 @@ class ConstituencyRegion < Entry
 
     self.model   = "ConstituencyAreas"
     self.columns = "Area_Id,Constituency_Id"
-    self.filter  = "Area/AreaType_Id%20eq%208%20and%20Constituency/EndDate%20eq%20null"
+    self.filter  = "(Area/AreaType_Id eq 8) and ((Constituency/EndDate gt datetime'2015-05-07') or (Constituency/EndDate eq null))"
     self.klass   = ConstituencyRegion
   end
 end
diff --git a/app/lib/feed/departments.rb b/app/lib/feed/departments.rb
@@ -11,7 +11,8 @@ class Department < Entry
 
     self.model   = "Departments"
     self.columns = "Department_Id,Name,Acronym,Url,StartDate,EndDate"
-    self.filter  = "EndDate%20eq%20null%20"
+    self.filter  = "EndDate eq null"
+    self.orderby = "Department_Id"
     self.klass   = Department
   end
 end
diff --git a/app/lib/feed/members.rb b/app/lib/feed/members.rb
@@ -10,7 +10,8 @@ class Member < Entry
 
     self.model   = "Members"
     self.columns = "Member_Id,NameFullTitle,Party,MembershipFrom_Id,StartDate"
-    self.filter  = "CurrentStatusActive%20eq%20true%20and%20House_Id%20eq%201"
+    self.filter  = "(CurrentStatusActive eq true) and (House_Id eq 1)"
+    self.orderby = "Member_id"
     self.klass   = Member
   end
 end
diff --git a/app/lib/feed/regions.rb b/app/lib/feed/regions.rb
@@ -8,7 +8,8 @@ class Region < Entry
 
     self.model   = "Areas"
     self.columns = "Area_Id,Name,OnsAreaId"
-    self.filter  = "AreaType_Id%20eq%208"
+    self.filter  = "AreaType_Id eq 8"
+    self.orderby = "OnsAreaId"
     self.klass   = Region
   end
 end
diff --git a/app/models/constituency.rb b/app/models/constituency.rb
@@ -61,6 +61,10 @@ def find_by_postcode(postcode)
       end
     end
 
+    def current
+      where(end_date: nil)
+    end
+
     def english
       where(arel_table[:ons_code].matches('E%'))
     end

diff --git a/app/views/admin/archived/petitions/_petition_details.html.erb b/app/views/admin/archived/petitions/_petition_details.html.erb
@@ -8,12 +8,20 @@
   <% if @petition.anonymized? %>
     <dt>Anonymized</dt>
     <dd><%= date_time_format(@petition.anonymized_at) %></dd>
-  <% elsif @petition.creator %>
+  <% elsif creator = @petition.creator %>
     <dt>Creator</dt>
     <dd>
-      <%= @petition.creator.name %><br />
-      <%= auto_link(@petition.creator.email) %>
+      <%= creator.name %><br />
+      <%= auto_link(creator.email) %>
     </dd>
+
+    <% if constituency = creator.constituency %>
+      <dt>Constituency</dt>
+      <dd>
+        <span class="creator-constituency"><%= constituency.name %></span><br />
+        <small class="creator-constituency-region"><%= constituency.region.name %></small>
+      </dd>
+    <% end %>
   <% end %>
 
   <% if @petition.removed? %>

diff --git a/app/views/constituencies/index.json.jbuilder b/app/views/constituencies/index.json.jbuilder
@@ -5,6 +5,8 @@ json.cache! :constituencies, expires_in: 1.hour do
       json.party constituency.party
       json.constituency constituency.name
       json.ons_code constituency.ons_code
+      json.start_date constituency.start_date
+      json.end_date constituency.end_date
     end
   end
 end
diff --git a/config/brakeman.ignore b/config/brakeman.ignore
@@ -34,22 +34,45 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "SSL Verification Bypass",
+      "warning_code": 71,
+      "fingerprint": "83faaaee2d372a0a73dc703bf46452d519d79dbf3b069a5007f71392ec7d4a3e",
+      "check_name": "SSLVerify",
+      "message": "SSL certificate verification was bypassed",
+      "file": "features/support/ssl_server.rb",
+      "line": 97,
+      "link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/",
+      "code": "Net::HTTP.new(host, @port).verify_mode = OpenSSL::SSL::VERIFY_NONE",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Capybara::Server",
+        "method": "responsive?"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "cwe_id": [
+        295
+      ],
+      "note": ""
+    },
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 4,
-      "fingerprint": "07b7188ce44b7041f5729077eea749b2def4b8e62736ba248267e3c96c1ca927",
+      "fingerprint": "859022bb61c3d1af5cdb14424490f6d3970c5b7bddd3784f62efb4f01e8fe02b",
       "check_name": "LinkToHref",
       "message": "Potentially unsafe model attribute in `link_to` href",
       "file": "app/views/local_petitions/all.html.erb",
       "line": 11,
       "link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
-      "code": "link_to(Constituency.find_by_slug!(params[:id]).mp_name, Constituency.find_by_slug!(params[:id]).mp_url, :rel => \"external\")",
+      "code": "link_to(Constituency.current.find_by_slug!(params[:id]).mp_name, Constituency.current.find_by_slug!(params[:id]).mp_url, :rel => \"external\")",
       "render_path": [
         {
           "type": "controller",
           "class": "LocalPetitionsController",
           "method": "all",
-          "line": 30,
+          "line": 32,
           "file": "app/controllers/local_petitions_controller.rb",
           "rendered": {
             "name": "local_petitions/all",
@@ -61,7 +84,7 @@
         "type": "template",
         "template": "local_petitions/all"
       },
-      "user_input": "Constituency.find_by_slug!(params[:id]).mp_url",
+      "user_input": "Constituency.current.find_by_slug!(params[:id]).mp_url",
       "confidence": "Weak",
       "cwe_id": [
         79
@@ -71,19 +94,19 @@
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 4,
-      "fingerprint": "22e002a1359fd28418d81e2cadeb49195a5597840a43d97787ac79a868acb51f",
+      "fingerprint": "b44e200c1415ee4d50599d5a9854799a8de42354f84c7530d5c382a35fe2547e",
       "check_name": "LinkToHref",
       "message": "Potentially unsafe model attribute in `link_to` href",
       "file": "app/views/local_petitions/show.html.erb",
       "line": 11,
       "link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
-      "code": "link_to(Constituency.find_by_slug!(params[:id]).mp_name, Constituency.find_by_slug!(params[:id]).mp_url, :rel => \"external\")",
+      "code": "link_to(Constituency.current.find_by_slug!(params[:id]).mp_name, Constituency.current.find_by_slug!(params[:id]).mp_url, :rel => \"external\")",
       "render_path": [
         {
           "type": "controller",
           "class": "LocalPetitionsController",
           "method": "show",
-          "line": 22,
+          "line": 24,
           "file": "app/controllers/local_petitions_controller.rb",
           "rendered": {
             "name": "local_petitions/show",
@@ -95,36 +118,13 @@
         "type": "template",
         "template": "local_petitions/show"
       },
-      "user_input": "Constituency.find_by_slug!(params[:id]).mp_url",
+      "user_input": "Constituency.current.find_by_slug!(params[:id]).mp_url",
       "confidence": "Weak",
       "cwe_id": [
         79
       ],
       "note": ""
     },
-    {
-      "warning_type": "SSL Verification Bypass",
-      "warning_code": 71,
-      "fingerprint": "83faaaee2d372a0a73dc703bf46452d519d79dbf3b069a5007f71392ec7d4a3e",
-      "check_name": "SSLVerify",
-      "message": "SSL certificate verification was bypassed",
-      "file": "features/support/ssl_server.rb",
-      "line": 97,
-      "link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/",
-      "code": "Net::HTTP.new(host, @port).verify_mode = OpenSSL::SSL::VERIFY_NONE",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Capybara::Server",
-        "method": "responsive?"
-      },
-      "user_input": null,
-      "confidence": "High",
-      "cwe_id": [
-        295
-      ],
-      "note": ""
-    },
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 114,
@@ -164,6 +164,6 @@
       "note": ""
     }
   ],
-  "updated": "2024-05-10 12:37:54 +0000",
+  "updated": "2024-05-31 17:06:26 +0000",
   "brakeman_version": "6.1.2"
 }