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-5980 Data display error in Delivery Note when Pack level 1 is entered #4421

Merged
merged 3 commits into from
Dec 22, 2023

Conversation

alannadolny
Copy link
Collaborator

No description provided.

@alannadolny alannadolny self-assigned this Dec 14, 2023
@alannadolny alannadolny added the warn: do not merge Marks a pull request that is not yet ready to be merged label Dec 14, 2023
@alannadolny
Copy link
Collaborator Author

I added a do not merge label to avoid merging this PR now, because it has to wait for 0.9.1

@@ -65,7 +65,7 @@
</g:each>
</td>
</g:if>
<g:if test="${requisitionItems.find { it.requisition?.shipment?.shipmentItems?.any { it.container }}}">
<g:if test="${requisitionItems.find { it.requisition?.shipment?.shipmentItems?.any { it.container && it.container?.parentContainer }}}">
Copy link
Collaborator

@kchelstowski kchelstowski Dec 14, 2023

Choose a reason for hiding this comment

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

wouldn't just:

.any { it.container?.parentContainer }

work?

@alannadolny alannadolny changed the base branch from feature/upgrade-to-grails-3.3.10 to release/0.9.0-hotfix1 December 20, 2023 14:48
@alannadolny alannadolny removed the warn: do not merge Marks a pull request that is not yet ready to be merged label Dec 20, 2023
@@ -65,7 +65,7 @@
</g:each>
</td>
</g:if>
<g:if test="${requisitionItems.find { it.requisition?.shipment?.shipmentItems?.any { it.container }}}">
<g:if test="${requisitionItems.find { it.requisition?.shipment?.shipmentItems?.any { it.container?.parentContainer }}}">
Copy link
Member

Choose a reason for hiding this comment

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

From what I can tell from the revision history, the code has gone from something like this

<g:if test="${shipmentItems.any { it.container }}">

to this

<g:if test="${shipmentItems.any { it.container && it.container?.parentContainer }}">

and now to this

<g:if test="${requisitionItems.find { it.requisition?.shipment?.shipmentItems?.any { it.container }}}">

I have no idea what this code is trying to achieve, so I feel like now would be a good time to move this logic to the domain. I just don't know exactly which object to interogate to get this information. Shipment? Requisition?

In either case, it should probably live outside of whatever loop we're iterating over in case the method is expensive (i.e. requires database integration).

whateverObjectWeShouldInterogate.hasParentContainer()
whateverObjectWeShouldInterogate.hasChildContrainer()

@awalkowiak awalkowiak merged commit 3a4eda1 into release/0.9.0-hotfix1 Dec 22, 2023
@awalkowiak awalkowiak deleted the OBPIH-5980 branch December 22, 2023 15:51
awalkowiak pushed a commit that referenced this pull request Jan 25, 2024
…ntered (#4421)

* OBPIH-5980 Add additional condition to avoid adding cell when related header is not visible

* OBPIH-5980 Fix after review

* OBPIH-5980 Move some logic from printPage to shipment domain
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

5 participants