Skip to content
This repository has been archived by the owner on Mar 24, 2020. It is now read-only.

Fixes - #270 Implemented mapping from DAMS4 to SHARE V2 API and updat… #460

Merged
merged 1 commit into from
Jun 11, 2018

Conversation

hweng
Copy link
Contributor

@hweng hweng commented Jun 7, 2018

Fixes #270

Local Checklist

  • Tests written and passing locally?
  • Code style checked?
  • QA-ed locally?
  • Rebased with master branch?
  • Configuration updated (if needed)?
  • Documentation updated (if needed)?

What does this PR do?

  1. Added support for mapping from DAMS4 to SHARE Version 2 schema.
  2. Updated push function due to SHARE Version 2 API changes.
  3. Refactored old SHARE mapping code to conform to Rubocop suggested expression, syntax and formatting.

@ucsdlib/developers - please review

'tags': osf_mads_fields(document),
'extra': osf_extra(document)
}
json_data = {"jsonData": field_map}

Choose a reason for hiding this comment

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

Lint/UselessAssignment: Useless assignment to variable - json_data.
Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

}
json_data = {"jsonData": field_map}
end
def export_to_API(document)

Choose a reason for hiding this comment

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

Naming/MethodName: Use snake_case for method names.

dams_data.each do |datum|
osf_data << datum
end
end

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

if dams_data.kind_of?(String)
osf_data << dams_data
elsif dams_data.kind_of?(Array)
dams_data.each do |datum|

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

dams_data = document["#{field_name}"]
if dams_data.kind_of?(String)
osf_data << dams_data
elsif dams_data.kind_of?(Array)

Choose a reason for hiding this comment

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

Style/ClassCheck: Prefer Object#is_a? over Object#kind_of?.

end
osf_data
end
def osf_mads_fields(document)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [27/15]

end
osf_data
end
osf_data

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

langs.each do |lang|
osf_data << lang
end
langs.each do |lang|

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

dams_data = document["#{field_name}"]
def osf_languages(document)
field_name = "language_tesim"
dams_data = document["#{field_name}"]

Choose a reason for hiding this comment

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

Style/UnneededInterpolation: Prefer to_s over string interpolation.

field_name = "language_tesim"
dams_data = document["#{field_name}"]
def osf_languages(document)
field_name = "language_tesim"

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

'tags': osf_mads_fields(document),
'extra': osf_extra(document)
}
{'jsonData': field_map}

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

'languages': osf_languages(document),
'date_published': osf_date_published(document),
'tags': osf_mads_fields(document),
'extra': osf_extra(document)

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

json_data = {"jsonData": field_map}
end
def export_to_api(document)
field_map = {

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

mads_field ||= YAML.load(ERB.new(IO.read(File.join(Rails.root, 'config', 'mads_field.yml'))).result)
field_names = mads_field.fetch('key')
field_names.each do |field_name|
dams_data = document["#{field_name}"]

Choose a reason for hiding this comment

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

Style/UnneededInterpolation: Prefer to_s over string interpolation.

end
def osf_mads_fields(document)
osf_data = []
mads_field ||= YAML.load(ERB.new(IO.read(File.join(Rails.root, 'config', 'mads_field.yml'))).result)

Choose a reason for hiding this comment

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

Security/YAMLLoad: Prefer using YAML.safe_load over YAML.load.
Rails/FilePath: Please use Rails.root.join('path', 'to') instead.


def osf_date_published(document)
field_name = 'date_json_tesim'
dams_data = document["#{field_name}"]

Choose a reason for hiding this comment

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

Style/UnneededInterpolation: Prefer to_s over string interpolation.


if dams_data != nil
url = "http://library.ucsd.edu/dc/collection/#{dams_data}"
osf_data = {'canonicalUri': url, 'providerUris': url}

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

dams_data = document["#{field_name}"]
osf_data = {}

if dams_data != nil

Choose a reason for hiding this comment

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

Style/NonNilCheck: Prefer !expression.nil? over expression != nil.


def osf_uris(document)
field_name = 'id'
dams_data = document["#{field_name}"]

Choose a reason for hiding this comment

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

Style/UnneededInterpolation: Prefer to_s over string interpolation.

end
end
osf_data
end

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

'tags': osf_mads_fields(document),
'extra': osf_extra(document)
}
{'jsonData': field_map}

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

