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

Add 'Original Name' field storage #735

Merged
merged 3 commits into from
Oct 22, 2019
Merged

Add 'Original Name' field storage #735

merged 3 commits into from
Oct 22, 2019

Conversation

seth-shaw-unlv
Copy link

@seth-shaw-unlv seth-shaw-unlv commented Oct 17, 2019

Github Issue: Islandora/documentation#1276

What does this Pull Request do?

Adds the PREMIS 3 namespace and creates the field storage for an 'original name' field for media.

How should this be tested?

  • Apply this PR
  • Apply Add 'Original Name' field to Media islandora_defaults#13
  • Import both the Islandora Core and Islandora Defaults features. E.g. drush fim -y islandora_core_feature; drush fim -y islandora_defaults
  • Create a Media; you should see an 'Original Name' field. Plop any text you want in there.
  • View the Media's JSON-LD; you should see the text from the original name field associated with the PREMIS 3 originalName predicate
  • View the Media in Fedora; the PREMIS 3 originalName value should be there too.

Additional Notes:

We should eventually figure out how to auto-populate this from the file upload widget.

Interested parties

@mjordan, @Islandora/8-x-committers

@mjordan
Copy link

mjordan commented Oct 18, 2019

@seth-shaw-unlv Travis is failing with a "No such feature is installed: islandora_core_feature" error. Can you take a look?

@mjordan mjordan self-requested a review October 18, 2019 14:18
@seth-shaw-unlv
Copy link
Author

@mjordan, I neglected to remove the field_permissions dependency Features included. It should build now.

@mjordan
Copy link

mjordan commented Oct 18, 2019

Cool, I'll re-pull so I can test on the plane 😄

@mjordan
Copy link

mjordan commented Oct 18, 2019

Still failing, this time on what appears to be a crayfish dependency issue.

@seth-shaw-unlv
Copy link
Author

@mjordan, yeah, we've been chatting about that on Slack. It needed a PR that is now merged. Should be ready to rebuild now. 🤞

@mjordan
Copy link

mjordan commented Oct 20, 2019

Getting the following when I try to import the features. I'm in the issue-1276 branch on both the islandora and islandora_defaults modules:

vagrant@claw:/var/www/html/drupal/web/modules$ drush fim islandora
vagrant@claw:/var/www/html/drupal/web/modules$ drush fim islandora_defaults

 Do you really want to import islandora_defaults : field.field.media.video.field_original_name? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : field.field.media.image.field_original_name? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : field.field.media.file.field_original_name? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : field.field.media.audio.field_original_name? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_form_display.media.audio.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_form_display.media.file.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_form_display.media.image.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_form_display.media.video.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_form_display.node.islandora_object.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_view_display.media.audio.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_view_display.media.file.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_view_display.media.image.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_view_display.media.video.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : core.entity_view_display.node.islandora_object.default? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : rdf.mapping.media.audio? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : rdf.mapping.media.file? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : rdf.mapping.media.image? (yes/no) [yes]:
 > 

 Do you really want to import islandora_defaults : rdf.mapping.media.video? (yes/no) [yes]:
 > 


In FieldConfig.php line 315:
                                                                                           
  Attempt to create a field field_original_name that does not exist on entity type media.  
                                                                                           

@seth-shaw-unlv
Copy link
Author

@mjordan, you need to drush fim islandora_core_feature. That is where the config is.

@seth-shaw-unlv
Copy link
Author

[vagrant@claw islandora_defaults]$ drush fim -y islandora_core_feature

 // Do you really want to import islandora_core_feature : field.storage.media.field_original_name?: yes.                

[vagrant@claw islandora_defaults]$ drush fim -y islandora_defaults

 // Do you really want to import islandora_defaults : field.field.media.video.field_original_name?: yes.                

 // Do you really want to import islandora_defaults : field.field.media.image.field_original_name?: yes.                

 // Do you really want to import islandora_defaults : field.field.media.file.field_original_name?: yes.                 

 // Do you really want to import islandora_defaults : field.field.media.audio.field_original_name?: yes.                

 // Do you really want to import islandora_defaults : core.entity_form_display.media.audio.default?: yes.               

 // Do you really want to import islandora_defaults : core.entity_form_display.media.file.default?: yes.                

 // Do you really want to import islandora_defaults : core.entity_form_display.media.image.default?: yes.               

 // Do you really want to import islandora_defaults : core.entity_form_display.media.video.default?: yes.               

 // Do you really want to import islandora_defaults : core.entity_view_display.media.audio.default?: yes.               

 // Do you really want to import islandora_defaults : core.entity_view_display.media.file.default?: yes.                

 // Do you really want to import islandora_defaults : core.entity_view_display.media.image.default?: yes.               

 // Do you really want to import islandora_defaults : core.entity_view_display.media.video.default?: yes.               

 // Do you really want to import islandora_defaults : rdf.mapping.media.audio?: yes.                                    

 // Do you really want to import islandora_defaults : rdf.mapping.media.file?: yes.                                     

 // Do you really want to import islandora_defaults : rdf.mapping.media.image?: yes.                                    

 // Do you really want to import islandora_defaults : rdf.mapping.media.video?: yes.                                    

[vagrant@claw islandora_defaults]$ 

@mjordan
Copy link

mjordan commented Oct 21, 2019

Sorry, will try again this evening.

@mjordan
Copy link

mjordan commented Oct 22, 2019

Works as advertised. However, the Original Name field is of type "Text (plain, long)", with 5 rows. Shouldn't it be "Text (plain)"?

@seth-shaw-unlv
Copy link
Author

Generally the 255 characters provided by a string field is enough to support a file name based on the max limit for modern filesystems. However, users may also want to include original path information as part of the field's value, so I opted for string_long which can support several thousand characters.

@mjordan
Copy link

mjordan commented Oct 22, 2019

Fair enough!

@mjordan
Copy link

mjordan commented Oct 22, 2019

@Islandora/8-x-committers I've tested this and am ready to merge, but does anyone feel like we need to autopopulate the field at this point? Or is it OK if that's a subsequent PR?

@dannylamb
Copy link

@mjordan Merging is fine by me 🙇‍♂️ We can keep Islandora/documentation#1276 open for someone to reference when they follow up.

Copy link

@mjordan mjordan left a comment

Choose a reason for hiding this comment

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

Tested with Islandora/islandora_defaults#13, works as intended.

@mjordan mjordan merged commit ed239b5 into Islandora:8.x-1.x Oct 22, 2019
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.

3 participants