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

(Work in Progress) Remove sufia dependency #19

Merged
merged 7 commits into from
Jun 26, 2015

Conversation

flyingzumwalt
Copy link
Contributor

I'm off for the week. If you want to see where I'm at, or push things forward (@kevinreiss, @jcoyne) , here's the rundown:

Broken / Still Needs Work

4 Tests are still failing on the branch. They need attention. Some notes:

Version History
hydra-works is not creating new content versions when you upload a new file, meaning there is no version history. This breaks the features around viewing and restoring revisions
For Example:
rspec ./spec/controllers/curation_concerns/generic_files_controller_spec.rb:214

Derivatives

These tests are marked pending right now (in generic_files_controller_spec), with a reference to samvera/hydra-derivatives#60, which explains the blockage.

Also Need to Revisit/Refactor

ManagesEmbargoesActor#interpret_visibility
This manipulates the actor’s self.attributes. That’s kinda sloppy.
In WW, these methods were chained, so they return true/false. In Sufia they often return other values. Need to reconcile that.

GenericFileActor#update_metadata
WW and Sufia take a very different approach to this step and handle the attributes hash differently. Currently GenericFileActor uses the Sufia approach while GenericWorkActor uses the WW approach. Need to decide which approach to use and make them both use it.
Also need to make sure that embargo_release_date and lease_expiration_date are being applied correctly. I think they are. Just need to double-check.

@flyingzumwalt
Copy link
Contributor Author

FYI Here the test failures I get when I run the tests on this branch:

