Closed PRs are accounted for

hpjaj commented 5 years ago

Background

The current logic is excluding closed PRs, and only tracking open and merged PRs. Keifer and I are now thinking that the developer puts in (sometimes significant) time on a PR, and sometimes they do not get merged. The dev should not get penalized for that, as it reflective of work that was done during the course of the week.

Issue Resolved

Issue #75

Definition of Done

[x] Includes closed PRs when pulling data from GitHub, and presenting data in our views

RobAWilkinson commented 5 years ago

Great call, will this also return reviewed PRs as well?

hpjaj commented 5 years ago

@RobAWilkinson - No, this wouldn't capture 'PR review' data, but that probably is something we could explore adding. Would need to do some discovery on GitHub's REST API's offerings around reviewed PR's. I bet we could figure something out!

hpjaj commented 5 years ago

@RobAWilkinson - Yep, just did a quick spike on it. We can definitely do that using their reviewed-by search criteria. The new Github::Service object would basically look like this:

    # Fetches all of the pull requests that:
    #   - the user code reviewed
    #   - stem from any of the repositories in the user's organization
    #   - have an updated datetime >= the passed datetime
    #
    # @return [HTTParty::Response]
    # @see https://developer.github.com/v3/search/#search-issues
    # @see https://www.rubydoc.info/github/jnunemaker/httparty/HTTParty/Response
    #
    def pull_requests_reviewed
      search_criteria_present!

      query    = "q=type:pr+org:#{org}+reviewed-by:#{username}+updated:>=#{datetime}"
      url      = "#{SEARCH_ENDPOINT}?#{results_size}&#{query}"
      response = HTTParty.get url, options

      handle! response
    end

And then when we convert the raw data in the Github::Persist class, we'd want to exclude any items where the author is the user. Their API won't do this for us, but NBD at all b/c we can do it with a Ruby .reject.

kfrz commented 5 years ago

Great stuff! :+1:

oddballteam / magnifier