-
Notifications
You must be signed in to change notification settings - Fork 13
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
FYST-1081 MD Populate exception code for resubmission #5687
FYST-1081 MD Populate exception code for resubmission #5687
Conversation
Heroku app: https://gyr-review-app-5687-f280bac51570.herokuapp.com/ |
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.
do we need to include this on the xml? seeing on the story:
XML field ExceptionCodes
@@ -38,7 +38,7 @@ def hash_for_pdf | |||
|
|||
'Maryland Physical Address Line 1 Street No and Street Name No PO Box': @xml_document.at('MarylandAddress AddressLine1Txt')&.text, | |||
'Maryland Physical Address Line 2 Apt No Suite No Floor No No PO Box': @xml_document.at('MarylandAddress AddressLine2Txt')&.text, | |||
'City': @xml_document.at('MarylandAddress CityNm')&.text, | |||
City: @xml_document.at('MarylandAddress CityNm')&.text, |
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.
why do you remove the quotes from ‘City’ 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.
hmm, maybe the symbol is enough and it doesn’t matter because the same is happening below as well.
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.
i'm assuming this is a rubymine suggestion -- we don't need quotes around single-word strings (and I guess something like primary_pension
further down is also considered to be a single word)
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.
I just have a clarifying question, otherwise this looks good! Thanks @tahsinaislam!
Link to pivotal/JIRA issue
https://codeforamerica.atlassian.net/browse/FYST-1081
Is PM acceptance required? (delete one)
What was done?
How to test?
Screenshots (for visual changes)