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

Save facts as array #37

Merged
merged 1 commit into from
Oct 20, 2016

Conversation

berniechiu
Copy link
Contributor

@berniechiu berniechiu commented Oct 14, 2016

Reference: #32

Features

(1) Parse facts as array object and save to DB
(2) I think rerun the migrations and parse the new data can see the result

Copy link
Collaborator

@AfolabiOlaoluwa AfolabiOlaoluwa left a comment

Choose a reason for hiding this comment

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

Yes, you are cool. Calling Ruby .inject([ ]) clause on lists and append that to result and returning the result will work.

@x6iae
Copy link
Member

x6iae commented Oct 15, 2016

👍 lgtm...

@@ -55,6 +55,10 @@ def collect_data(disease_link)
end

def facts(disease_page)
disease_page.at('h3:contains("Key facts")').try(:next_element).try(:text)
parsed_lists = disease_page.at('h3:contains("Key facts")').try(:next_element)
if parsed_lists && (lists = parsed_lists.search("li")).present?
Copy link
Member

@x6iae x6iae Oct 16, 2016

Choose a reason for hiding this comment

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

What happens when (lists = parsed_lists.search("li")).present? returns false?
I think we can return parsed_lists in that case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That looks like, on parsed_lists it's searches for presence of ("li"), and you are calling the result you had on parsed_lists straight to lists, And you are returning the ones that are false

parsed_lists will return true (only ones with "li"). Probably you can negate it to return false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if false, then the result will not be set. So it becomes nil in this case. It is fine when a serialized Array column with nil is set since it will return an empty arrary.

Copy link
Member

Choose a reason for hiding this comment

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

Alright thanks... merging away

@x6iae x6iae merged commit f8a9714 into devcenter-square:develop Oct 20, 2016
@osioke
Copy link
Contributor

osioke commented Oct 27, 2016

@SundayAdefila did this fix #32

cc: @berniechiu @AfolabiOlaoluwa

@x6iae
Copy link
Member

x6iae commented Oct 27, 2016

Not been able to check because of issue #40 😢

@x6iae
Copy link
Member

x6iae commented Oct 31, 2016

Update:
this has taken care of facts for #32. now remaining prevention.

👍 @berniechiu

cc: @AfolabiOlaoluwa @osioke

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