Instant Article API publishing

theonion / django-bulbs

DEPRECATED: This project is now part of the Mono Repo (https://github.com/theonion/omni)

MIT License

26 stars 7 forks source link

Instant Article API publishing #209

Closed MichaelButkovic closed 8 years ago

MichaelButkovic commented 8 years ago

Integration with FB API

mparent61 commented 8 years ago

Looking good so far

MichaelButkovic commented 8 years ago

@mparent61 @benghaziboy @spra85 ready for review

camsom commented 8 years ago

👍

MichaelButkovic commented 8 years ago

@spra85 @mparent61 made updates based on feedback

mparent61 commented 8 years ago

Remaining concern is that much of the FB API interaction is not covered in local/test environments, and only loosely covered by unit tests.

At the very least I would make sure that the settings access + template rendering is executed before the environment check, so that settings + template errors can be caught earlier in testing.

What abou using VCR to record single IA article create + delete interactions, which could then be replayed via tests?

MichaelButkovic commented 8 years ago

I'd be fine using VCR, but the requests mocks are already sending back the exact payloads from the API.

MichaelButkovic commented 8 years ago

@mparent61 more updates

mparent61 commented 8 years ago

Yeah, the API responses are simple enough that VCR probably is overkill. VCR would potentially make maintenance easier in the future (auto-regenerate mocks), but we can punt on it.

The refactoring you just did to always render even in non-prod seems like the low-hanging fruit here.

mparent61 commented 8 years ago

👍

spra85 commented 8 years ago

👍

coveralls commented 8 years ago

Coverage decreased (-0.08%) to 86.155% when pulling 23e26d9a93eb3c9660dd2d0e220320b41d210532 on ia-publishing into 3621edb26f7c4e05de259dc0cfd1a526533989d4 on master.