Skip to content

Commit

Permalink
MODBULKOPS-355 - Are you sure" preview displays outdated values after…
Browse files Browse the repository at this point in the history 
… User changed selection on bulk edit form and clicked "Confirm changes" (#274)

* MODBULKOPS-355 - Holdings fail to be bulk edited from Central tenant
  • Loading branch information
@khandramai
khandramai authored Oct 11, 2024
1 parent 1f0be9e commit 8d98358
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 12 deletions.
2 changes: 1 addition & 1 deletion descriptors/ModuleDescriptor-template.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"provides": [
{
"id": "bulk-operations",
"version": "1.3",
"version": "1.4",
"handlers": [
{
"methods": [ "POST" ],
Expand Down
13 changes: 9 additions & 4 deletions src/main/java/org/folio/bulkops/service/BulkOperationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static org.folio.bulkops.domain.dto.OperationStatusType.COMPLETED;
import static org.folio.bulkops.domain.dto.OperationStatusType.COMPLETED_WITH_ERRORS;
import static org.folio.bulkops.domain.dto.OperationStatusType.DATA_MODIFICATION;
import static org.folio.bulkops.domain.dto.OperationStatusType.DATA_MODIFICATION_IN_PROGRESS;
import static org.folio.bulkops.domain.dto.OperationStatusType.EXECUTING_QUERY;
import static org.folio.bulkops.domain.dto.OperationStatusType.FAILED;
import static org.folio.bulkops.domain.dto.OperationStatusType.NEW;
Expand Down Expand Up @@ -113,7 +114,7 @@
@RequiredArgsConstructor
public class BulkOperationService {
public static final String FILE_UPLOADING_FAILED_REASON = "File uploading failed, reason: %s";
public static final String STEP_S_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS = "Step %s is not applicable for bulk operation status %s";
public static final String STEP_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS = "Step %s is not applicable for bulk operation status %s";
public static final String ERROR_STARTING_BULK_OPERATION = "Error starting Bulk Operation: ";
@Value("${application.file-uploading.max-retry-count}")
private int maxRetryCount;
Expand Down Expand Up @@ -224,6 +225,10 @@ public BulkOperation triggerByQuery(UUID userId, QueryRequest queryRequest) {
}

public void confirm(BulkOperation operation) {

operation.setStatus(DATA_MODIFICATION_IN_PROGRESS);
bulkOperationRepository.save(operation);

if (operation.getEntityType() == INSTANCE_MARC) {
confirmForInstanceMarc(operation);
return;
Expand Down Expand Up @@ -558,14 +563,14 @@ public BulkOperation startBulkOperation(UUID bulkOperationId, UUID xOkapiUserId,
}
return operation;
} else {
throw new BadRequestException(format(STEP_S_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS, step, operation.getStatus()));
throw new BadRequestException(format(STEP_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS, step, operation.getStatus()));
}
} else if (BulkOperationStep.COMMIT == step) {
if (REVIEW_CHANGES.equals(operation.getStatus())) {
executor.execute(getRunnableWithCurrentFolioContext(() -> commit(operation)));
return operation;
} else {
throw new BadRequestException(format(STEP_S_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS, step, operation.getStatus()));
throw new BadRequestException(format(STEP_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS, step, operation.getStatus()));
}
} else {
throw new IllegalOperationStateException("Bulk operation cannot be started, reason: invalid state: " + operation.getStatus());
Expand Down Expand Up @@ -598,7 +603,7 @@ private String executeDataExportJob(BulkOperationStep step, ApproachType approac
}
}
} else {
throw new BadRequestException(format(STEP_S_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS, step, operation.getStatus()));
throw new BadRequestException(format(STEP_IS_NOT_APPLICABLE_FOR_BULK_OPERATION_STATUS, step, operation.getStatus()));
}
} catch (Exception e) {
log.error(ERROR_STARTING_BULK_OPERATION + e.getCause());
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/db/changelog/changelog-master.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@
<include file="changes/29-08-2024_add_used_tenants.xml" relativeToChangelogFile="true"/>
<include file="changes/06-08-2024_update_bulk_operation_table_add_di_job_profile_id.xml" relativeToChangelogFile="true"/>
<include file="changes/20-09-2024_add_tenants_to_rules.xml" relativeToChangelogFile="true"/>
<include file="changes/08-10-2024_add_bulk_operation_data_modification_in_progress_status.xml" relativeToChangelogFile="true"/>
</databaseChangeLog>
1 change: 1 addition & 0 deletions .../changelog/changes/08-10-2024_add_bulk_operation_data_modification_in_progress_status.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TYPE OperationStatusType ADD VALUE IF NOT EXISTS 'DATA_MODIFICATION_IN_PROGRESS';
13 changes: 13 additions & 0 deletions .../changelog/changes/08-10-2024_add_bulk_operation_data_modification_in_progress_status.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.8.xsd">


<changeSet id="08-10-2024_add_bulk_operation_data_modification_in_progress_status" author="firebird">
<sqlFile path="08-10-2024_add_bulk_operation_data_modification_in_progress_status.sql" relativeToChangelogFile="true" />
</changeSet>

</databaseChangeLog>
1 change: 1 addition & 0 deletions src/main/resources/swagger.api/schemas/operation_status_type.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"RETRIEVING_RECORDS",
"SAVING_RECORDS_LOCALLY",
"DATA_MODIFICATION",
"DATA_MODIFICATION_IN_PROGRESS",
"REVIEW_CHANGES",
"APPLY_CHANGES",
"SUSPENDED",
Expand Down
14 changes: 7 additions & 7 deletions src/test/java/org/folio/bulkops/service/BulkOperationServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ void shouldConfirmChanges(ApproachType approach) {
assertThat(capturedDataProcessingEntity.getEndTime(), notNullValue());

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture()));
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(3)).save(bulkOperationCaptor.capture()));
var capturedBulkOperation = bulkOperationCaptor.getValue();
assertThat(capturedBulkOperation.getLinkToModifiedRecordsCsvFile(), equalTo(expectedPathToModifiedCsvFile));
assertThat(capturedBulkOperation.getStatus(), equalTo(OperationStatusType.REVIEW_CHANGES));
Expand Down Expand Up @@ -484,7 +484,7 @@ void shouldPopulateErrorToBulkOperationIfS3IssuesForConfirmChanges() {
bulkOperationService.startBulkOperation(bulkOperationId, UUID.randomUUID(), new BulkOperationStart().approach(ApproachType.IN_APP).step(EDIT));

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture()));
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(3)).save(bulkOperationCaptor.capture()));
var capturedBulkOperation = bulkOperationCaptor.getValue();

