Skip to content

OBPIH-6344 Show translated product names in PO view page tabs#4615

Merged
awalkowiak merged 1 commit intofeature/upgrade-to-grails-3.3.10from
OBPIH-6344
May 14, 2024
Merged

OBPIH-6344 Show translated product names in PO view page tabs#4615
awalkowiak merged 1 commit intofeature/upgrade-to-grails-3.3.10from
OBPIH-6344

Conversation

@kchelstowski
Copy link
Collaborator

No description provided.

<tr class="${i%2?'even':'odd'}">
<td>${invoiceItem?.orderItem?.id}</td>
<td>${invoiceItem?.product?.productCode}</td>
<td>${invoiceItem?.description}</td>
Copy link
Member

Choose a reason for hiding this comment

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

can you briefly explain to me why we need this conditional? We only want to show the description if there's an adjustment?

Copy link
Collaborator Author

@kchelstowski kchelstowski May 14, 2024

Choose a reason for hiding this comment

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

@ewaterman I'm sorry, I forgot to finish the answer yesterday.
description transient field is defined like this:

String getDescription() {
        return orderAdjustment ? orderAdjustment.description : product?.name
}

so on the frontend side I had to find some way to determine whether I return the orderAdjustment's description or the product name. If the product name, then we'd want to return a synonym of the product, otherwise we don't want to change anything and still return the orderAdjustment.description.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, feels like we could try to move this to the domain (and return either description or product), but I am not sure how we could achieve this in a simple way, where we want to have either string or product and based on that use a format:displayName tag or not. Perhaps we could extend the displayName tag and if attr.product is type of string, then just return it as is or if it is type of product, then use the existing logic 🤔 But I am not sure if that won't overcomplicate it too much and I think we can leave it as is right now (?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, feels like we could try to move this to the domain (and return either description or product)

This is currently in the domain, so I might not understand what you mean.
If you meant a different method, still, it would be impossible to distinguish on the frontend side (without having the if logic) if it's the description or the product.
In my opinion trying to handle this one, minor case in the generic format:displayName seems not worth it, unless we face more of those edge cases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are currently returning a product name in the domain (unless it somehow returns translated one)

Copy link
Collaborator Author

@kchelstowski kchelstowski May 14, 2024

Choose a reason for hiding this comment

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

yes it does, but I didn't want to change it to return the translated name (in case somewhere it is not expected, and the description transient is used there) + again, on the frontend I would still need a mechanism to determine whether it is the product name, or the order adjustment description in order to know, if I should display the tooltip (we used to add tooltips with an original name of a product).

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's why I wrote: "It feels like we could try to move this to the domain (...)", but we'd need to also "extend the displayName tag", but it might unnecessarily overcomplicate it and does not 100% feel right.

@awalkowiak awalkowiak merged commit d4fb62b into feature/upgrade-to-grails-3.3.10 May 14, 2024
@awalkowiak awalkowiak deleted the OBPIH-6344 branch May 14, 2024 08:57
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.

4 participants