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

OBPIH-5329 Improve performance after deleting location #3787

Merged
merged 2 commits into from
Jan 20, 2023

Conversation

awalkowiak
Copy link
Collaborator

plus fixed deleting bin location with zone
plus fixed finding location type by name during import

There was an issue due to the fact, that 'get' expects one result but these criteria might return more objects.
@awalkowiak awalkowiak merged commit c27e2b6 into release/0.8.21 Jan 20, 2023
@awalkowiak awalkowiak deleted the OBPIH-5329 branch January 20, 2023 15:02
Copy link
Member

@jmiranda jmiranda left a comment

Choose a reason for hiding this comment

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

Sorry for not getting to this sooner.

if (zone) {
existingLocation.zone = null
zone.removeFromLocations(existingLocation)
zone.save(flush: true)
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be necessary since there's no association between zone and internal locations so this is basically a no-op. I assume this is in response to an issue you had (something like "deleted object would be re-saved"). In the future, can you add any issues you encounter to the ticket a) so there's an audit trail of the issue and b) so we can discuss it.

}
}
// Find by name with like, but value more the exact match
locationType = LocationType.findByNameOrNameIlike(params.locationType, params.locationType + "%")
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why this was necessary?

@@ -34,7 +34,8 @@ class RefreshProductAvailabilityEvent extends ApplicationEvent {
this.locationId = source?.isInternalLocation() ? source?.parentLocation?.id : source?.id
this.productIds = []
this.forceRefresh = forceRefresh
this.disableRefresh = disableRefresh
// Avoid refreshing PA for all location, when there is no locationId
this.disableRefresh = this.locationId ? disableRefresh : Boolean.TRUE
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be the only commit for this ticket. The other issues should have separate bug tickets that include the stacktrace and/or steps to reproduce.

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.

None yet

2 participants