Added method Person.manifestations to return all works for person i…

…ncluding unpublished, and refactored other methods in `Person` class to utilize it and `published_manifestations`.

app/controllers/authors_controller.rb

@@ -396,8 +396,8 @@ def show
     @author = Person.find(params[:id])
     @published_works = @author.original_works.count
     @published_xlats = @author.translations.count
-    @total_orig_works = @author.original_work_count_including_unpublished
-    @total_xlats = @author.translations_count_including_unpublished
+    @total_orig_works = @author.manifestations(:author).count
+    @total_xlats = @author.manifestations(:translator).count
     @aboutnesses = @author.aboutnesses
 
     if @author.nil?

app/models/concerns/record_with_involved_authorities.rb

@@ -4,6 +4,10 @@
 module RecordWithInvolvedAuthorities
   extend ActiveSupport::Concern
 
+  # Returns list of authorities involved into given object with the given role.
+  # Note: some roles can be specified both on Work and Expression level and this method will fetch only part of them.
+  # If you need a full list use {#Manifestation.involved_authorities_by_role}
+  # @param role [String/Symbol] role code
   def involved_authorities_by_role(role)
     role = role.to_s
     raise "Unknown role #{role}" unless InvolvedAuthority.roles.keys.include?(role)

app/models/person.rb

@@ -122,19 +122,26 @@ def works_since(since, maxitems)
     end
   end
 
-  def published_manifestations(role = nil)
+  # @param roles [String / Symbol] optional, if provided will only return Manifestations where person has
+  #   one of the given roles.
+  # @return relation representing [Manifestation] objects current person is involved into.
+  def manifestations(*roles)
     rel = involved_authorities
-    rel = rel.where(role: role) if role.present?
+    rel = rel.where(role: roles.to_a) if roles.present?
     ids = rel.pluck(:work_id, :expression_id)
 
     work_ids = ids.map(&:first).compact.uniq
     expression_ids = ids.map(&:last).compact.uniq
 
-    Manifestation.all_published
-                 .joins(:expression)
+    Manifestation.joins(:expression)
                  .where('expressions.work_id in (?) or expressions.id in (?)', work_ids, expression_ids)
   end
 
+  # Works like {#manifestaions} method, but returns only published manifestations
+  def published_manifestations(*roles)
+    manifestations(*roles).all_published
+  end
+
   def cached_works_count
     Rails.cache.fetch("au_#{self.id}_work_count", expires_in: 24.hours) do
       published_manifestations.count
@@ -221,49 +228,31 @@ def has_comment?
   end
 
   def all_genres
-    works_genres = original_works.select('works.genre').distinct.pluck(:genre)
-    translation_genres = translations.select('works.genre').distinct.pluck(:genre)
-    return (works_genres + translation_genres).uniq.sort
+    published_manifestations(:author, :translator).joins(expression: :work)
+                                                  .select('works.genre')
+                                                  .distinct
+                                                  .pluck(:genre)
+                                                  .sort
   end
 
   def all_languages
-    work_langs = original_works.pluck('works.orig_lang')
+    work_langs = original_works.joins(expression: :work).pluck('works.orig_lang')
     #translation_langs = translations.pluck('works.orig_lang')
     #all_languages = work_langs + translation_langs
     #return all_languages.uniq
     return work_langs.uniq
   end
 
   def original_works
-    Manifestation.all_published
-                 .joins(expression: [work: :involved_authorities])
-                 .where(involved_authorities: { person_id: id })
+    published_manifestations(:author)
   end
 
   def translations
-    Manifestation.all_published
-                 .joins(expression: %i(work involved_authorities))
-                 .merge(InvolvedAuthority.role_translator)
-                 .where(involved_authorities: { person_id: id })
+    published_manifestations(:translator)
   end
 
   def all_works_including_unpublished
-    works = Manifestation.joins(expression: [work: :involved_authorities])
-                         .merge(InvolvedAuthority.where(person_id: id))
-    xlats = Manifestation.joins(expression: :involved_authorities)
-                         .merge(InvolvedAuthority.role_translator.where(person_id: id))
-    (works + xlats).uniq.sort_by(&:sort_title)
-  end
-
-  def original_work_count_including_unpublished
-    Manifestation.joins(expression: { work: :involved_authorities })
-                 .merge(InvolvedAuthority.where(person_id: id)).count
-  end
-
-  def translations_count_including_unpublished
-    Manifestation.joins(expression: :involved_authorities)
-                 .merge(InvolvedAuthority.role_translator.where(person_id: id))
-                 .count
+    manifestations(:author, :translator).sort_by(&:sort_title)
   end
 
   def all_works_title_sorted
