Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MODORDER-1087]-Delete received pieces in bulk #952

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

yuntianhu
Copy link
Contributor

Description

Purpose: Libraries that use the Receiving app for receipt of print serials may wish to delete pieces that precede a specific date. Currently, FOLIO supports the deletion of pieces, but one at a time. When a library wishes to purge receiving records for multiple years on an ongoing workflow, the ability to delete pieces in bulk will streamline this workflow.

Weeding old journal issues; receiving records no longer necessary since items have been withdrawn

User story statement(s):

As a serials librarian,
I want to delete multiple pieces at one time
so that the continuous addition of pieces does not eventually impact performance of the Receiving app.

Scenarios:

Bulk edit received/expected pieces:

Given user has received multiple piece records for a given receiving title

When expanding received/expected pieces accordion

Then user has option in action menu for Bulk edit

Select one or more pieces:

Given user clicks Bulk edit

When toggles appear in table

Then user can select one or more pieces

Edit pieces:

Given user has selected one or more pieces

When user clicks edit piece button

Then Bulk edit piece form is displayed

Delete all pieces

Given user clicks edit piece button

When Bulk edit piece form is displayed

Then "Delete all" button is active

Delete all pieces confirmation

Given user is viewing Bulk edit piece form

When user clicks "Delete all"

Then confirmation modal is shown

Message: "This will permanently remove all the pieces you have selected. Any related items with a status of "On order" will also be deleted. Would you like to proceed?

AND user can confirm or cancel

Library enhancement suggested by Lafayette College (May 2022 adopter)

Environment

None

@yuntianhu yuntianhu requested review from a team and SerhiiNosko May 28, 2024 17:32
@@ -79,4 +85,26 @@ protected Future<Void> updatePoLine(PieceDeletionHolder holder, RequestContext r
: pieceDeleteFlowPoLineService.updatePoLine(holder, requestContext);
}

public Future<List<Void>> batchDeletePiece (PieceCollection entity, RequestContext requestContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is some formatting issue, also we need to accept list of ids to delete instead of full PieceCollection object

Suggested change
public Future<List<Void>> batchDeletePiece (PieceCollection entity, RequestContext requestContext) {
public Future<List<Void>> batchDeletePieces(List<String> pieceIds, RequestContext requestContext) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example how you can define the an endpoint:
DELETE /<endpoint_name>>
Content-Type: application/json

{
"ids": [1, 2, 3]
}

@@ -79,4 +85,26 @@ protected Future<Void> updatePoLine(PieceDeletionHolder holder, RequestContext r
: pieceDeleteFlowPoLineService.updatePoLine(holder, requestContext);
}

public Future<List<Void>> batchDeletePiece (PieceCollection entity, RequestContext requestContext) {
List <String> ids = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should keep an existing logic that Delete by Id endpoint uses with many existing checks.
In the simplest way you can invoke this method in cycle:
Future deletePiece(String pieceId, boolean deleteHolding, RequestContext requestContext)
It is not optimezed solution of course, but can be as starting point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thank you for pointing it out, will do! :)

entity.getPieces().stream().forEach(v -> ids.add(v.getId()));
List<Future<PieceDeletionHolder>> deletionHolders = ids.stream()
.map(pieceId -> {
PieceDeletionHolder holder = new PieceDeletionHolder().withDeleteHolding(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could not hardcode deleteHolding to true, it should be fetched from request param as doing in Delete By Id endpoint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for pointing it out, I will correct.

Copy link

sonarcloud bot commented Jun 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
71.4% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

}).when(basePieceFlowHolderBuilder).updateHolderWithTitleInformation(PieceDeletionHolderCapture.capture(), eq(requestContext));

final ArgumentCaptor<PieceDeletionHolder> pieceDeletionHolderCapture = ArgumentCaptor.forClass(PieceDeletionHolder.class);
doReturn(succeededFuture(null)).when(pieceDeleteFlowPoLineService).updatePoLine(pieceDeletionHolderCapture.capture(), eq(requestContext));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also separate declaration doReturn and action and verification part to be easy to read



// Mock response setup for deletion
//MockServer.addMockEntry(PIECES_STORAGE, JsonObject.mapFrom(jsonObject).encode()); // Simplified mock data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also remove commentds, if it is not helpful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for pointing it out ! will do

}
responses:
204:
description: "Pieces records successfully deleted"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Piece records or Pieces more correct

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also refactor Sentences in other places to be gramatically correct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, sure will.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants