Tim/parature faq feed

After scanning the code and not digging too deeply into the implementation, I have some feedback:

- The importer shouldn't know if it's running in a spec or not. i.e. the `@test` instance variable shouldn't exist. You need to pass the path of an XML file to the importer when you initialize it in the spec. From there, the importer should read from the given file rather than from the default source URL. It should act in much the same way as if it was actually running in production. The spec’s job is to cause the importer to behave as if it was actually importing live data, but in a controlled environment in which its output can be observed and verified to be correct. If you think about it, having the @test flag in place and doing different things in testing and production doesn’t really test the code, because it will executing one block of code when the spec is run, but another when run in production.

- The code appears to convert an XML node into a Ruby hash, then into a JSON string. It then modifies the JSON string before finally converting it back to a Ruby hash. You probably don’t need to convert this to JSON at any stage. The goal is to convert XML to a Ruby hash, so the JSON stuff seems unnecessary. I think you should look at how other importers use the nokogiri gem to extract data from XML. This could be useful here.

- On line 32 you set `article_count = 379`. If in the future there were articles with an ID greater than 379, I guess the importer wouldn’t import them. Is there a way to write the importer such that it can successfully import all articles in the case where we don’t know the highest ID value? Given that we have to “hit and hope” article URLs, I understand that this may be tricky or even impossible.

I appreciate how difficult this source is to import. It’s way more difficult to deal with than any other I’d seen. The fact that you’ve got the importer to successfully fetch all necessary data is an achievement in itself!