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

create pssg download job #3438

Merged
merged 12 commits into from
Oct 17, 2019
Merged

create pssg download job #3438

merged 12 commits into from
Oct 17, 2019

Conversation

MrBilnon
Copy link
Contributor

Description of change

Creates the PSSG download job

https://github.com/department-of-veterans-affairs/vets-contrib/issues/3103

Testing done

Created rspec for testing how it would interact with data coming from the drive time band client

Acceptance Criteria (Definition of Done)

Applies to all PRs

  • Appropriate logging
  • Provide link to originating GitHub issue, or connected to it via ZenHub
  • Does not contain any sensitive information (i.e. PII/credentials/internal URLs/etc., in logging, hardcoded, or in specs)

@va-vfs-bot
Copy link

1 Warning
⚠️ PR is exceeds 250 LoC. Consider breaking up into multiple smaller ones.

Generated by 🚫 Danger

Copy link
Contributor

@omgitsbillryan omgitsbillryan left a comment

Choose a reason for hiding this comment

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

LGTM.

You can ignore the "Danger" message. It's a new feature that will need tweaking... like ignoring fixture files that would easily bump the max LoC.

Comment on lines 20 to 39
vha_id = extract_vha_id(attributes)
facility = BaseFacility.find_facility_by_id(vha_id)
return if facility.nil?

drive_time_band = facility.drivetime_bands.find_or_initialize_by(vha_facility_id: extract_name(attributes))
drive_time_band.min = attributes.dig('FromBreak')
drive_time_band.max = attributes.dig('ToBreak')
drive_time_band.name = attributes.dig('Name')
drive_time_band.polygon = extract_polygon(drive_time_data)
facility.save
end

def extract_name(attributes)
name = attributes.dig('Name')
name.partition(':').first.strip!
end

def extract_vha_id(attributes)
'vha_' + extract_name(attributes)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

You could get rid of def extract_vha_id and reduce the calls to extract_name from 2 down to 1 with something like this:

name = extract_name(attributes)
vha_id = 'vha_' + name
#...
drive_time_band = facility.drivetime_bands.find_or_initialize_by(vha_facility_id: name)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I added some exception handling but not sure if it's pragmatically correct in ruby

@va-vfs-bot va-vfs-bot temporarily deployed to wh/pssg_download_job/master October 17, 2019 14:53 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to wh/pssg_download_job/master October 17, 2019 15:02 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to wh/pssg_download_job/master October 17, 2019 15:21 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to wh/pssg_download_job/master October 17, 2019 16:02 Inactive
Copy link
Contributor

@edmkitty edmkitty left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

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.

4 participants