Failures:

  1) CurationConcerns::GenericFilesController when signed in update restoring an old version should be successful
     Failure/Error: expect(generic_file.latest_version.label).to eq('version2')
     NoMethodError:
       undefined method `latest_version' for #<ActiveFedora::File:0x007fe9ced12be8>
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/activemodel-4.2.1/lib/active_model/attribute_methods.rb:433:in `method_missing'
     # ./curation_concerns-models/app/models/concerns/curation_concerns/generic_file_behavior.rb:28:in `latest_version'
     # ./spec/controllers/curation_concerns/generic_files_controller_spec.rb:215:in `block (5 levels) in <top (required)>'

  2) DownloadsController#show sends requested file content
     Failure/Error: Hydra::Works::AddFileToGenericFile.call(generic_file, image_file_path, :thumbnail)
     ArgumentError:
       supplied path must be a string
     # /Users/matt/.rvm/gems/ruby-2.2.0/bundler/gems/hydra-works-013d81759577/lib/hydra/works/services/generic_file/add_file.rb:8:in `call'
     # ./spec/controllers/downloads_controller_spec.rb:47:in `block (3 levels) in <top (required)>'

  3) Add an attached file should update the file
     Failure/Error: within '.related_files' do
     Capybara::ElementNotFound:
       Unable to find css ".related_files"
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/capybara-2.4.4/lib/capybara/node/finders.rb:41:in `block in find'
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/capybara-2.4.4/lib/capybara/node/base.rb:84:in `synchronize'
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/capybara-2.4.4/lib/capybara/node/finders.rb:30:in `find'
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/capybara-2.4.4/lib/capybara/session.rb:676:in `block (2 levels) in <class:Session>'
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/capybara-2.4.4/lib/capybara/session.rb:282:in `within'
     # /Users/matt/.rvm/gems/ruby-2.2.0/gems/capybara-2.4.4/lib/capybara/dsl.rb:51:in `block (2 levels) in <module:DSL>'
     # ./spec/features/add_file_spec.rb:30:in `block (2 levels) in <top (required)>'

  4) CurationConcerns::GenericFile#related_files when the files belong to a batch related_files should return related files and not return itself
     Failure/Error: expect(f1.related_files).to match_array [f2, f3]
       expected collection contained:  [#<CurationConcerns::GenericFile id: "7h14f9418", mime_type: nil, format_label: [], file_size: [], last_modified: [], filename: [], original_checksum: [], rights_basis: [], copyright_basis: [], copyright_note: [], well_formed: [], valid: [], status_message: [], file_title: [], file_author: [], page_count: [], file_language: [], word_count: [], character_count: [], paragraph_count: [], line_count: [], table_count: [], graphics_count: [], byte_order: [], compression: [], color_space: [], profile_name: [], profile_version: [], orientation: [], color_map: [], image_producer: [], capture_device: [], scanning_software: [], exif_version: [], gps_timestamp: [], latitude: [], longitude: [], character_set: [], markup_basis: [], markup_language: [], bit_depth: [], channels: [], data_format: [], offset: [], frame_rate: [], label: nil, depositor: "mjg36", relative_path: nil, import_url: nil, part_of: [], resource_type: [], title: [], creator: [], contributor: [], description: [], tag: [], rights: [], publisher: [], date_created: [], date_uploaded: nil, date_modified: nil, subject: [], language: [], identifier: [], based_near: [], related_url: [], bibliographic_citation: [], source: [], head_id: nil, tail_id: nil, batch_id: "7h14f9397", generic_work_id: nil, embargo_id: "cdebdfcc-2e8e-4561-a02a-feaa14a0a539", lease_id: "e1bfa169-db02-4f25-869e-258d6c313f0d">, #<CurationConcerns::GenericFile id: "7h14f942j", mime_type: nil, format_label: [], file_size: [], last_modified: [], filename: [], original_checksum: [], rights_basis: [], copyright_basis: [], copyright_note: [], well_formed: [], valid: [], status_message: [], file_title: [], file_author: [], page_count: [], file_language: [], word_count: [], character_count: [], paragraph_count: [], line_count: [], table_count: [], graphics_count: [], byte_order: [], compression: [], color_space: [], profile_name: [], profile_version: [], orientation: [], color_map: [], image_producer: [], capture_device: [], scanning_software: [], exif_version: [], gps_timestamp: [], latitude: [], longitude: [], character_set: [], markup_basis: [], markup_language: [], bit_depth: [], channels: [], data_format: [], offset: [], frame_rate: [], label: nil, depositor: "mjg36", relative_path: nil, import_url: nil, part_of: [], resource_type: [], title: [], creator: [], contributor: [], description: [], tag: [], rights: [], publisher: [], date_created: [], date_uploaded: nil, date_modified: nil, subject: [], language: [], identifier: [], based_near: [], related_url: [], bibliographic_citation: [], source: [], head_id: nil, tail_id: nil, batch_id: "7h14f9397", generic_work_id: nil, embargo_id: "49d14c51-4421-4e78-bd68-2ddfcb4e30e7", lease_id: "41c98704-d80e-494c-83a6-3d43a67305e3">]
       actual collection contained:    []
       the missing elements were:      [#<CurationConcerns::GenericFile id: "7h14f9418", mime_type: nil, format_label: [], file_size: [], last_modified: [], filename: [], original_checksum: [], rights_basis: [], copyright_basis: [], copyright_note: [], well_formed: [], valid: [], status_message: [], file_title: [], file_author: [], page_count: [], file_language: [], word_count: [], character_count: [], paragraph_count: [], line_count: [], table_count: [], graphics_count: [], byte_order: [], compression: [], color_space: [], profile_name: [], profile_version: [], orientation: [], color_map: [], image_producer: [], capture_device: [], scanning_software: [], exif_version: [], gps_timestamp: [], latitude: [], longitude: [], character_set: [], markup_basis: [], markup_language: [], bit_depth: [], channels: [], data_format: [], offset: [], frame_rate: [], label: nil, depositor: "mjg36", relative_path: nil, import_url: nil, part_of: [], resource_type: [], title: [], creator: [], contributor: [], description: [], tag: [], rights: [], publisher: [], date_created: [], date_uploaded: nil, date_modified: nil, subject: [], language: [], identifier: [], based_near: [], related_url: [], bibliographic_citation: [], source: [], head_id: nil, tail_id: nil, batch_id: "7h14f9397", generic_work_id: nil, embargo_id: "cdebdfcc-2e8e-4561-a02a-feaa14a0a539", lease_id: "e1bfa169-db02-4f25-869e-258d6c313f0d">, #<CurationConcerns::GenericFile id: "7h14f942j", mime_type: nil, format_label: [], file_size: [], last_modified: [], filename: [], original_checksum: [], rights_basis: [], copyright_basis: [], copyright_note: [], well_formed: [], valid: [], status_message: [], file_title: [], file_author: [], page_count: [], file_language: [], word_count: [], character_count: [], paragraph_count: [], line_count: [], table_count: [], graphics_count: [], byte_order: [], compression: [], color_space: [], profile_name: [], profile_version: [], orientation: [], color_map: [], image_producer: [], capture_device: [], scanning_software: [], exif_version: [], gps_timestamp: [], latitude: [], longitude: [], character_set: [], markup_basis: [], markup_language: [], bit_depth: [], channels: [], data_format: [], offset: [], frame_rate: [], label: nil, depositor: "mjg36", relative_path: nil, import_url: nil, part_of: [], resource_type: [], title: [], creator: [], contributor: [], description: [], tag: [], rights: [], publisher: [], date_created: [], date_uploaded: nil, date_modified: nil, subject: [], language: [], identifier: [], based_near: [], related_url: [], bibliographic_citation: [], source: [], head_id: nil, tail_id: nil, batch_id: "7h14f9397", generic_work_id: nil, embargo_id: "49d14c51-4421-4e78-bd68-2ddfcb4e30e7", lease_id: "41c98704-d80e-494c-83a6-3d43a67305e3">]
     # ./spec/models/curation_concerns/generic_file_spec.rb:228:in `block (5 levels) in <top (required)>'

Finished in 7 minutes 54 seconds (files took 4.43 seconds to load)
264 examples, 4 failures, 19 pending

Failed examples:

rspec ./spec/controllers/curation_concerns/generic_files_controller_spec.rb:214 # CurationConcerns::GenericFilesController when signed in update restoring an old version should be successful
rspec ./spec/controllers/downloads_controller_spec.rb:46 # DownloadsController#show sends requested file content
rspec ./spec/features/add_file_spec.rb:16 # Add an attached file should update the file
rspec ./spec/models/curation_concerns/generic_file_spec.rb:227 # CurationConcerns::GenericFile#related_files when the files belong to a batch related_files should return related files and not return itself

@flyingzumwalt flyingzumwalt force-pushed the remove_sufia_dependency branch 2 times, most recently from bc2ebae to 764d700 Compare June 18, 2015 18:45
@flyingzumwalt flyingzumwalt changed the title (Work in Progress - Don't merge) Remove sufia dependency (Work in Progress) Remove sufia dependency Jun 26, 2015
@flyingzumwalt
Copy link
Contributor Author

Merging this into master even though there is one remaining failing test so that other work can happen in parallel.

The remaining failure is a doozie. CurationConcerns::GenericWork isn't actually including Works::GenericWork and the relationship between CurationConcerns::GenericFile and Batch is at odds with the relationship between CurationConcerns::GenericFile and CurationConcerns::GenericWork. Fixing this breaks a whole lot of tests. The handling of Batches might need to be completely refactored.

flyingzumwalt added a commit that referenced this pull request Jun 26, 2015
(Work in Progress) Begin Removing sufia dependency
@flyingzumwalt flyingzumwalt merged commit 6b9b856 into master Jun 26, 2015
@flyingzumwalt flyingzumwalt deleted the remove_sufia_dependency branch June 26, 2015 16:06
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.

None yet

1 participant