5514 – Import fact-check as published report

[vasconsaurus]

Description

Context

While trying to run a fact check import Jessie noted that the fact checks weren't imported as published.
I looked at some older imports and seems to have been the case for a while even though we set fc.publish_report = true in the FetchBot.

What

We were updating the published value for Dynamic but weren't for FactCheck. We hadn't implemented that as part of the big articles work.

So we just need to make sure both are updated as published when importing through the FetchBot.

References: 5390, 5514

How has this been tested?

rails test test/models/bot/fetch_test.rb:257

Things to pay attention to during code review

Please describe parts of the change that require extra attention during code review, for example:

• File FFFF, line LL: This refactoring does this and this. Is it consistent with how it’s implemented elsewhere?
• Etc.

Checklist

• I have performed a self-review of my own code
• I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing
• I have added regression tests, if the PR fixes a bug
• I have added logging, exception reporting, and custom tracing with any additional information required for debugging
• I considered secure coding practices when writing this code. Any security concerns are noted above.
• I have commented my code in hard-to-understand areas, if any
• I have made needed changes to the README
• My changes generate no new warnings
• If I added a third party module, I included a rationale for doing so and followed our current guidelines

[vasconsaurus]

@caiosba @melsawy could you take a look?

Made a draft PR so it's easier to discuss.

[caiosba]

thanks @vasconsaurus - in theory, publish_report = true should still work, but probably some other change would be required... this was probably a regression from all the articles work (AFAIR, we didn't test manually-importing things with our script). But if tests pass and importing works, this change LGTM - it's consistent with the data model.

[vasconsaurus]

@caiosba It should work, that is why I'm unsure on this one.. I'll do some more poking around today and see what I can find. Yesterday I did stick a byebug inside update_report, and it seemed to not get to it when running publish_report = true, it did when I changed it.

Anyway! Will double check and come back.

[vasconsaurus]

@caiosba still will look some more into this after lunch, but for now:

It seems Dynamic is updated correctly to published, but the FactCheck.report_status is never changed.

(byebug) Dynamic.last
#<Dynamic id: 272, annotation_type: "report_design", ... , data: {"options"=>{ ... "state"=>"published"}, ... >
(byebug) FactCheck.last
#<FactCheck ... report_status: "unpublished"... >

Also, we set unpublished_state to unpublished and data[:state] to published. After setting data[:state] to published should unpublished_state still remain as unpublished?

check-api/app/models/fact_check.rb

Lines 125 to 126 in 04e2897

	unpublished_state = data[:state].blank? ? 'unpublished' : 'paused'
	data[:state] = (self.publish_report ? 'published' : unpublished_state) if data[:state].blank? || !self.publish_report.nil?

[caiosba]

Ah, that actually makes sense, @vasconsaurus ... we simply haven't implemented that as part of the big articles work. I think we're good as far as debugging is concerned. Just one question... I don't know if the solution is...

(1) Replace the current line fc.publish_report = true by fc.report_status = "published" like you did

(2) In addition to the current line fc.publish_report = true, add a line fc.report_status = "published"

At the end of the day, we need to be sure that both the fact-check and the report have the status "published".

[caiosba]

Thanks, @vasconsaurus ! Did the test fail before the fix?

[vasconsaurus]

@caiosba

[caiosba]

@caiosba

AMAZING, TDD FTW