diff --git a/app/models/activity/document_activity_provider.rb b/app/models/activity/document_activity_provider.rb index a8f5dd3d61..dc02868e95 100644 --- a/app/models/activity/document_activity_provider.rb +++ b/app/models/activity/document_activity_provider.rb @@ -49,11 +49,11 @@ class Activity::DocumentActivityProvider < Activity::BaseActivityProvider end def event_path(event, activity) - url_helpers.project_document_path(url_helper_parameter(event)) + url_helpers.project_documents_url(url_helper_parameter(event)) end def event_url(event, activity) - url_helpers.project_document_url(url_helper_parameter(event)) + url_helpers.project_documents_url(url_helper_parameter(event)) end private diff --git a/app/models/document.rb b/app/models/document.rb index c73dca1bf4..10ba31222e 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -50,9 +50,17 @@ class Document < ActiveRecord::Base validates_presence_of :project, :title, :category validates_length_of :title, :maximum => 60 - scope :visible, lambda {|*args| { :include => :project, - :conditions => Project.allowed_to_condition(args.first || User.current, :view_documents) } } - scope :with_attachments, includes(:attachments).where("attachments.container_id is not NULL" ) + scope :visible, lambda { + includes(:project) + .where(Project.allowed_to_condition(User.current, :view_documents)) + .references(:projects) + } + + scope :with_attachments, lambda { + includes(:attachments) + .where('attachments.container_id is not NULL') + .references(:attachments) + } after_initialize :set_default_category diff --git a/app/models/document_observer.rb b/app/models/document_observer.rb index 1678a465e9..97b05a3c1c 100644 --- a/app/models/document_observer.rb +++ b/app/models/document_observer.rb @@ -34,11 +34,9 @@ class DocumentObserver < ActiveRecord::Observer def after_create(document) + return unless Setting.notified_events.include?('document_added') - return unless Notifier.notify?(:document_added) - - users = User.find_all_by_mails(document.recipients) - users.each do |user| + document.recipients.each do |user| DocumentsMailer.document_added(user, document).deliver end end diff --git a/app/views/documents/_document.html.erb b/app/views/documents/_document.html.erb index d67ce92637..54226e0e07 100644 --- a/app/views/documents/_document.html.erb +++ b/app/views/documents/_document.html.erb @@ -34,5 +34,5 @@ See doc/COPYRIGHT.rdoc for more details.

<%= format_time(document.updated_on) %>

- <%= textilizable(truncate_lines(document.description)) %> + <%= format_text(truncate_lines(document.description)) %>
diff --git a/app/views/documents/edit.html.erb b/app/views/documents/edit.html.erb index b386fb907e..5cf794b51d 100644 --- a/app/views/documents/edit.html.erb +++ b/app/views/documents/edit.html.erb @@ -32,7 +32,7 @@ See doc/COPYRIGHT.rdoc for more details. <%= toolbar title: Document.model_name.human %> -<%= labelled_tabular_form_for @document, url: project_document_path(@document) do |f| -%> +<%= labelled_tabular_form_for @document, url: document_path(@document), method: 'PATCH' do |f| -%> <%= render partial: "documents/form", locals: { f: f } %> <%= styled_button_tag l(:button_save), class: "-highlight -with-icon icon-yes" %> <% end %> diff --git a/app/views/documents_mailer/document_added.html.erb b/app/views/documents_mailer/document_added.html.erb index c7468c5184..89c8f349de 100644 --- a/app/views/documents_mailer/document_added.html.erb +++ b/app/views/documents_mailer/document_added.html.erb @@ -30,6 +30,6 @@ See doc/COPYRIGHT.rdoc for more details. ++#%> -

<%= link_to(@document.title, project_document_url(@document)) %> (<%= @document.category.name %>)

+

<%= link_to(@document.title, project_documents_url(@document)) %> (<%= @document.category.name %>)

