samvera / hydra-pcdm

Samvera implementation of the PCDM model
Other
11 stars 10 forks source link

implements Hydra::PCDM::VersionedFile #115

Closed flyingzumwalt closed 9 years ago

flyingzumwalt commented 9 years ago

@mjgiarlo suggested a bunch of changes to code that was actually copied from file_spec.rb, so I made those changes in both versioned_file_spec.rb and file_spec.rb

mjgiarlo commented 9 years ago

:+1:

Thoughts, @jcoyne @elrayle @awead @cam156 @hectorcorrea @terrellt :grey_question:

jcoyne commented 9 years ago

Please update this PR to use ActiveFedora 9.2.0.rc2

jcoyne commented 9 years ago

:-1: has_many_versions is basically a no-op. All files are already versionable. I do like the parts where the use predicates are fixed though.

jcoyne commented 9 years ago

see https://github.com/projecthydra/active_fedora/pull/846 for a illustration of why we don't need PCDM::VersionedFile.

flyingzumwalt commented 9 years ago

@jcoyne please explain the correct way, then, for these lines from hydra-works to specify that :original_file should be versioned but the others should not.

    directly_contains_one :original_file, through: :files, type: ::RDF::URI("http://pcdm.org/use#OriginalFile"), class_name: "Hydra::PCDM::VersionedFile"
    directly_contains_one :thumbnail, through: :files, type: ::RDF::URI("http://pcdm.org/use#ThumbnailImage"), class_name: "Hydra::PCDM::File"
    directly_contains_one :extracted_text, through: :files, type: ::RDF::URI("http://pcdm.org/use#ExtractedText"), class_name: "Hydra::PCDM::File"

2 weeks ago your advice was to use class_name to specify a Class that uses has_many_files to ensure that all of its instances are always marked versionable when they're created & retrieved.

jcoyne commented 9 years ago

@flyingzumwalt Specifying class_name was the correct answer for getting a class that uses has_many_files. Its only tonight that I looked at the code and saw that has_many_files only sets an instance variable that is never used. It appears to be a vestigial function that was outmoded when @awead added the version stuff. I don't completely understand how Fedora 4 does versioning, but so far as I can tell, has_many_files doesn't have any effect.

jcoyne commented 9 years ago

@flyingzumwalt I think the only way to version a file is to explicitly call create_version.

awead commented 9 years ago

@jcoyne yes, that's sounds right. I think the is_versionable? methods and so forth are really for 1) identifying files that should be versioned, and 2) identifying files that have versions but may not be indicated as such in the AF class.

I think you can call create_version on any file and it will work regardless of has_versions... that's probably bad.

FWIW, I found Fedora's treatment of versions a bit strange and the present state of AF's versioning was basically put there so we could create versions for Sufia's files. That being said, it probably needs an overhaul. If I can help out in that regard, let me know.

jcoyne commented 9 years ago

@awead see also: https://github.com/projecthydra/sufia/pull/1223

flyingzumwalt commented 9 years ago

new PR on hydra-works makes this PR obsolete projecthydra-labs/hydra-works#137