-
Notifications
You must be signed in to change notification settings - Fork 27
Incremental activities #36
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
module HackerOne | ||
module Client | ||
module Incremental | ||
class Activities | ||
include ResourceHelper | ||
|
||
DOTFILE = '.hackerone_client_incremental_activities'.freeze | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is your plan to store the latest fetched thing in this file? If so, that isn't going to work if you have more than one machine that uses this lib and I would let this up to the user how to store this data. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, that's the plan! We need some kind of persistent storage to store the I think you have a point, let's remove this logic? |
||
|
||
attr_reader :program, :updated_at_after, :page_size | ||
|
||
def initialize(program, updated_at_after: nil, page_size: 25) | ||
@program = program | ||
@updated_at_after = updated_at_after | ||
@page_size = page_size | ||
end | ||
|
||
def loop_through_activities | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😎 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. flatmap There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @oreoshake wow, you're so cool |
||
load_dotfile | ||
|
||
loop do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't |
||
fetch_current_page | ||
|
||
activities.each do |activity_json| | ||
activity = HackerOne::Client::Activities | ||
.build(activity_json) | ||
yield activity | ||
end | ||
|
||
break if next_page.nil? | ||
end | ||
end | ||
|
||
def activities | ||
current_page[:data] | ||
end | ||
|
||
def next_page? | ||
next_updated_at_after.present? | ||
end | ||
|
||
def next_page | ||
return nil unless next_page? | ||
|
||
@updated_at_after = next_updated_at_after | ||
fetch_current_page | ||
end | ||
|
||
def load_dotfile | ||
return nil unless File.exist?(dotfile_filepath) | ||
dotfile_content = JSON.parse( | ||
File.read(dotfile_filepath) | ||
) | ||
|
||
return nil unless dotfile_content.key?(program.handle) | ||
|
||
@updated_at_after = dotfile_content | ||
.fetch(program.handle) | ||
.fetch('updated_at_after') | ||
end | ||
|
||
def store_dotfile | ||
new_dotfile_content = { | ||
program.handle => { | ||
updated_at_after: updated_at_after | ||
} | ||
} | ||
File.open(dotfile_filepath, 'w') do |f| | ||
f.puts(JSON.pretty_generate(new_dotfile_content)) | ||
end | ||
end | ||
|
||
private | ||
|
||
def fetch_current_page | ||
@current_page = nil | ||
current_page | ||
end | ||
|
||
def current_page | ||
@current_page ||= make_get_request( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think avoiding memoization here can clean up some of the state introduced by the ivars. |
||
'incremental/activities', | ||
extract_data: false, | ||
params: { | ||
handle: program.handle, | ||
first: page_size, | ||
updated_at_after: updated_at_after | ||
} | ||
) | ||
end | ||
|
||
def dotfile_filepath | ||
File.join(Dir.home, DOTFILE) | ||
end | ||
|
||
def next_updated_at_after | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method name is a little hard to grok. |
||
current_page[:meta][:max_updated_at] | ||
end | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this file in general, I think the tiny methods might work better as inline code with a comment rather than a separate one line method that's only called in one place. I think avoiding ivars in favor of locals will help readability. Double emphasis on think.