Skip to content

Commit

Permalink
fix: no attachments after research plan save
Browse files Browse the repository at this point in the history 
* fix: add Attachment Fetcher as uploader for attachments

* fix: upload attachments via AttachmentFetcher

* fix: add needed parameter for next promise step

* fix: upload attachments in genericElsFetcher only if no researchplan

* fix: create multiple attachments on researchplan create

* test: add tests for getting attachments of researchplan

* fix: removed deprecated thumbnail job

* fix: WIP map identifier to attachment

* fix: removed index from loop

* fix: add identifier to params

* refactor: use loop with index instead of own index variable

* todo: remove uploading of attachments via GenericElsFetcher and do the same for update usecase


Refs: ComPlat#1564
---------
Co-authored-by: nh9378 <[email protected]>
  • Loading branch information
@FabianMauz @adambasha0
FabianMauz and adambasha0 authored Oct 6, 2023
1 parent c1b45ae commit 3c0b1e4
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 55 deletions.
23 changes: 17 additions & 6 deletions app/api/chemotion/attachable_api.rb
View file
Original file line number Diff line number Diff line change
@@ -1,41 +1,50 @@
# frozen_string_literal: true

# rubocop:disable Rails/SkipsModelValidations

module Chemotion
class AttachableAPI < Grape::API
resource :attachable do
params do
optional :files, type: Array[File], desc: 'files', default: []
optional :attachable_type, type: String, desc: 'attachable_type'
optional :attachable_id, type: Integer, desc: 'attachable id'
optional :attfilesIdentifier, type: Array[String], desc: 'file identifier'
optional :del_files, type: Array[Integer], desc: 'del file id', default: []
end
after_validation do
case params[:attachable_type]
when 'ResearchPlan'
error!('401 Unauthorized', 401) unless ElementPolicy.new(current_user, ResearchPlan.find_by(id: params[:attachable_id])).update?
error!('401 Unauthorized', 401) unless ElementPolicy.new(
current_user,
ResearchPlan.find_by(id: params[:attachable_id]),
).update?
end
end

desc 'Update attachable records'
post 'update_attachments_attachable' do
attachable_type = params[:attachable_type]
attachable_id = params[:attachable_id]

if params.fetch(:files, []).any?
attach_ary = []
rp_attach_ary = []
params[:files].each do |file|
params[:files].each_with_index do |file, index|
next unless (tempfile = file[:tempfile])

a = Attachment.new(
identifier: params[:attfilesIdentifier][index],
bucket: file[:container_id],
filename: file[:filename],
file_path: file[:tempfile],
created_by: current_user.id,
created_for: current_user.id,
content_type: file[:type],
attachable_type: attachable_type,
attachable_id: attachable_id
attachable_id: attachable_id,
)

begin
a.save!
attach_ary.push(a.id)
Expand All @@ -45,12 +54,14 @@ class AttachableAPI < Grape::API
tempfile.unlink
end
end

