fixes #7
fixes #7
Conversation
| # method should return hash or nil (not string) | ||
| def article_with_image | ||
| figure_matches = body.to_s.match(/<figure.*?>.*?(<img.*?>.*?<\/img>|<img.*?\/>).*?(.*?(<img.*?>.*?<\/img>|<img.*?\/>).*?)*<\/figure>/) |
There was a problem hiding this comment.
I've used regex since it allows to extract the first image only and takes less code for string parsing
There was a problem hiding this comment.
previous implementation returned string in case of issues - it was not handled on views and it is not the best practice to return hash and string types - or raising errors or nil.
| private | ||
|
|
||
| def posting_image_params(html) |
There was a problem hiding this comment.
- the issue in previous implementation was that if we pass here html with multiple images and some of attributes are missing for first image - then the response is inconsistent: some attr are for first image and missing ones are from next image
- also I've included bot '' and "" options for html values which was missing in previous implementation
There was a problem hiding this comment.
i've removed lambda since it only adds complexity to the code and is not needed
| # method should return hash or nil (not string) | ||
| def article_with_image | ||
| figure_matches = body.to_s.match(/<figure.*?>.*?(<img.*?>.*?<\/img>|<img.*?\/>).*?(.*?(<img.*?>.*?<\/img>|<img.*?\/>).*?)*<\/figure>/) |
There was a problem hiding this comment.
previous implementation returned string in case of issues - it was not handled on views and it is not the best practice to return hash and string types - or raising errors or nil.
| belongs_to :author, class_name: 'User', foreign_key: 'user_id' | ||
| belongs_to :editor, class_name: 'User', foreign_key: 'editor_id' | ||
|
|
||
| def article_with_image | ||
| return type if type != 'Article' |
There was a problem hiding this comment.
I've removed this checking and moved out article methods to appropriate class
| @@ -1,6 +1,6 @@ | |||
| -image = posting.article_with_image | |||
| -image = posting.respond_to?(:article_with_image) ? posting.article_with_image : nil | |||
There was a problem hiding this comment.
it fixes the issue with strings we had before: now we have nil or hash
| @@ -1,6 +1,6 @@ | |||
| -image = posting.article_with_image | |||
| -image = posting.respond_to?(:article_with_image) ? posting.article_with_image : nil | |||
| -if image | |||
| figure id="img_#{posting.id}" | |||
| img src="#{image['src']}" alt="#{image['alt'] || posting.title}" data-image="#{image['data-image']}" | |||
There was a problem hiding this comment.
we can also use symbol keys instead of string keys in the #article_with_image method response - it is better for ruby code performance
| => posting_snippet(posting) # uses HTML_Truncator gem and calls .html_safe on the output |
There was a problem hiding this comment.
posting_snippet helper is not implemented so it will not work, but we can implement it this way:
def posting_snippet(posting)
posting.body.to_s.html_safe
end| # method should return hash or nil (not string) | ||
| def article_with_image | ||
| figure_matches = body.to_s.match(/<figure.*?>.*?(<img.*?>.*?<\/img>|<img.*?\/>).*?(.*?(<img.*?>.*?<\/img>|<img.*?\/>).*?)*<\/figure>/) |
There was a problem hiding this comment.
I've converted body.to_s to fix issue with nil body values
| @@ -10,7 +10,7 @@ | |||
| "your feedback below<strong>!</strong></p>\r\n<figure><img src=\"https://images."\ | |||
| "peerspot.com/image/upload/c_limit,f_auto,q_auto,w_550/bvvrzbv97pp5srg612"\ | |||
| "le16pv99rg.jpg\" data-image=\"27c79574-7aa7-4eea-8515-d2a128698803.jpg\" alt=\"Spotlight"\ | |||
| " #3 - a community digest\"></figure>\r\n<p><strong><br></strong></p>\r\n<h2>Trending"\ | |||
| " #3 - a community digest\"/></figure>\r\n<p><strong><br></strong></p>\r\n<h2>Trending"\ | |||
There was a problem hiding this comment.
I've added correct ending of the tag '/>' (before it was just '>')
There was a problem hiding this comment.
btw i'm handling both tag style cases in article_with_image implementation:
No description provided.