@@ -283,27 +272,17 @@ def all_works_by_title(term)
   def title # convenience method for polymorphic handling (e.g. Taggable)
     name
   end
+
   def original_works_by_genre
-    ret = {}
-    get_genres.map{|g| ret[g] = []}
-    Manifestation.all_published
-                 .joins(expression: [work: :involved_authorities])
-                 .includes(:expression)
-                 .merge(InvolvedAuthority.where(person_id: id)).each do |m|
-      ret[m.expression.work.genre] << m
-    end
-    return ret
+    hash = published_manifestations(:author).preload(expression: :work)
+                                            .group_by { |m| m.expression.work.genre }
+    Work::GENRES.index_with { |genre| hash[genre] || [] }
   end
 
   def translations_by_genre
-    ret = {}
-    get_genres.map{|g| ret[g] = []}
-    Manifestation.all_published
-                 .joins(expression: :involved_authorities)
-                 .merge(InvolvedAuthority.role_translator.where(person_id: id)).each do |m|
-      ret[m.expression.work.genre] << m
-    end
-    return ret
+    hash = published_manifestations(:translator).preload(expression: :work)
+                                                .group_by { |m| m.expression.work.genre }
+    Work::GENRES.index_with { |genre| hash[genre] || [] }
   end
 
   def featured_work
@@ -313,18 +292,7 @@ def featured_work
   end
 
   def latest_stuff
-    latest_original_works = Manifestation.all_published
-                                         .joins(expression: [work: :involved_authorities])
-                                         .where(involved_authorities: { person_id: id })
-                                         .order(created_at: :desc).limit(20)
-
-    latest_translations = Manifestation.all_published
-                                       .joins(expression: [:involved_authorities])
-                                       .merge(InvolvedAuthority.role_translator)
-                                       .where(involved_authorities: { person_id: id })
-                                       .order(created_at: :desc).limit(20)
-
-    (latest_original_works + latest_translations).sort_by(&:created_at).reverse.first(20)
+    published_manifestations(:author, :translator).order(created_at: :desc).limit(20).to_a
   end
 
   def cached_latest_stuff

spec/models/person_spec.rb

@@ -22,13 +22,13 @@
       before do
         create(:manifestation, author: person, genre: 'poetry')
         create(:manifestation, author: person, genre: 'poetry')
-        create(:manifestation, illustrator: person, genre: 'fables')
+        create(:manifestation, illustrator: person, genre: 'fables') # illustrated works should not be included
         create(:manifestation, translator: person, orig_lang: 'ru', genre: 'article')
         create(:manifestation, translator: person, orig_lang: 'ru', genre: 'memoir')
         create(:manifestation, editor: person, genre: 'prose') # edited works should not be included
       end
 
-      it { is_expected.to eq %w(article fables memoir poetry) }
+      it { is_expected.to eq %w(article memoir poetry) }
     end
 
     describe '.most_read' do
@@ -119,7 +119,7 @@
       subject(:latest_stuff) { person.latest_stuff }
 
       it 'returns latest original and translated works' do
-        expect(latest_stuff).to match_array [original_work, translated_work]
+        expect(latest_stuff).to contain_exactly(original_work, translated_work)
       end
 
       context 'when more than 20 records present' do
@@ -170,7 +170,147 @@
         create_list(:manifestation, 2, author: person)
       end
 