-<%= textilizable @document.description %> +<%= format_text @document.description %> diff --git a/app/views/documents_mailer/document_added.text.erb b/app/views/documents_mailer/document_added.text.erb index cb857ecde0..dfe6d94c69 100644 --- a/app/views/documents_mailer/document_added.text.erb +++ b/app/views/documents_mailer/document_added.text.erb @@ -31,6 +31,6 @@ See doc/COPYRIGHT.rdoc for more details. ++#%> <%= @document.title %> (<%= @document.category.name %>) -<%= project_document_url(@document) %> +<%= project_documents_url(@document) %> <%= @document.description %> diff --git a/config/routes.rb b/config/routes.rb index 2f91b2d500..a82d484763 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -31,8 +31,8 @@ OpenProject::Application.routes.draw do - scope 'projects/:project_id' do - resources :documents, :shallow => true, :as => :project_documents + resources :projects do + resources :documents, shallow: true end end diff --git a/spec/application_helper_spec.rb b/spec/application_helper_spec.rb index bd16b06b4b..7a725663dd 100644 --- a/spec/application_helper_spec.rb +++ b/spec/application_helper_spec.rb @@ -101,13 +101,13 @@ describe ApplicationHelper do context "By id and given project" do subject { format_text("#{identifier}:document##{document.id}", :project => the_other_project) } - it { is_expected.to eq("

Test document

") } + it { is_expected.to eq("

Test document

") } end context "By name and given project" do subject { format_text("#{identifier}:document:\"#{document.title}\"", :project => the_other_project) } - it { is_expected.to eq("

Test document

") } + it { is_expected.to eq("

Test document

") } end context "Invalid link" do diff --git a/spec/controllers/documents_controller_spec.rb b/spec/controllers/documents_controller_spec.rb index 6b6f40253f..f0af7c3c3c 100644 --- a/spec/controllers/documents_controller_spec.rb +++ b/spec/controllers/documents_controller_spec.rb @@ -165,7 +165,7 @@ LOREM }.to change{Document.count}.by -1 expect(response).to redirect_to "/projects/#{project.identifier}/documents" - expect{Document.find(document.id)}.to raise_error + expect{Document.find(document.id)}.to raise_error ActiveRecord::RecordNotFound end end diff --git a/spec/models/document_observer_spec.rb b/spec/models/document_observer_spec.rb index 3004e74e66..9dcaf502c5 100644 --- a/spec/models/document_observer_spec.rb +++ b/spec/models/document_observer_spec.rb @@ -53,10 +53,9 @@ describe DocumentObserver do it "calls the DocumentsMailer, when a new document has been added" do document = FactoryGirl.build(:document) # make sure, that we have actually someone to notify - allow(document).to receive(:recipients).and_return(user.mail) + allow(document).to receive(:recipients).and_return([user]) # ... and notifies are actually sent out - allow(Notifier).to receive(:notify?).and_return(true) - + Setting.notified_events = Setting.notified_events << 'document_added' expect(DocumentsMailer).to receive(:document_added).and_return(mail) document.save diff --git a/spec/models/document_spec.rb b/spec/models/document_spec.rb index a8447b8c9b..e8f1eb63ea 100644 --- a/spec/models/document_spec.rb +++ b/spec/models/document_spec.rb @@ -57,8 +57,8 @@ describe Document do end it "should send out email-notifications" do - allow(valid_document).to receive(:recipients).and_return([user.mail]) - allow(Notifier).to receive(:notify?).with(:document_added).and_return(true) + allow(valid_document).to receive(:recipients).and_return([user]) + Setting.notified_events = Setting.notified_events << 'document_added' expect{ valid_document.save @@ -72,7 +72,7 @@ describe Document do expect(document.recipients).not_to be_empty expect(document.recipients.count).to eql 1 - expect(document.recipients).to include admin.mail + expect(document.recipients.map(&:mail)).to include admin.mail end it "should set a default-category, if none is given" do