Merge pull request #15603 from everypolitician/simplify-personcard

Simplify the instantiation of PersonCard objects

Gemfile.lock

@@ -1,8 +1,9 @@
 GIT
   remote: https://github.com/everypolitician/everypolitician-popolo.git
-  revision: 4e6e75ddfece32889243595e7305ba9007e3c8b4
+  revision: 40dc460d8340df9ae78469b755ce73b4fd074ce3
   specs:
-    everypolitician-popolo (0.5.0)
+    everypolitician-popolo (0.8.0)
+      require_all
 
 GIT
   remote: https://github.com/everypolitician/everypolitician-ruby.git

lib/everypolitician_extensions.rb

@@ -37,6 +37,21 @@ def memberships_at_end
         mem.end_date.to_s.empty? || mem.end_date == end_date
       end
     end
+
+    def people
+      @people ||= memberships.map(&:person).uniq(&:id)
+    end
+
+    def top_identifiers
+      @top_identifiers ||= people
+                           .map(&:identifiers)
+                           .compact
+                           .flatten
+                           .reject { |i| i[:scheme] == 'everypolitician_legacy' }
+                           .group_by { |i| i[:scheme] }
+                           .sort_by { |s, ids| [-ids.size, s] }
+                           .map { |s, _ids| s }
+    end
   end
 
   module MembershipExtension

lib/page/term_table.rb

@@ -56,12 +56,10 @@ def group_data
     end
 
     def people
-      @people ||= people_for_current_term.sort_by { |e| [e.sort_name, e.name] }.map do |person|
+      @people ||= term.people.sort_by { |e| [e.sort_name, e.name] }.map do |person|
         PersonCard.new(
-          person:          person,
-          proxy_image:     image_proxy_url(person.id),
-          memberships:     person_memberships(person),
-          top_identifiers: top_identifiers
+          person: person,
+          term:   term
         )
       end
     end
@@ -81,49 +79,13 @@ def popolo
       @popolo ||= house.popolo
     end
 
-    def top_identifiers
-      @tidx ||= people_for_current_term
-                .map(&:identifiers)
-                .compact
-                .flatten
-                .reject { |i| i[:scheme] == 'everypolitician_legacy' }
-                .group_by { |i| i[:scheme] }
-                .sort_by { |s, ids| [-ids.size, s] }
-                .map { |s, _ids| s }
-    end
-
-    def person_memberships(person)
-      membership_lookup[person.id]
-    end
-
-    def image_proxy_url(id)
-      'https://mysociety.github.io/politician-image-proxy' \
-      "/#{country.slug}/#{house.slug}/#{id}/140x140.jpeg"
-    end
-
     # Caches for faster lookup
-    def membership_lookup
-      @membership_lookup ||= current_term_memberships.group_by(&:person_id)
-    end
-
     def area_lookup
       @area_lookup ||= popolo.areas.group_by(&:id)
     end
 
     def org_lookup
       @org_lookup ||= popolo.organizations.group_by(&:id)
     end
-
-    def current_term_memberships
-      @ctm ||= term.memberships
-    end
-
-    def current_term_people_ids
-      @ctpids ||= Set.new(current_term_memberships.map(&:person_id))
-    end
-
-    def people_for_current_term
-      @pct ||= popolo.persons.select { |p| current_term_people_ids.include?(p.id) }
-    end
   end
 end

lib/person_cards.rb

@@ -1,14 +1,14 @@
 # frozen_string_literal: true
 
 class PersonCard
-  attr_reader :proxy_image, :memberships
-
-  # TODO: pass fewer arguments!
-  def initialize(person:, proxy_image:, memberships:, top_identifiers:)
+  def initialize(person:, term:)
     @person          = person
-    @memberships     = memberships # should be able to get from Person
-    @proxy_image     = proxy_image # get from Legislature
-    @top_identifiers = top_identifiers # get from Legislature
+    @term            = term
+  end
+
+  def proxy_image
+    'https://mysociety.github.io/politician-image-proxy' \
+    "/#{legislature.country.slug}/#{legislature.slug}/#{id}/140x140.jpeg"
   end
 
   def id
@@ -39,9 +39,21 @@ def identifiers
     Section::Identifiers.new(person, top_identifiers: top_identifiers).data
   end
 
+  def memberships
+    person.memberships.where(legislative_period_id: term.id)
+  end
+
   private
 
-  attr_reader :person, :top_identifiers
+  attr_reader :person, :term
+
+  def top_identifiers
+    term.top_identifiers
+  end
+
+  def legislature
+    term.legislature
+  end
 
   class Section
     CardLine = Struct.new(:type, :value, :link)