-      it { is_expected.to match_array [original_work, translated_work, original_and_translated_work] }
+      it { is_expected.to contain_exactly(original_work, translated_work, original_and_translated_work) }
+    end
+
+    describe '.manifestations' do
+      let!(:as_author) { create(:manifestation, author: person) }
+      let!(:as_translator) { create(:manifestation, translator: person, orig_lang: 'en') }
+      let!(:as_author_and_illustrator) { create(:manifestation, author: person, illustrator: person) }
+      let!(:as_author_unpublished) { create(:manifestation, author: person, status: :unpublished) }
+      let!(:as_illustrator_on_expression_level) do
+        create(:manifestation).tap do |manifestation|
+          manifestation.expression.involved_authorities.create!(role: :illustrator, person: person)
+        end
+      end
+      let!(:as_illustrator_on_both_levels) do
+        create(:manifestation, illustrator: person).tap do |manifestation|
+          manifestation.expression.involved_authorities.create!(role: :illustrator, person: person)
+        end
+      end
+
+      before do
+        create_list(:manifestation, 5)
+      end
+
+      context 'when single role is passed' do
+        it 'returns all manifestations where person has given role, including unpublished' do
+          expect(person.manifestations(:author)).to contain_exactly(
+            as_author,
+            as_author_unpublished,
+            as_author_and_illustrator
+          )
+          expect(person.manifestations(:translator)).to eq [as_translator]
+          expect(person.manifestations(:illustrator)).to contain_exactly(
+            as_author_and_illustrator,
+            as_illustrator_on_expression_level,
+            as_illustrator_on_both_levels
+          )
+          expect(person.manifestations(:editor)).to be_empty
+        end
+      end
+
+      context 'when several roles are passed' do
+        it 'returns all manifestations where person has given role, including unpublished' do
+          expect(person.manifestations(:author, :editor)).to contain_exactly(
+            as_author,
+            as_author_unpublished,
+            as_author_and_illustrator
+          )
+
+          expect(person.manifestations(:author, :translator)).to contain_exactly(
+             as_author,
+             as_author_unpublished,
+             as_author_and_illustrator,
+             as_translator
+           )
+
+          expect(person.manifestations(:translator, :illustrator)).to contain_exactly(
+            as_translator,
+            as_author_and_illustrator,
+            as_illustrator_on_expression_level,
+            as_illustrator_on_both_levels
+          )
+
+          expect(person.manifestations(:editor, :other)).to be_empty
+        end
+      end
+
+      context 'when no roles are passed' do
+        it 'returns all manifestations where person has any role, including unpublished' do
+          expect(person.manifestations).to contain_exactly(
+             as_author,
+             as_author_unpublished,
+             as_author_and_illustrator,
+             as_translator,
+             as_illustrator_on_expression_level,
+             as_illustrator_on_both_levels
+           )
+        end
+      end
+    end
+
+    describe '.published_manifestations' do
+      let!(:as_author) { create(:manifestation, author: person) }
+      let!(:as_translator) { create(:manifestation, translator: person, orig_lang: 'en') }
+      let!(:as_author_and_illustrator) { create(:manifestation, author: person, illustrator: person) }
+      let!(:as_author_unpublished) { create(:manifestation, author: person, status: :unpublished) }
+
+      before do
+        create_list(:manifestation, 5)
+      end
+
+      it 'works correctly and ignores unpublished works' do
+        expect(person.published_manifestations).to contain_exactly(as_author, as_translator, as_author_and_illustrator)
+        expect(person.published_manifestations(:author)).to contain_exactly(as_author, as_author_and_illustrator)
+        expect(person.published_manifestations(:translator, :illustrator)).to contain_exactly(
+          as_translator, as_author_and_illustrator
+        )
+        expect(person.published_manifestations(:editor)).to be_empty
+      end
+    end
+
+    describe '.original_works_by_genre' do
+      subject { person.original_works_by_genre }
+
+      let!(:fables) { create_list(:manifestation, 5, genre: :fables, author: person) }
+      let!(:poetry) { create_list(:manifestation, 2, genre: :poetry, author: person) }
+
+      it 'works correctly' do
+        is_expected.to eq ({
+          'article' => [],
+          'drama' => [],
+          'fables' => fables,
+          'letters' => [],
+          'lexicon' => [],
+          'memoir' => [],
+          'poetry' => poetry,
+          'prose' => [],
+          'reference' => []
+        })
+      end
+    end
+
+    describe '.translations_by_genre' do
+      subject { person.translations_by_genre }
+
+      let!(:memoirs) { create_list(:manifestation, 5, genre: :memoir, orig_lang: :ru,  translator: person) }
+      let!(:poetry) { create_list(:manifestation, 2, genre: :poetry, orig_lang: :en, translator: person) }
+      let!(:articles) { create_list(:manifestation, 3, genre: :article, orig_lang: :de, translator: person) }
+
+      it 'works correctly' do
+        is_expected.to eq ({
+          'article' => articles,
+          'drama' => [],
+          'fables' => [],
+          'letters' => [],
+          'lexicon' => [],
+          'memoir' => memoirs,
+          'poetry' => poetry,
+          'prose' => [],
+          'reference' => []
+        })
+      end
     end
   end
 end