TransferThumbnailToPublicJob.set(queue: "transfer_thumbnail_to_public_#{current_user.id}").perform_later(rp_attach_ary) if rp_attach_ary.any?
end
Attachment.where('id IN (?) AND attachable_type = (?)', params[:del_files].map!(&:to_i), attachable_type).update_all(attachable_id: nil) if params[:del_files].any?
if params[:del_files].any?
Attachment.where('id IN (?) AND attachable_type = (?)', params[:del_files].map!(&:to_i),
attachable_type).update_all(attachable_id: nil)
end
true
end
end
end
end
# rubocop:enable Rails/SkipsModelValidations
2 changes: 2 additions & 0 deletions app/packs/src/fetchers/AttachmentFetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,12 @@ export default class AttachmentFetcher {
static updateAttachables(files, attachableType, attachableId, dels) {
const data = new FormData();
files.forEach(file => {
data.append('attfilesIdentifier[]', file.id);
data.append('files[]', file.file, file.name);
});
data.append('attachable_type', attachableType);
data.append('attachable_id', attachableId);

dels.forEach(f => {
data.append('del_files[]', f.id);
});
Expand Down
45 changes: 27 additions & 18 deletions app/packs/src/fetchers/GenericElsFetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,11 @@ export default class GenericElsFetcher extends GenericBaseFetcher {
});
data.append('elInfo', JSON.stringify(elMap));
}
if (
hasAttach === true
&& element.attachments
&& element.attachments.length > 0
) {
const newFiles = (element.attachments || []).filter(
(a) => a.is_new && !a.is_deleted
);
const delFiles = (element.attachments || []).filter(
(a) => !a.is_new && a.is_deleted
);
(newFiles || []).forEach((file) => {
data.append('attfiles[]', file.file, file.name);
data.append('attfilesIdentifier[]', file.id);
});
(delFiles || []).forEach((f) => {
data.append('delfiles[]', f.id);
});

if (GenericElsFetcher.shouldUploadAttachments(hasAttach, element)) {
GenericElsFetcher.prepareAttachmentParam(element, data);
}

(element.segments || []).forEach((segment) => {
segMap[segment.segment_klass_id] = {
type: 'SegmentProps',
Expand Down Expand Up @@ -218,4 +204,27 @@ export default class GenericElsFetcher extends GenericBaseFetcher {
static createRepo(params) {
return this.execData(params, 'create_repo_klass');
}

static prepareAttachmentParam(element, data) {
const newFiles = (element.attachments || []).filter(
(a) => a.is_new && !a.is_deleted
);
const delFiles = (element.attachments || []).filter(
(a) => !a.is_new && a.is_deleted
);
(newFiles || []).forEach((file) => {
data.append('attfiles[]', file.file, file.name);
data.append('attfilesIdentifier[]', file.id);
});
(delFiles || []).forEach((f) => {
data.append('delfiles[]', f.id);
});
}

static shouldUploadAttachments(hasAttach, element) {
return hasAttach === true
&& element.attachments
&& element.attachments.length > 0
&& element.type !== 'research_plan';
}
}
64 changes: 38 additions & 26 deletions app/packs/src/fetchers/ResearchPlansFetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export default class ResearchPlansFetcher {

static create(researchPlan) {
researchPlan.convertTemporaryImageFieldsInBody();

const promise = fetch('/api/v1/research_plans/', {
credentials: 'same-origin',
method: 'post',
Expand All @@ -40,10 +41,18 @@ export default class ResearchPlansFetcher {
'Content-Type': 'application/json'
},
body: JSON.stringify(researchPlan.serialize())
}).then((response) => response.json()).then((json) => GenericElsFetcher.uploadGenericFiles(researchPlan, json.research_plan.id, 'ResearchPlan', true)
.then(() => this.fetchById(json.research_plan.id))).catch((errorMessage) => {
console.log(errorMessage);
});
})
.then((response) => response.json())
.then((json) => AttachmentFetcher.updateAttachables(
researchPlan.getNewAttachments(),
'ResearchPlan',
json.research_plan.id,
researchPlan.getMarkedAsDeletedAttachments()
)().then(() => GenericElsFetcher.uploadGenericFiles(researchPlan, json.research_plan.id, 'ResearchPlan', true)
.then(() => this.fetchById(json.research_plan.id))))
.catch((errorMessage) => {
console.log(errorMessage);
});
return promise;
}

Expand All @@ -59,13 +68,16 @@ export default class ResearchPlansFetcher {
'Content-Type': 'application/json'
},
body: JSON.stringify(researchPlan.serialize())
}).then( response => response.json())
.then((json) =>{ return GenericElsFetcher.uploadGenericFiles(researchPlan, json.research_plan.id, 'ResearchPlan', true)})
.then(() => {
return ResearchPlansFetcher.updateAnnotations(researchPlan) })
.then(() =>{
return this.fetchById(researchPlan.id)} )
.catch((errorMessage) => {console.log(errorMessage);});
}).then((response) => response.json())
.then((json) => AttachmentFetcher.updateAttachables(
researchPlan.getNewAttachments(),
'ResearchPlan',
json.research_plan.id,
researchPlan.getMarkedAsDeletedAttachments()
)().then(() => GenericElsFetcher.uploadGenericFiles(researchPlan, json.research_plan.id, 'ResearchPlan', true)
.then(() => ResearchPlansFetcher.updateAnnotations(researchPlan))
.then(() => this.fetchById(researchPlan.id))))
.catch((errorMessage) => { console.log(errorMessage); });

if (containerFiles.length > 0) {
const tasks = [];
Expand Down Expand Up @@ -258,28 +270,28 @@ export default class ResearchPlansFetcher {
static updateAnnotations(researchPlan) {
return Promise.all(
[
ResearchPlansFetcher.updateAnnotationsOfAttachments(researchPlan),
BaseFetcher.updateAnnotationsInContainer(researchPlan,[])
]);
}

static updateAnnotationsOfAttachments(researchPlan){
ResearchPlansFetcher.updateAnnotationsOfAttachments(researchPlan),
BaseFetcher.updateAnnotationsInContainer(researchPlan, [])
]
);
}

