[MODSOURCE-840] Extend domain event with source MARC #664
Conversation
This reverts commit 1cb5237.
RomanChernetskyi
requested review from
psmagin,
Aliaksandr-Fedasiuk,
dmytrokrutii,
VRohach,
KaterynaSenchenko,
RuslanLavrov,
okolawole-ebsco and
JavokhirAbdullayev
| .onSuccess(v -> recordDomainEventPublisher.publishRecordUpdated(oldRecord, | ||
| updatedRecord | ||
| .withErrorRecord(oldRecord.getErrorRecord()) | ||
| .withRawRecord(oldRecord.getRawRecord()) |
There was a problem hiding this comment.
Is this correct? Here we assign oldRecord.getRawRecord() to updatedRecord. Same comment on previous line .withErrorRecord(oldRecord.getErrorRecord())
There was a problem hiding this comment.
Yes, it is correct, but I already removed this piece of code because we decided not to send raw records and error records to make the domain event less overwhelmed with unneeded data
| return getRecordById(txQE, oldRecord.getId()) | ||
| .compose(optionalRecord -> optionalRecord.map(exisitngRecord -> | ||
| insertOrUpdateRecord(txQE, oldRecord).compose(r -> insertOrUpdateRecord(txQE, newRecord)) | ||
| .onSuccess(updatedRecord -> recordDomainEventPublisher.publishRecordUpdated(exisitngRecord, updatedRecord, okapiHeaders))) |
There was a problem hiding this comment.
typo in exisitngRecord. Should be existingRecord
There was a problem hiding this comment.
Fixed, thanks
| .compose(optionalRecord -> optionalRecord.map(exisitngRecord -> | ||
| insertOrUpdateRecord(txQE, oldRecord).compose(r -> insertOrUpdateRecord(txQE, newRecord)) | ||
| .onSuccess(updatedRecord -> recordDomainEventPublisher.publishRecordUpdated(exisitngRecord, updatedRecord, okapiHeaders))) | ||
| .orElse(Future.failedFuture(new NotFoundException(format(RECORD_NOT_FOUND_TEMPLATE, oldRecord.getId()))))); |
There was a problem hiding this comment.
Per the existing logic, we insert new record to DB if the record do not already exist. Per the new logic, new throw NotFoundException when record do not exist in DB. Is it ok? (I don't know as I am not familiar with this module)
There was a problem hiding this comment.
It's ok because this method updates the existing oldRecord and creates newRecord, so we expect to have oldRecord at DB before executing insert operation
|
Hi @RomanChernetskyi, Changes looks good to me except few minor comments. This change is going to break mod-linked-data application. Is it possible to wait few days for merging this PR? We will create a ticket & make changes in linked data per the new event payload. Here is my understanding about the payload change: Before:
After this change:
@PBobylev What do you think? May be we can make change in linked-data soon? If the payload change I mentioned above is correct, I think this is a minor change in linked-data. |
| @@ -48,7 +55,7 @@ public Future<String> handle(KafkaConsumerRecord<String, String> consumerRecord) | |||
| logInput(authorityId, eventSubType, tenantId); | |||
| return (switch (eventSubType) { | |||
| case SOFT_DELETE -> performSoftDelete(authorityId, tenantId); | |||
There was a problem hiding this comment.
Shouldn't it be a delete event on soft delete?
There was a problem hiding this comment.
So far, we decided to send an update on the soft delete of Marc's records, but during further refinements, requirements could be changes
|
|
Here is the PR to do the corresponding change in mod-linked-data: folio-org/mod-linked-data#111 |
|
@RomanChernetskyi Here is the corresponding change in mod-linked-data folio-org/mod-linked-data#111 |
Purpose
Create/Update/Delete operations in the module should generate related events with the new/old body for the entity (instances/authorities)
Acceptance Criteria:
Approach
Send domain events with old and new record
Jira
https://folio-org.atlassian.net/browse/MODSOURCE-840