I754 #756
Merged
I754 #756
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
Prior to this commit, we'd attempt to export the field even if the "from" object did not respond to that property. That would raise a `NoMethodError`. With this commit, we're skipping the building of the value/object if the hyrax_record does not respond to the source method. Related to: - notch8/britishlibrary#289 - #754
The `current_record_ids` was removed in 4cf0207 (e.g. #749) in favor of `current_records_for_export`; however the `Bulkrax::CsvParser#total` method was not updated to reflect this change. In this commit, we fix that issue. Also we tighten up the logic regarding the sorted_entries array. There was an assumption that we would have a `nil` limit; but the documentation mentions that 0 and `nil` are equivalent. But `(0..(0 || total))` is different from `(0..(nil || total))`. Partially closes #754
jeremyf
added a commit
to notch8/britishlibrary
that referenced
this pull request
Mar 7, 2023
This commit includes two primary things: - CsvParser fix to logic (see samvera/bulkrax#756) - CsvEntry fix for creator property disparity between FileSets and other works Without this hack, when we export a FileSet, it skips exporting the FileSet's creator. This is because we are overloading the `creator` in the parser. With this change, the complex `creator` object for works will export correctly (e.g. have the constituent field parts in the CSV). And the FileSet will have a `creator` column. Related to: - #289 - samvera/bulkrax#756
jeremyf
added a commit
to notch8/britishlibrary
that referenced
this pull request
Mar 7, 2023
This commit includes two primary things: - CsvParser fix to logic (see samvera/bulkrax#756) - CsvEntry fix for creator property disparity between FileSets and other works Without this hack, when we export a FileSet, it skips exporting the FileSet's creator. This is because we are overloading the `creator` in the parser. With this change, the complex `creator` object for works will export correctly (e.g. have the constituent field parts in the CSV). And the FileSet will have a `creator` column. Related to: - #289 - samvera/bulkrax#756
ShanaLMoore
approved these changes
Mar 7, 2023
sephirothkod
approved these changes
Mar 7, 2023
jeremyf
added a commit
to notch8/adventist-dl
that referenced
this pull request
Mar 8, 2023
There are two fixes this includes: - Avoid building value/object if hyrax_record does not respond - Fixing bug regarding total and export limits Related to: - samvera/bulkrax#756
jeremyf
added a commit
to notch8/britishlibrary
that referenced
this pull request
Mar 8, 2023
The Bulkrax update provides the following fixes: - Avoid building value/object if hyrax_record does not respond - Fixing bug regarding total and export limits As such, some of the overrides are no longer needed. Related to: - samvera/bulkrax#756
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.
Avoid building value/object if hyrax_record does not respond
bd42b4a
Prior to this commit, we'd attempt to export the field even if the
"from" object did not respond to that property. That would raise a
NoMethodError.With this commit, we're skipping the building of the value/object if the
hyrax_record does not respond to the source method.
Related to:
Fixing bug regarding total and export limits
951128d
The
current_record_idswas removed in 4cf0207 (e.g. #749) in favor ofcurrent_records_for_export; however theBulkrax::CsvParser#totalmethod was not updated to reflect this change.
In this commit, we fix that issue. Also we tighten up the logic
regarding the sorted_entries array. There was an assumption that we
would have a
nillimit; but the documentation mentions that 0 andnilare equivalent. But(0..(0 || total))is different from(0..(nil || total)).Partially closes #754