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

Use the FileMetadata#type not FileSet#original_file_id for use #4236

Merged
merged 2 commits into from
Jan 31, 2020

Conversation

no-reply
Copy link
Contributor

FileSet's use "relation" should be controlled exclusively by the related
FileMetadata's #type property. Move toward removing the original_file_id and
other FileSet attributes by reworking the existing queries.

@samvera/hyrax-code-reviewers

Replaces hard-coded uses of the test adapter with configured ones. The adapter
is still used across the board, but with more flexibility.
Copy link
Contributor

@jeremyf jeremyf left a comment

Choose a reason for hiding this comment

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

A few +1 comments, one point of clarification, and one possible blocker regarding spec name.

FileSet's use "relation" should be controlled exclusively by the related
FileMetadata's `#type` property. Move toward removing the `original_file_id` and
other `FileSet` attributes by reworking the existing queries.
jeremyf added a commit that referenced this pull request Jan 31, 2020
From [CircleCI build 17168][1] for [PR #4326][2], we encountered an
error in which we attempted to `+` together `nil` and a String object.
This commit will prevent an application exception when we encounter that
behavior. Below is log entry regarding the error.

```log
ActionView::Template::Error:
       undefined method `+' for nil:NilClass
     # ./app/views/hyrax/homepage/_recent_document.html.erb:7
```

**Is this appropriate?**

I don't know. It looks as though we might want to treat this as in
error. But, given some of the challenges we've had with our feature
specs, I think I'd prefer that this erratic error not break the build.

(Note: When this build broke, Ruby 2.4.7 and 2.6.2 builds of the same
code worked)

[1]:https://circleci.com/gh/samvera/hyrax/17168#tests/containers/5
[2]:#4236
@jeremyf
Copy link
Contributor

jeremyf commented Jan 31, 2020

This error looks weird given other Ruby builds work:

1) Hyrax::CollectionIndexer#generate_solr_document with block yields the document that includes our fields
     # ActiveFedora pattern must become extensible
     Failure/Error: expect { |b| indexer.generate_solr_document(&b) }.to yield_with_args(a_hash_including(doc))
     
       expected given block to yield with arguments, but yielded with unexpected arguments
       expected: [(a hash including {"generic_type_sim" => ["Collection"], "thumbnail_path_ss" => "/downloads/1234?file=thumbnail", "member_of_collection_ids_ssim" => ["col1", "col2"], "member_of_collections_ssim" => ["col1 title", "col2 title"], "visibility_ssi" => "restricted"})]
            got: [{"collection_type_gid_ssim"=>["gid://internal/hyrax-collectiontype/1"], "depositor_ssim"=>["user134@...020-01-31T19:10:52Z", "title_sim"=>["Collection Title 29"], "title_tesim"=>["Collection Title 29"]}]

From https://circleci.com/gh/samvera/hyrax/17173#tests/containers/2

@jeremyf jeremyf merged commit 0a2e9a4 into master Jan 31, 2020
@jeremyf jeremyf deleted the file-navigators branch January 31, 2020 20:04
jeremyf added a commit that referenced this pull request Feb 2, 2020
From [CircleCI build 17168][1] for [PR #4326][2], we encountered an
error in which we attempted to `+` together `nil` and a String object.
This commit will prevent an application exception when we encounter that
behavior. Below is log entry regarding the error.

```log
ActionView::Template::Error:
       undefined method `+' for nil:NilClass
     # ./app/views/hyrax/homepage/_recent_document.html.erb:7
```

**Is this appropriate?**

I don't know. It looks as though we might want to treat this as in
error. But, given some of the challenges we've had with our feature
specs, I think I'd prefer that this erratic error not break the build.

(Note: When this build broke, Ruby 2.4.7 and 2.6.2 builds of the same
code worked)

[1]:https://circleci.com/gh/samvera/hyrax/17168#tests/containers/5
[2]:#4236
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.

2 participants