mads_field ||= YAML.safe_load(ERB.new(IO.read(Rails.root.join('config', 'mads_field.yml'))).result)
field_names = mads_field.fetch('key')
field_names.each do |field_name|
dams_data = document["#{field_name}"]

Choose a reason for hiding this comment

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

Style/UnneededInterpolation: Prefer to_s over string interpolation.

dams_data = document['date_json_tesim']
osf_data = ''

if !dams_data.nil?

Choose a reason for hiding this comment

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

Style/NegatedIf: Favor unless over if for negative conditions.

def osf_date_published(document)
dams_data = document['date_json_tesim']
osf_data = ''

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

dams_data = document['id']
osf_data = {}

if !dams_data.nil?

Choose a reason for hiding this comment

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

Style/NegatedIf: Favor unless over if for negative conditions.

osf_data = (osf_data.is_a?(Time) || osf_data.is_a?(DateTime)) ? osf_data : Time.now
end
def agent_type(type)
type = 'principalinvestigator' if type == 'principal investigator' || type == 'Principal Investigator'

Choose a reason for hiding this comment

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

Style/MultipleComparison: Avoid comparing a variable with multiple items in a conditional, use Array#include? instead.

end
end
end
osf_data << {agent_type: 'Publisher', type: 'Organization', 'name': 'UC San Diego Library Digital Collections'}

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

relationships = JSON.parse(datum)
relationships.each do |key, value|
value.each do |v|
osf_data << {agent_type: agent_type(key), type: 'Person', 'name': v}

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

dams_data = document['relationship_json_tesim']
osf_data =[]

if !dams_data.nil?

Choose a reason for hiding this comment

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

Style/NegatedIf: Favor unless over if for negative conditions.

end
def osf_related_agents(document)
dams_data = document['relationship_json_tesim']
osf_data =[]

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator =.

dams_data.each do |datum|
date = JSON.parse(datum)
if date['type'] == 'issued'
d_date = date['beginDate']|| ''

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator ||.

end
end
end
osf_data = (osf_data.blank?) ? osf_data << { 'name': 'UC San Diego Library' } : osf_data

Choose a reason for hiding this comment

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

Style/RedundantParentheses: Don't use parentheses around a method call.
Style/TernaryParentheses: Omit parentheses for ternary conditions.

relationships = JSON.parse(datum)
relationships.each do |key, value|
value.each do |v|
osf_data << {"name": v}

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

unless dams_data.nil?
dams_data.each do |datum|
relationships = JSON.parse(datum)
relationships.each do |key, value|

Choose a reason for hiding this comment

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

Lint/UnusedBlockArgument: Unused block argument - key. If it's necessary, use _ or _key as an argument name to indicate that it won't be used.

end
def osf_contributors(document)
dams_data = document['relationship_json_tesim']
osf_data =[]

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator =.

dams_data.each do |datum|
title = JSON.parse(datum)
osf_data = title['name'] ? title['name'] : ''
osf_data += title['name'] && !title['translationVariant'].blank? ? ' : ' : ''

Choose a reason for hiding this comment

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

Rails/Present: Use title['translationVariant'].present? instead of !title['translationVariant'].blank?.

end
def osf_title(document)
dams_data= document['title_json_tesim']
osf_data=''

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator =.

osf_data
end
def osf_title(document)
dams_data= document['title_json_tesim']

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator =.

end
osf_data
end
def osf_title(document)

Choose a reason for hiding this comment

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

Metrics/CyclomaticComplexity: Cyclomatic complexity for osf_title is too high. [8/7]
Metrics/MethodLength: Method has too many lines. [18/15]

@@ -224,8 +224,15 @@ def osf_delete
def osf_push
@document = get_single_doc_via_search(1, {:q => "id:#{params[:id]}"} )
authorize! :show, @document
document = ShareNotify::PushDocument.new("http://library.ucsd.edu/dc/collection/#{params[:id]}", osf_date(@document))
document = ShareNotify::PushDocument.new("http://library.ucsd.edu/dc/collection/#{params[:id]}")
document.type = "DataSet" if @document["unit_code_tesim"].first == "rdcp"

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

@coveralls
Copy link

coveralls commented Jun 7, 2018

Coverage Status

