From 69b3e3c73cff705c93dadadbf6c82750c9625a21 Mon Sep 17 00:00:00 2001 From: Damir Sultanbekov Date: Thu, 16 May 2024 02:00:37 +0500 Subject: [PATCH] Added method `Person.manifestations` to return all works for person including unpublished, and refactored other methods in `Person` class to utilize it and `published_manifestations`. --- app/controllers/authors_controller.rb | 4 +- .../record_with_involved_authorities.rb | 4 + app/models/person.rb | 88 ++++------- spec/models/person_spec.rb | 148 +++++++++++++++++- 4 files changed, 178 insertions(+), 66 deletions(-) diff --git a/app/controllers/authors_controller.rb b/app/controllers/authors_controller.rb index 5fc6c505..7cf53754 100644 --- a/app/controllers/authors_controller.rb +++ b/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? diff --git a/app/models/concerns/record_with_involved_authorities.rb b/app/models/concerns/record_with_involved_authorities.rb index c96c0595..3de39861 100644 --- a/app/models/concerns/record_with_involved_authorities.rb +++ b/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) diff --git a/app/models/person.rb b/app/models/person.rb index b982a0f0..e0ec3adb 100644 --- a/app/models/person.rb +++ b/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,13 +228,15 @@ 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 @@ -235,35 +244,15 @@ def all_languages 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 diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index b04f46fb..8957d964 100644 --- a/spec/models/person_spec.rb +++ b/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