Skip to content

Commit

Permalink
Merge pull request #8064 from ministryofjustice/CTSKF-832_250116
Browse files Browse the repository at this point in the history
Ctskf 832 - CCCD - Allow for multiple documents on a message
  • Loading branch information
VinceChiuMOJ authored Feb 6, 2025
2 parents cc80e17 + ed68d1f commit ed006ad
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 42 deletions.
6 changes: 3 additions & 3 deletions app/controllers/messages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ def create
end

def download_attachment
raise 'No attachment present on this message' unless message.attachment.attached?
raise 'No attachment present on this message' unless message.attachments.attached?

redirect_to message.attachment.blob.url(disposition: 'attachment'), allow_other_host: true
redirect_to message.attachments.first.blob.url(disposition: 'attachment'), allow_other_host: true
end

private
Expand All @@ -59,7 +59,7 @@ def message_params
params.require(:message).permit(
:sender_id,
:claim_id,
:attachment,
:attachments,
:body,
:claim_action,
:written_reasons_submitted
Expand Down
10 changes: 5 additions & 5 deletions app/interfaces/api/entities/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@ class Document < BaseEntity

private

def attachment
object.is_a?(ActiveStorage::Attachment) ? object : object.attachment
def attachments
object.is_a?(ActiveStorage::Attachment) ? object : object.attachments
end

def url
attachment.blob.url(disposition: 'attachment') if attachment.attached?
attachments.first.blob.url(disposition: 'attachment') if attachments.attached?
end

def file_name
attachment.filename if attachment.attached?
attachments.first.filename if attachments.attached?
end

def size
attachment.byte_size if attachment.attached?
attachments.first.byte_size if attachments.attached?
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/interfaces/api/entities/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ class Message < BaseEntity
expose :created_at, format_with: :utc
expose :sender_uuid
expose :body
expose :attachment,
expose :attachments,
as: :document,
using: API::Entities::Document,
if: ->(instance, _opts) { instance.attachment.present? }
if: ->(instance, _opts) { instance.attachments.present? }

private

Expand Down
14 changes: 3 additions & 11 deletions app/models/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ class Message < ApplicationRecord

attr_accessor :claim_action, :written_reasons_submitted

has_one_attached :attachment
has_many_attached :attachments

validates :attachment,
validates :attachments,
size: { less_than: 20.megabytes },
content_type: %w[
application/pdf
Expand All @@ -49,9 +48,8 @@ class Message < ApplicationRecord

scope :most_recent_last, -> { includes(:user_message_statuses).order(created_at: :asc) }

after_create :generate_statuses, :process_claim_action, :process_written_reasons, :send_email_if_required,
:duplicate_message_attachment
before_destroy -> { attachment.purge }
after_create :generate_statuses, :process_claim_action, :process_written_reasons, :send_email_if_required
before_destroy -> { attachments.purge }

class << self
def for(object)
Expand Down Expand Up @@ -109,10 +107,4 @@ def process_written_reasons
def claim_updater
Claims::ExternalUserClaimUpdater.new(claim, current_user: sender)
end

def duplicate_message_attachment
return unless attachment.attached?

attachments.attach(attachment.blob)
end
end
6 changes: 3 additions & 3 deletions app/presenters/message_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def sender_is_a?(klass)
def body
h.tag.div do
h.concat(simple_format(message.body))
attachment_field if message.attachment.present?
attachment_field if message.attachments.present?
end
end

Expand Down Expand Up @@ -39,11 +39,11 @@ def download_file_link
end

def attachment_file_name
message.attachment.filename.to_s
message.attachments.first.filename.to_s
end

def attachment_file_size
h.number_to_human_size(message.attachment.byte_size)
h.number_to_human_size(message.attachments.first.byte_size)
end

def hide_author?
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_message_controls.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
link_errors: true,
label: { text: t('.written_reasons') }

= f.govuk_file_field :attachment,
= f.govuk_file_field :attachments,
label: { text: t('.attachment_label') },
hint: { text: t('.accepted_files_help_text') }

Expand Down
3 changes: 2 additions & 1 deletion features/step_definitions/messaging_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
When(/^I upload a file$/) do
available_docs = Dir.glob "#{Rails.root}/spec/fixtures/files/*.pdf"
@uploaded_file_path = available_docs.first
@external_user_claim_show_page.messages_panel.upload_file(@uploaded_file_path)
file_field = page.find('input[type="file"]')
file_field.attach_file(@uploaded_file_path)
end

When(/^I click send$/) do
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/messages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@
let(:test_url) { 'https://document.storage/attachment.doc#123abc' }

before do
message.attachment.attach(io: StringIO.new, filename: 'attachment.doc')
message.attachments.attach(io: StringIO.new, filename: 'attachment.doc')
allow(Message).to receive(:find).with(message[:id].to_s).and_return(message)
allow(message.attachment.blob).to receive(:url).and_return(test_url)
allow(message.attachments.first.blob).to receive(:url).and_return(test_url)
end

it { is_expected.to redirect_to test_url }
Expand Down
12 changes: 7 additions & 5 deletions spec/factories/messages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@
end

trait :with_attachment do
attachment do
Rack::Test::UploadedFile.new(
File.expand_path('features/examples/shorter_lorem.docx', Rails.root),
'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
)
attachments do
[
Rack::Test::UploadedFile.new(
File.expand_path('features/examples/shorter_lorem.docx', Rails.root),
'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
)
]
end
end
end
9 changes: 4 additions & 5 deletions spec/models/message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
it { is_expected.to validate_presence_of(:claim_id).with_message('Message claim_id cannot be blank') }
it { is_expected.to validate_presence_of(:body).with_message('Message body cannot be blank') }

it { is_expected.to have_one_attached(:attachment) }
it { is_expected.to have_many_attached(:attachments) }

it do
is_expected.to validate_content_type_of(:attachment)
is_expected.to validate_content_type_of(:attachments)
.allowing(
'application/pdf',
'application/msword',
Expand All @@ -45,7 +45,7 @@
).rejecting('text/plain', 'text/html')
end

it { is_expected.to validate_size_of(:attachment).less_than_or_equal_to(20.megabytes) }
it { is_expected.to validate_size_of(:attachments).less_than_or_equal_to(20.megabytes) }

describe '.for' do
let(:message) { create(:message) }
Expand Down Expand Up @@ -215,8 +215,7 @@
context 'with an attachment' do
let(:trait) { :with_attachment }

it { expect { destroy_message }.to change(ActiveStorage::Attachment, :count).by(-2) }
it { expect { destroy_message }.to change(ActiveStorage::Blob, :count).by(-1) }
it { expect { destroy_message }.to change(ActiveStorage::Attachment, :count).by(-message.attachments.count) }
end
end
end
8 changes: 4 additions & 4 deletions spec/presenters/message_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
RSpec.describe MessagePresenter, type: :helper do
subject(:presenter) { described_class.new message, helper }

let(:attachment) { nil }
let(:message) { build(:message, attachment:) }
let(:attachments) { nil }
let(:message) { build(:message, attachments: [attachments]) }

describe '#body' do
context 'without an attachment' do
Expand All @@ -16,10 +16,10 @@
context 'with an attachment' do
let(:file) { File.expand_path('features/examples/shorter_lorem.docx', Rails.root) }
let(:file_size) { number_to_human_size(File.size(file)) }
let(:attachment) { Rack::Test::UploadedFile.new(file) }
let(:attachments) { Rack::Test::UploadedFile.new(file) }

before do
allow(message.attachment).to receive(:url).and_return('http://example.com')
allow(message.attachments.first).to receive(:url).and_return('http://example.com')
end

it 'includes a download link to the attachment' do
Expand Down

0 comments on commit ed006ad

Please sign in to comment.