Coverage decreased (-0.2%) to 65.893% when pulling c9e0605 on feature/SHAREV2_mapping into dfa8b3e on develop.

date = JSON.parse(datum)
if date['type'] == 'issued'
d_date = date['beginDate'] || ''
osf_data = DateTime.new(d_date.to_i,1,1) if d_date.match( '^\d{4}$' )

Choose a reason for hiding this comment

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

Style/DateTime: Prefer Date or Time over DateTime.
Rails/TimeZone: Do not use DateTime.new without zone. Use one of Time.zone.local, DateTime.current, DateTime.new.in_time_zone, DateTime.new.utc, DateTime.new.getlocal, DateTime.new.iso8601, DateTime.new.jisx0301, DateTime.new.rfc3339, DateTime.new.to_i, DateTime.new.to_f instead.
Layout/SpaceAfterComma: Space missing after comma.
Performance/RedundantMatch: Use =~ in places where the MatchData returned by #match will not be used.
Layout/SpaceInsideParens: Space inside parentheses detected.

type = 'principalinvestigator' if ['principal investigator', 'Principal Investigator'].include? type
agent_dataset ||= YAML.safe_load(ERB.new(IO.read(Rails.root.join('config', 'share_agent_type.yml'))).result)
share_agent_type = agent_dataset.fetch('key')
type = (share_agent_type.include? type) ? type : 'Contributor'

Choose a reason for hiding this comment

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

Lint/UselessAssignment: Useless assignment to variable - type.
Style/TernaryParentheses: Omit parentheses for ternary conditions.

end
end
end
osf_data = osf_data.blank? ? osf_data << { 'name': 'UC San Diego Library' } : osf_data

Choose a reason for hiding this comment

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

Rails/Presence: Use osf_data.presence || osf_data << { 'name': 'UC San Diego Library' } instead of osf_data.blank? ? osf_data << { 'name': 'UC San Diego Library' } : osf_data.

end
end
end
osf_data = (osf_data.is_a?(Time) || osf_data.is_a?(DateTime)) ? osf_data : Time.now

Choose a reason for hiding this comment

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

Style/TernaryParentheses: Omit parentheses for ternary conditions.
Rails/TimeZone: Do not use Time.now without zone. Use one of Time.zone.now, Time.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead.

date = JSON.parse(datum)
if date['type'] == 'issued'
d_date = date['beginDate'] || ''
osf_data = Time.utc(d_date.to_i, 1, 1) if d_date.match('^\d{4}$')

Choose a reason for hiding this comment

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

Performance/RedundantMatch: Use =~ in places where the MatchData returned by #match will not be used.

def agent_type(type)
type = 'principalinvestigator' if ['principal investigator', 'Principal Investigator'].include? type
agent_dataset ||= YAML.safe_load(ERB.new(IO.read(Rails.root.join('config', 'share_agent_type.yml'))).result)
(agent_dataset.fetch('key').include? type) ? type : 'Contributor'

Choose a reason for hiding this comment

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

Style/TernaryParentheses: Omit parentheses for ternary conditions.

end
end
end
osf_data = osf_data.presence || osf_data << { 'name': 'UC San Diego Library' }

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

…ed push function.

Refactor code to pass rubocop sytle check.

Refactor the mapping code to use Rails.root.join to fetch config file.

Refactor code to Array#include? for comparison and use to_s over string interpolation

Refactored  argument name and remove all trailing whitespace

Refactor to use Time instead of DateTime and use Rails/Presence

Refactor the regex and reduce the title method Complexity

refactor for Style/TernaryParentheses

Fixing failed tests.
@hweng
Copy link
Contributor Author

hweng commented Jun 8, 2018

@mcritchlow I moved to this this PR as there are fair amount of refactoring for old code according to Rubocop. And the agent_type, mads_type was moved to config file as you suggested.

@hweng
Copy link
Contributor Author

hweng commented Jun 8, 2018

@ucsdlib/developers - All Hound Rubocop check passed, please review. Thanks!

Copy link
Member

@mcritchlow mcritchlow left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@VivianChu
Copy link
Member

👍

1 similar comment
@lsitu
Copy link
Member

lsitu commented Jun 11, 2018

👍

@lsitu lsitu merged commit 7008f85 into develop Jun 11, 2018
@lsitu lsitu deleted the feature/SHAREV2_mapping branch June 11, 2018 16:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants