Some fixes and improvements for the articles feature #1976
Conversation
Reference: CV2-5000.
Reference: CV2-5000.
…to an item. Reference: CV2-5000.
Reference: CV2-5000.
| @@ -153,7 +153,7 @@ def get_title | |||
| return self.send(title_mapping[title_field]).to_s | |||
| end | |||
| title = self.original_title | |||
| [self.analysis['file_title'], self.analysis['title'], self.fact_check_title, self.claim_description_content].each do |value| | |||
| [self.analysis['file_title'], self.analysis['title'], self.fact_check_title(true), self.claim_description_content].each do |value| | |||
There was a problem hiding this comment.
I think for self.fact_check_title(true) we should not call cached field with true to enforce regenerate cached field and make sure that cached field updated in relevant actions
There was a problem hiding this comment.
There was a race condition so I had to force that refresh... the event was triggering a call to this method, so it would fail.
| @@ -174,7 +174,7 @@ def media_slug | |||
| end | |||
|
|
|||
| def get_description | |||
| return self.fact_check_summary if self.get_main_channel == CheckChannels::ChannelCodes::FETCH | |||
| return self.fact_check_summary(true) if self.get_main_channel == CheckChannels::ChannelCodes::FETCH | |||
There was a problem hiding this comment.
I think for self.fact_check_summary(true) we should not call cached field with true to enforce regenerate cached field and make sure that cached field updated in relevant actions
There was a problem hiding this comment.
There was a race condition so I had to force that refresh... the event was triggering a call to this method, so it would fail.
app/models/team.rb
Outdated
| @@ -497,6 +497,9 @@ def filtered_explainers(filters = {}) | |||
| # Filter by text | |||
| query = query.where('(title ILIKE ? OR url ILIKE ? OR description ILIKE ?)', *["%#{filters[:text]}%"]*3) if filters[:text].to_s.size > 2 | |||
|
|
|||
| # Exclude the ones already applied to a target item | |||
| query = query.where.not(id: ProjectMedia.find(filters[:target_id].to_i).explainer_ids) unless ProjectMedia.find_by_id(filters[:target_id].to_i).nil? | |||
There was a problem hiding this comment.
I do not know if the ProjectMedia with filters[:target_id] exist this means that the query to find the ProjectMedia item will run twice one to check that item exists and the second one for the original query
if yes I think it's better to get the object then do our check
i.e
pm = ProjectMedia.find_by_id(filters[:target_id].to_i)
query = query.where.not(id: pm.explainer_ids) unless pm.nil?
app/models/team.rb
Outdated
| @@ -533,6 +536,9 @@ def filtered_fact_checks(filters = {}) | |||
| # Filter by text | |||
| query = query.where('(fact_checks.title ILIKE ? OR fact_checks.url ILIKE ? OR fact_checks.summary ILIKE ?)', *["%#{filters[:text]}%"]*3) if filters[:text].to_s.size > 2 | |||
|
|
|||
| # Exclude the ones already applied to a target item | |||
| query = query.where.not('fact_checks.id' => ProjectMedia.find(filters[:target_id].to_i).fact_check_id) unless ProjectMedia.find_by_id(filters[:target_id].to_i).nil? | |||
There was a problem hiding this comment.
if the query run twice in case the item exists then I suggest to do the following:
pm = ProjectMedia.find_by_id(filters[:target_id].to_i)
query = query.where.not('fact_checks.id' => pm.fact_check_id) unless pm.nil?
|
@melsawy, thanks for your comments - please re-review. |
Description
Reference: CV2-5000.
Checklist