allow specifying request headers#54
Conversation
darkphnx
left a comment
darkphnx
left a comment
There was a problem hiding this comment.
I've made a few minor suggestions for your implementation. Pretty good for a first Ruby PR!
I think the one thing that's missing is some tests in spec/calendar_spec.rb to demonstrate that the headers are being passed to the URI#open method. Let me know if you'd like some help in writing that test.
lib/ical_filter_proxy/calendar.rb
Outdated
| attr_accessor :ical_url, :api_key, :timezone, :filter_rules, :clear_existing_alarms, :alarm_triggers, :request_headers | ||
|
|
||
| def initialize(ical_url, api_key, timezone = 'UTC') | ||
| def initialize(ical_url, api_key, timezone = 'UTC', request_headers={}) |
There was a problem hiding this comment.
| def initialize(ical_url, api_key, timezone = 'UTC', request_headers={}) | |
| def initialize(ical_url, api_key, timezone = 'UTC', request_headers = {}) |
lib/ical_filter_proxy/calendar.rb
Outdated
| self.ical_url = ical_url | ||
| self.api_key = api_key | ||
| self.timezone = timezone | ||
| self.request_headers=request_headers |
There was a problem hiding this comment.
| self.request_headers=request_headers | |
| self.request_headers = request_headers |
lib/ical_filter_proxy/calendar.rb
Outdated
|
|
||
| def raw_original_ical | ||
| URI.open(ical_url).read | ||
| URI.open(ical_url, request_headers&.reduce({}, :merge) || {}).read |
There was a problem hiding this comment.
I'm not too sure what you're trying to do with the reduce here.
Since request headers is already a hash you shouldn't need to reconstruct it, instead you should be able to pass it directly such as:
URI.open(ical_url, request_headers || {}).read
|
Hi, I do not know exactly How I'd write a test to check that URI#open receives the request_headers correctly. My intuition would be to mock the method but that seems a bit to complicated for such a small thing. |
2 participants
This pr allows for configurable request headers, when getting the original ical. I wrote this because Microsoft owa now does some User-Agent Filtering. Please note that this is my first ever ruby Pull Request.