Add rescue behavior for MLH API

[katjuell]

Description

In cases where we are not getting information returned from MLH, show dummy data instead.

Test process

Set faraday_connection to return an empty string, and test that data shows correctly on events page.

Requirements to merge

• [x] My code follows the style guidelines of this project
• [x] I have performed a self-review of my own code
• [ ] I have commented my code in hard-to-understand areas
• [x] My changes generate no new warnings
• [ ] I have added tests that prove my fix is effective or that my feature works
• [x] New and existing unit tests pass locally with my changes

[flyf-sh]

Failures:

  1) MlhTable the call to the API is unsuccessful creates a mock table object
     Failure/Error: expect(table).to have_key('data')

[katjuell]

@MattIPv4 when testing with both a malformed @api_url and faraday_connection set to an empty string (as well as both of those conditions separately), I don't see that error, and the sample events load. Perhaps I am missing something about the test process you used?

Agreed re: using the more generic error – that should cover the two cases of MLH behavior that we know about (and hopefully anything else that come up)

[MattIPv4]

When testing the placeholder handling within MlhTable, I removed the rescue on the helper, to ensure it was just the MlhTable logic that was touching the placeholder service.

If you walk through the logic, the error I saw appears to be what should happen (though wrong):

If MlhTable#faraday_connection returns the placeholder service (either set manually for testing, or through an API call failure), it will return the placeholder data, which is an array of events.

https://github.com/digitalocean/hacktoberfest/blob/8d5fbd844b588a1f4609e6301d0f2a1737043a5c/app/services/mlh_table.rb#L34

MlhTable#records uses this, and as the placeholder data is not a string, it will just return the placeholder data, an array of events.

https://github.com/digitalocean/hacktoberfest/blob/8d5fbd844b588a1f4609e6301d0f2a1737043a5c/app/services/mlh_table.rb#L42

PagesHelper#all_events calls MlhTable#records, which returns an array of events, and tries to access the data hash key. This errors!

https://github.com/digitalocean/hacktoberfest/blob/8d5fbd844b588a1f4609e6301d0f2a1737043a5c/app/helpers/pages_helper.rb#L5

Having MlhTable#faraday_connection return { 'data' => AirtablePlaceholderService.call('Meetups') } in the event of a call failure will match the expected data structure.

The parent rescue on the helper essentially hides this cascading failure, but we should fix not only the helper logic for if it fails, but also the API call directly.

[katjuell]

Makes sense to test without the rescue on the helper! And yeah, this intervention does make sense – I restructured the Meetups array in the placeholder service, and when we call it from various points in the app, the expectations on what is returned differ.

I just had to add a rescue in and a slight change to the conditional logic to get that suggested change to work, but it does now work for a non-200 response on the API call without the helper rescues in place.