add <tags> to goobi notify call with object tags in specified format #139
Conversation
peetucket
force-pushed
the
send-tags-to-goobi
branch
from
7aebb0f
to
d0010d3
Compare
app/models/dor/goobi.rb
Outdated
| @@ -31,6 +31,9 @@ def xml_request | |||
| <sdrWorkflow>#{Dor::Config.goobi.dpg_workflow_name}</sdrWorkflow> | |||
| <goobiWorkflow>#{goobi_workflow_name}</goobiWorkflow> | |||
| <ocr>#{goobi_ocr_tag_present?}</ocr> | |||
| <tags> | |||
There was a problem hiding this comment.
maybe extract this into a method that just generates the <tags> element? That would make this method less complex and easier to test the behavior.
app/models/dor/service_item.rb
Outdated
| # @return [Array] | ||
| def goobi_tag_list | ||
| @druid_obj.tags.map do |tag| | ||
| tag_split = tag.split(':', 2).map(&:strip) # only split on the first colon |
There was a problem hiding this comment.
How about instantiating a Tag object that responds to name and value?
There was a problem hiding this comment.
So create a new tag object in the "Dor" namespace we already have setup and use it here?
There was a problem hiding this comment.
Yes, It could even be a nested class in ServiceItem, if we aren't going to use it externally.
There was a problem hiding this comment.
Wasn't sure what you meant about nesting inside of ServiceItem, but did an experiment with a standalone GoobiTag class here: #140
app/models/dor/service_item.rb
Outdated
|
|
||
| # returns an array of arrays, each element contains an array of [name, value] of DOR object tags in the format expected to pass to Goobi | ||
| # the name of the tag is the first namespace part of the tag (before first colon), value of the tag is everything after this | ||
| # @return [Array] |
There was a problem hiding this comment.
Please specify what the Array contains.
There was a problem hiding this comment.
Described current behavior, but if we move to a new Tag object, this would change the return structure to an array of Tags...do you think its worth creating a new Tag class for this use (agree the array of array is not ideal, but the name of a tag can be repeated as we define it, so we can't use a hash)
spec/goobi_spec.rb
Outdated
|
|
||
| it 'creates the correct xml request with no tags present' do | ||
| allow(item).to receive(:tags).and_return([]) | ||
| expect(@goobi.xml_request).to be_equivalent_to <<-END |
There was a problem hiding this comment.
this test might be easier to read if we do
...to include('<tags></tags>')
spec/service_item_spec.rb
Outdated
| @@ -42,6 +42,27 @@ | |||
| end | |||
| end | |||
|
|
|||
| describe '.goobi_tag_list' do | |||
There was a problem hiding this comment.
typically instance methods would be described as #goobi_tag_list
There was a problem hiding this comment.
ok, will update the rest of the describe blocks in this spec too
peetucket
force-pushed
the
send-tags-to-goobi
branch
from
3c56375
to
bfc91ff
Compare
peetucket
force-pushed
the
send-tags-to-goobi
branch
from
bfc91ff
to
88f3122
Compare
|
Thanks @peetucket. This looks good. It's still |
|
Yeah, still WIP -- waiting for some additional stakeholder verification/clarification. |
peetucket
changed the title
|
No longer WIP - we can move forward with this |
fixes #138