-
Notifications
You must be signed in to change notification settings - Fork 2
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
set second indicator depending on the objects APO #68
Conversation
Coverage increased (+0.03%) to 98.599% when pulling 64d9569f931e5466ebc3f420f77caef97162cf63 on indicator2-for-etd-eems into 94da1b4 on master. |
64d9569
to
ea0fe97
Compare
spec/update_marc_spec.rb
Outdated
@@ -199,11 +199,30 @@ | |||
end | |||
|
|||
describe '.get_2nd_indicator' do | |||
it 'should return 1' do | |||
it 'should return 1 for a non dorn digital APO' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: dorn -> born
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end | ||
end | ||
|
||
def extract_druid(druid) | ||
druid.match(/druid(:[a-z]{2})(\d{3})([a-z]{2})(\d{4})$/i).to_s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely we already have logic to do this.. somewhere.. right? Either in dor-services, druid-tools, or up in active-fedora.
Or, maybe it's time to finally get sul-dlss-deprecated/dor-services#263 in, so we aren't re-inventing our own tools for managing relationships?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, probably, just extracted since it was better than what was happening before and was going to use it twice...but if use admin_policy_object_id than it is not necessary to change this method and i can go back
Coverage increased (+0.03%) to 98.599% when pulling ea0fe973643171f0dddca34880be77b117039ef5 on indicator2-for-etd-eems into 94da1b4 on master. |
1 similar comment
Coverage increased (+0.03%) to 98.599% when pulling ea0fe973643171f0dddca34880be77b117039ef5 on indicator2-for-etd-eems into 94da1b4 on master. |
@@ -118,6 +121,10 @@ def get_x2_constituent_info | |||
end.join('') | |||
end | |||
|
|||
def born_digital? | |||
@druid_obj.relationships(:is_governed_by).map { |apo| BORN_DIGITAL_APOS.include? extract_druid(apo) }.include? true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#relationships
is a pretty low-level method. Can we use e.g. #admin_policy_object
(or maybe #admin_policy_object_id
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup will use admin_policy_object_id
ea0fe97
to
84ffa7b
Compare
Coverage increased (+0.03%) to 98.599% when pulling 84ffa7b4e96bb64ab5d5c1bd750d9a6668460abb on indicator2-for-etd-eems into 94da1b4 on master. |
1 similar comment
Coverage increased (+0.03%) to 98.599% when pulling 84ffa7b4e96bb64ab5d5c1bd750d9a6668460abb on indicator2-for-etd-eems into 94da1b4 on master. |
84ffa7b
to
6e0dcd2
Compare
Now using admin_policy_object_id to check for apo to avoid using relationships. Leaving all else alone. |
fixes #67