Prefer [] over [""] for blank values that are split #839
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
summer-cook
approved these changes
Jul 20, 2023
irb(main):002:0> [].blank?
=> true
irb(main):003:0> [""].blank?
=> falseThis ^ is the only potential issue with this change that I can think of off the top of my head; if a project is doing presence checks on "empty" values and that logic depends on the original behavior. |
|
This may offer a partial solution to: |
jeremyf
added a commit
that referenced
this pull request
Aug 2, 2023
Prior to this commit, when exporting a FileSet and GenericWork we'd see a "creator_1" and "creator" column. The "creator" column might look as follows: `["Sandra Samvera"]`. Which meant that creator was not an `ActiveTriples::Relation` (based on the prior logic). Yet the "creator" field was stored as multi-value. Perhaps having been previously coerced into an Array. With this commit, we're favoring the Bulkrax parser field mapping `"join"` configuration over whether or not the object is an =ActiveTriples::Relation=. That is to say, if you told us to join the field, we're going to do that regardless of the data we have. Likewise, if the field's value is not an enumerable, we're not going to introduce an ordinal suffix (e.g. "creator_1" when we have a scalar creator). In other words don't do the join logic if we don't have an "array" or we weren't told to join arrays. Perhaps we could interrogate the model to ask if it's single value or not? But this reduces the implementation knowledge of the properties by looking at a more primitive level (is the data multi-valued or not). Related to: - scientist-softserv/palni-palci#624 - #839
jeremyf
added a commit
that referenced
this pull request
Aug 2, 2023
Prior to this commit, when exporting a FileSet and GenericWork we'd see a "creator_1" and "creator" column. The "creator" column might look as follows: `["Sandra Samvera"]`. Which meant that creator was not an `ActiveTriples::Relation` (based on the prior logic). Yet the "creator" field was stored as multi-value. Perhaps having been previously coerced into an Array. With this commit, we're favoring the Bulkrax parser field mapping `"join"` configuration over whether or not the object is an =ActiveTriples::Relation=. That is to say, if you told us to join the field, we're going to do that regardless of the data we have. Likewise, if the field's value is not an enumerable, we're not going to introduce an ordinal suffix (e.g. "creator_1" when we have a scalar creator). In other words don't do the join logic if we don't have an "array" or we weren't told to join arrays. Perhaps we could interrogate the model to ask if it's single value or not? But this reduces the implementation knowledge of the properties by looking at a more primitive level (is the data multi-valued or not). Related to: - scientist-softserv/palni-palci#624 - #839
jeremyf
added a commit
that referenced
this pull request
Aug 2, 2023
Prior to this commit, when exporting a FileSet and GenericWork we'd see a "creator_1" and "creator" column. The "creator" column might look as follows: `["Sandra Samvera"]`. Which meant that creator was not an `ActiveTriples::Relation` (based on the prior logic). Yet the "creator" field was stored as multi-value. Perhaps having been previously coerced into an Array. With this commit, we're favoring the Bulkrax parser field mapping `"join"` configuration over whether or not the object is an =ActiveTriples::Relation=. That is to say, if you told us to join the field, we're going to do that regardless of the data we have. Likewise, if the field's value is not an enumerable, we're not going to introduce an ordinal suffix (e.g. "creator_1" when we have a scalar creator). In other words don't do the join logic if we don't have an "array" or we weren't told to join arrays. Perhaps we could interrogate the model to ask if it's single value or not? But this reduces the implementation knowledge of the properties by looking at a more primitive level (is the data multi-valued or not). Related to: - scientist-softserv/palni-palci#624 - #839
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.