OBPIH-6275 Export boolean values as true/false booleans, instead of a…#4881
OBPIH-6275 Export boolean values as true/false booleans, instead of a…#4881awalkowiak merged 2 commits intodevelopfrom
Conversation
…n empty string for false boolean
kchelstowski
requested review from
alannadolny,
awalkowiak,
drodzewicz and
ewaterman
github-actions
bot
added
the
domain: backend
| value = element.defaultValue | ||
| } | ||
| properties[fieldName] = value ?: "" | ||
| // We can't just check the truthiness of the actualValue, because the false boolean would be evaluated to an empty string |
There was a problem hiding this comment.
"actualValue" -> "value" in these two comments
There was a problem hiding this comment.
Thanks, I copied it from the other file and forgot to change it
| // We can't just check the truthiness of the actualValue, because the false boolean would be evaluated to an empty string | ||
| def cellValue = actualValue == null ? "" : actualValue | ||
| // POI can't handle objects so we need to convert all objects to strings unless they are numeric or boolean | ||
| if (!(cellValue instanceof Number) && !(cellValue instanceof Boolean)) { |
There was a problem hiding this comment.
if we're not passing booleans as strings anymore, what do they appear as in the document?
How does this affect imports? Will existing documents that use the old format still work? If not, this could cause some errors for users who are trying to import old docs. For most cases I imagine this is a non-issue because people do an export (or download template) before an import so will have the latest docs, but I image there are other scenarios where a user might try to use old template because they have it downloaded already.
There was a problem hiding this comment.
We can still pass booleans as strings for some imports. This method is used only for exports, it doesn't touch the import, so nothing would be broken in terms of the import.
The only change is that now, if e.g. a location/product source is inactive, it won't be exported as an empty string, but with the actual boolean value:
vs
This change actually would help users, that they won't eventually have to fill those empty cells with false, as the exported file would already contain the value, that is supposed to be passed during the import.
There was a problem hiding this comment.
@kchelstowski how will this affect the other Excel documents that are exported and then used for import? Like picklist, product associations, etc? Will these be affected or not at all?
There was a problem hiding this comment.
@awalkowiak this would affect e.g. location export, and I mentioned it in the description, but as I said, I believe this could be somewhat expected, as this would fix the same issue in the location export as well (users wouldn't have to fill empty cells with false), as the exported file would be compatible to be imported.
Moreover, in the ticket I had an info bar:
Check Locations export as well, and other exports with active/inactive status
so I believe it would be expected to be done in those as well, but I haven't asked Manon explicitly about that.
TL:DR - I will ask Manon to make sure it's fine to impact/fix this issue in other exports as well and will reach out to you guys.
There was a problem hiding this comment.
I confirm that Manon doesn’t have anything against if it impacted other exports, like Locations export.
| } | ||
| properties[fieldName] = value ?: "" | ||
| // We can't just check the truthiness of the value, because the false boolean would be evaluated to an empty string | ||
| properties[fieldName] = value == null ? "" : value |
There was a problem hiding this comment.
can't we just use elvis operator here?
There was a problem hiding this comment.
@alannadolny no we can't, I explained it in the comment above, that we couldn't just check the truthiness of the value, because the false boolean would be evaluated to an empty string, which we want to avoid.
4 participants
…n empty string for false boolean
✨ Description of Change
Link to GitHub issue or Jira ticket:
Description:
Initially I wanted to just parse booleans to string for the product sources export, not to impact the generic export builder, but it turned out, that the validation while importing the template back, would prevent the active field from being imported, as it would be passed as string, not as a boolean.
The change would potentially impact e.g. the Locations export, but I'd say there is nothing wrong with that, as it also fixes the same problem in the locations export, that would be probably addressed in a ticket anyway.
📷 Screenshots & Recordings (optional)