assertThat(capturedBulkOperation.getStatus(), equalTo(OperationStatusType.FAILED));
Expand Down Expand Up @@ -553,7 +553,7 @@ void shouldConfirmChangesForInstanceMarc() {
assertThat(capturedDataProcessingEntity.getEndTime(), notNullValue());

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture()));
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(3)).save(bulkOperationCaptor.capture()));
var capturedBulkOperation = bulkOperationCaptor.getValue();
assertThat(capturedBulkOperation.getLinkToModifiedRecordsMarcFile(), equalTo(expectedPathToModifiedMarcFile));
assertThat(capturedBulkOperation.getStatus(), equalTo(OperationStatusType.REVIEW_CHANGES));
Expand Down Expand Up @@ -608,7 +608,7 @@ void shouldPopulateErrorToBulkOperationIfS3IssuesForConfirmChangesForInstanceMa
bulkOperationService.startBulkOperation(bulkOperationId, UUID.randomUUID(), new BulkOperationStart().approach(ApproachType.IN_APP).step(EDIT));

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture()));
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(3)).save(bulkOperationCaptor.capture()));
var capturedBulkOperation = bulkOperationCaptor.getValue();
assertThat(capturedBulkOperation.getStatus(), equalTo(OperationStatusType.FAILED));
assertThat(capturedBulkOperation.getErrorMessage(),equalTo(ErrorCode.ERROR_NOT_CONFIRM_CHANGES_S3_ISSUE));
Expand Down Expand Up @@ -667,7 +667,7 @@ void shouldFailConfirmChangesForInstanceMarcIfMarcWriterNotAvailable() {
Awaitility.await().untilAsserted(() -> verify(dataProcessingRepository, times(2)).save(dataProcessingCaptor.capture()));

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture()));
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(3)).save(bulkOperationCaptor.capture()));
var capturedBulkOperation = bulkOperationCaptor.getValue();
assertThat(capturedBulkOperation.getStatus(), equalTo(OperationStatusType.FAILED));
}
Expand Down Expand Up @@ -764,7 +764,7 @@ void shouldConfirmChangesForItemWhenValidationErrorAndOtherValidChangesExist(App
verify(errorService).saveError(eq(bulkOperationId), eq("10101"), any(String.class));

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture()));
Awaitility.await().untilAsserted(() -> verify(bulkOperationRepository, times(3)).save(bulkOperationCaptor.capture()));
var capturedBulkOperation = bulkOperationCaptor.getValue();
assertThat(capturedBulkOperation.getLinkToModifiedRecordsCsvFile(), equalTo(expectedPathToModifiedCsvFile));
assertThat(capturedBulkOperation.getCommittedNumOfErrors(), equalTo(1));
Expand Down Expand Up @@ -834,7 +834,7 @@ void shouldUpdateStatusesWhenConfirmChangesFails() {
assertThat(capturedDataProcessingEntity.getEndTime(), notNullValue());

var bulkOperationCaptor = ArgumentCaptor.forClass(BulkOperation.class);
verify(bulkOperationRepository).save(bulkOperationCaptor.capture());
verify(bulkOperationRepository, times(2)).save(bulkOperationCaptor.capture());
var capturedBulkOperation = bulkOperationCaptor.getValue();
assertThat(capturedBulkOperation.getStatus(), equalTo(OperationStatusType.FAILED));
assertThat(capturedBulkOperation.getEndTime(), notNullValue());
Expand Down

0 comments on commit 8d98358

Please sign in to comment.