MODBULKOPS-383 - 404 Not Found error (instead of optimistic locking error) on Confirmation screen when bulk edit Items via Central tenant #300
Conversation
| return format("/inventory/view/%s/%s/%s", instanceId, holdingId, item.getId()); | ||
| try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenantIdOfEntity, folioModuleMetadata, folioExecutionContext))) { | ||
| var holding = holdingsClient.getHoldingById(holdingId); | ||
| var instanceId = holding.getInstanceId(); |
There was a problem hiding this comment.
Could you please keep only the holdingsClient invocation inside the try-with-resources block? This will be logically correct and help avoid potential issues in the future. The purpose of this try-with-resources block is to switch the tenant only for the client invocation and then revert it to the original tenant after the block completes.
|
|
||
| public String resolve(EntityType type, BulkOperationsEntity entity) { | ||
|
|
||
| public String resolve(EntityType type, BulkOperationsEntity original) { |
There was a problem hiding this comment.
What was the purpose of renaming the variable entity to original? This utility method should be applied not only to the original entity but potentially to any entity whose type needs to be determined.
There was a problem hiding this comment.
From one side for item in ecs environment for inventory link resolving it is necessary to know tenant, the tenant is populated in original json files. The other side previous code is used this method for original entities only.
There was a problem hiding this comment.
Please, return name of parameter original to entity. This generic method can be used not only for original entity - this can be confusing.
|
hi @khandramai , please take a look. |
|
MODBULKOPS-383 - 404 Not Found error (instead of optimistic locking error) on Confirmation screen when bulk edit Items via Central tenant
Purpose
Approach
Add tenant initialization of item entity to retrieve holding from correct tenant
TODOS and Open Questions
Learning
Pre-Merge Checklist:
Before merging this PR, please go through the following list and take appropriate actions.
If there are breaking changes, please STOP and consider the following:
Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.
While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.