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

ANW-451 #1205

Merged
merged 15 commits into from Apr 23, 2018
Merged

ANW-451 #1205

merged 15 commits into from Apr 23, 2018

Conversation

avatar382
Copy link
Collaborator

Also includes fixes for:
ANW-175
ANW-176

@avatar382
Copy link
Collaborator Author

Review note: The commit "MODS Export: export dates as per spec" was refactored and superseded by "MODS Export: Updates to date processing: set point, keyDate, and enco…"

@avatar382
Copy link
Collaborator Author

Just rebased with latest master as of 4/13.

@@ -35,7 +35,7 @@ def serialize_mods_inner(mods, xml)


xml.language {
xml.languageTerm(:type => 'code') {
Copy link
Contributor

Choose a reason for hiding this comment

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

@avatar382 @cdibella Do we need to have both the language code and language term exported in two different XML tags? As a reference, I am looking at lines 19-27 in the spreadsheet attached to ANW-451.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the intent is to have two different XML tags - one with a type "text" and the other with a type "code"

@@ -171,7 +171,6 @@ def handle_notes(notes)
def handle_extent(extents)
extents.each do |ext|
e = ext['number']
e << " (#{ext['portion']})" if ext['portion']
Copy link
Contributor

@lmcglohon lmcglohon Apr 17, 2018

Choose a reason for hiding this comment

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

@avatar382 @cdibella I may have missed a conversation, but not seeing the removal of this from the extents string described in the spreadsheet.

@@ -81,7 +81,7 @@ def serialize_mods_inner(mods, xml)
xml.temporal term
when 'uniform_title'
xml.titleInfo term
when 'genre_form', 'style_period'
when 'genre_form', 'style_period', 'technique'
Copy link
Contributor

Choose a reason for hiding this comment

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

@avatar382 @cdibella From spreadsheet line 231, should this also include 'function'?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, function should be in there too - all three map to the mods tag genre.

content = ASpaceExport::Utils.extract_note_text(note)
mods_note = case note['type']
when 'physdesc'
new_mods_note('note',
Copy link
Contributor

Choose a reason for hiding this comment

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

@avatar382 @cdibella From note on line 72 of spreadsheet, shouldn't both the physdesc and dimensions notes be wrapped in a physicalDescription tag? If so, I believe that is the last (5th) input parameter for the new_mods_note method.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, is the wrapper tag for both of these things

Copy link
Contributor

Choose a reason for hiding this comment

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

my tag disappeared, at least on the web page, but yup, physicalDescription is the wrapper

@avatar382
Copy link
Collaborator Author

I've fixed the language code and genre fixes. Looking at the physicalDescription one, the note tags are wrapped in a physicalDescription tag, but not individually.

Here's some sample output:
https://gist.github.com/avatar382/70a8c530fbe7760bfad12645e33c7191

Does this look right?

@lmcglohon
Copy link
Contributor

@avatar382 @cdibella I think having everything wrapped inside one physical description is correct - Christine?

@cdibella
Copy link
Contributor

cdibella commented Apr 20, 2018 via email

@lmcglohon lmcglohon merged commit 96d9243 into archivesspace:master Apr 23, 2018
@coveralls
Copy link

Coverage Status

Coverage increased (+80.6%) to 83.229% when pulling dc4ee46 on avatar382:ANW-451 into ee4e77c on archivesspace:master.

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

5 participants