diff --git a/app/controllers/authors_controller.rb b/app/controllers/authors_controller.rb index 5fc6c505..5f15e3ed 100644 --- a/app/controllers/authors_controller.rb +++ b/app/controllers/authors_controller.rb @@ -353,6 +353,23 @@ def to_manual_toc end end + def show + @author = Person.find(params[:id]) + @published_works = @author.original_works.count + @published_xlats = @author.translations.count + @total_orig_works = @author.manifestations(:author).count + @total_xlats = @author.manifestations(:translator).count + @aboutnesses = @author.aboutnesses + + if @author.nil? + flash[:error] = t(:no_such_item) + redirect_to '/' + else + # TODO: add other types of content + @page_title = "#{t(:author_details)}: #{@author.name}" + end + end + def new @person = Person.new(intellectual_property: :unknown) @page_title = t(:new_author) @@ -381,7 +398,7 @@ def create def destroy @author = Person.find(params[:id]) - Chewy.strategy(:atomic) { + Chewy.strategy(:atomic) do if @author.nil? flash[:error] = t(:no_such_item) redirect_to '/' @@ -389,23 +406,6 @@ def destroy @author.destroy! redirect_to action: :list end - } - end - - 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 - @aboutnesses = @author.aboutnesses - - if @author.nil? - flash[:error] = t(:no_such_item) - redirect_to '/' - else - # TODO: add other types of content - @page_title = t(:author_details)+': '+@author.name end end 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..56829e07 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -1,10 +1,12 @@ +# frozen_string_literal: true + require 'rails_helper' describe Person do describe 'validations' do it 'considers empty Person invalid' do p = described_class.new - expect(p).to_not be_valid + expect(p).not_to be_valid end it 'considers Person with all mandatory fields filled as valid' do @@ -22,34 +24,37 @@ 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 + subject { person.most_read(limit).pluck(:id) } + let!(:manifestation_1) { create(:manifestation, author: person, impressions_count: 10, genre: :fables) } let!(:manifestation_2) { create(:manifestation, author: person, impressions_count: 20, genre: :memoir) } let!(:manifestation_3) { create(:manifestation, author: person, impressions_count: 30, genre: :article) } - subject { person.most_read(limit).map{ |rec| rec[:id] } } - context 'when limit is less than total number of works' do let(:limit) { 2 } + it { is_expected.to eq [manifestation_3.id, manifestation_2.id] } end context 'when limit is equal to total number of works' do let(:limit) { 3 } + it { is_expected.to eq [manifestation_3.id, manifestation_2.id, manifestation_1.id] } end context 'when limit is bigger than total number of works' do let(:limit) { 4 } + it { is_expected.to eq [manifestation_3.id, manifestation_2.id, manifestation_1.id] } end end @@ -112,14 +117,14 @@ end describe '.latest_stuff' do + subject(:latest_stuff) { person.latest_stuff } + let!(:original_work) { create(:manifestation, author: person) } let!(:translated_work) { create(:manifestation, orig_lang: 'ru', translator: person) } let!(:edited_work) { create(:manifestation, orig_lang: 'ru', editor: person) } - 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 @@ -161,7 +166,8 @@ # this item should not be duplicated let!(:original_and_translated_work) do - create(:manifestation, title: "original translated #{title} work", language: 'he', orig_lang: 'ru', author: person, translator: person) + create(:manifestation, title: "original translated #{title} work", language: 'he', orig_lang: 'ru', + author: person, translator: person) end before do @@ -170,7 +176,139 @@ 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 + + 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 + ) + 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 + ) + + 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 + ) + 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(:result) { 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 + expect(result).to eq({ + 'article' => [], + 'drama' => [], + 'fables' => fables, + 'letters' => [], + 'lexicon' => [], + 'memoir' => [], + 'poetry' => poetry, + 'prose' => [], + 'reference' => [] + }) + end + end + + describe '.translations_by_genre' do + subject(:result) { 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 + expect(result).to eq({ + 'article' => articles, + 'drama' => [], + 'fables' => [], + 'letters' => [], + 'lexicon' => [], + 'memoir' => memoirs, + 'poetry' => poetry, + 'prose' => [], + 'reference' => [] + }) + end end end end