const updateTasks=[];
static updateAnnotationsOfAttachments(researchPlan) {
const updateTasks = [];
researchPlan.attachments
.filter((attach => attach.hasOwnProperty('updatedAnnotation')))
.forEach(attach => {
let data = new FormData();
.filter(((attach) => attach.hasOwnProperty('updatedAnnotation')))
.forEach((attach) => {
const data = new FormData();
data.append('updated_svg_string', attach.updatedAnnotation);
updateTasks.push(fetch('/api/v1/attachments/' + attach.id + '/annotation', {
updateTasks.push(fetch(`/api/v1/attachments/${attach.id}/annotation`, {
credentials: 'same-origin',
method: 'post',
body: data
})
.catch((errorMessage) => {
console.log(errorMessage);
}));
})
.catch((errorMessage) => {
console.log(errorMessage);
}));
});

return Promise.all(updateTasks);
}
Expand Down
4 changes: 3 additions & 1 deletion app/packs/src/fetchers/WellplatesFetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ export default class WellplatesFetcher {
'Content-Type': 'application/json'
},
body: JSON.stringify(wellplate.serialize())
}).then((response) => response.json()).then((json) => {
})
.then((response) => response.json())
.then((json) => {
if (files.length <= 0) {
return new Wellplate(json.wellplate);
}
Expand Down
19 changes: 15 additions & 4 deletions app/packs/src/models/ResearchPlan.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export default class ResearchPlan extends Element {
return this._wellplates || [];
}

upsertAttachments(attachmentsToAdd=[]) {
upsertAttachments(attachmentsToAdd = []) {
const idsOfAttachmentsInResearchPlan = this.attachments.map(
(attachmentInResearchPlan) => attachmentInResearchPlan.identifier
);
Expand Down Expand Up @@ -252,8 +252,19 @@ export default class ResearchPlan extends Element {
delete value.old_value;
});
}
getAttachmentByIdentifier(identifier){
return this.attachments
.filter((attachment)=>attachment.identifier===identifier)[0];

getAttachmentByIdentifier(identifier) {
return this.attachments
.filter((attachment) => attachment.identifier === identifier)[0];
}

getNewAttachments() {
return this.attachments
.filter((attachment) => attachment.is_new === true && !attachment.is_deleted);
}

getMarkedAsDeletedAttachments() {
return this.attachments
.filter((attachment) => attachment.is_deleted === true && !attachment.is_new);
}
}
45 changes: 45 additions & 0 deletions spec/javascripts/packs/src/models/ResearchPlan.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import Attachment from 'src/models/Attachment';
import ResearchPlan from 'src/models/ResearchPlan';
import expect from 'expect';
import {
describe, it
} from 'mocha';

describe('ResearchPlan', () => {
const researchPlan = ResearchPlan.buildEmpty();
Expand Down Expand Up @@ -180,4 +183,46 @@ describe('ResearchPlan', () => {
expect(researchPlan.body).toEqual([permanentBodyField, expected]);
});
});

describe('.getNewAttachments', () => {
describe('.with two new attachments but one is already deleted, one was already there', () => {
const attachmentNewAndDeleted = new Attachment();
const attachmentNew = new Attachment();
const attachmentPresent = new Attachment();
researchPlan.attachments = [attachmentNewAndDeleted, attachmentNew, attachmentPresent];
attachmentNewAndDeleted.is_new = true;
attachmentNewAndDeleted.is_deleted = true;

attachmentNew.is_new = true;
attachmentPresent.is_deleted = false;

attachmentPresent.is_new = false;
attachmentPresent.is_deleted = false;

it('one attachment was found', () => {
expect(researchPlan.getNewAttachments().length).toEqual(1);
});
});
});

describe('.getMarkedAsDeletedAttachments', () => {
describe('.with two new attachments but one is already deleted, one was already there', () => {
const attachmentNewAndDeleted = new Attachment();
const attachmentNew = new Attachment();
const attachmentPresent = new Attachment();
researchPlan.attachments = [attachmentNewAndDeleted, attachmentNew, attachmentPresent];
attachmentNewAndDeleted.is_new = true;
attachmentNewAndDeleted.is_deleted = true;

attachmentNew.is_new = true;
attachmentPresent.is_deleted = false;

attachmentPresent.is_new = false;
attachmentPresent.is_deleted = false;

it('one attachment was found', () => {
expect(researchPlan.getNewAttachments().length).toEqual(1);
});
});
});
});

0 comments on commit 3c0b1e4

Please sign in to comment.