From 485651df5506ff0a9e0f396adf1416b265fbc60f Mon Sep 17 00:00:00 2001 From: Guillaume Petiot Date: Mon, 25 Mar 2024 16:19:52 +0000 Subject: [PATCH] Check end date of contributions --- CHANGES.md | 10 ++ bin/main.ml | 16 ++- lib/contributions.ml | 4 +- lib/contributions.mli | 7 +- test/lib/test_contributions.ml | 251 +++++++++++++++++++-------------- 5 files changed, 174 insertions(+), 114 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3d5eb92..687573b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,13 @@ +## unreleased + +### Fixed + +- Take into account the end date of the specified period when filtering github activity (#, @gpetiot) + +### Changed + +- API: `Contributions.of_json` parameter `~from` is replaced by `~period` (#, @gpetiot) + ## 1.0.1 ### Fixed diff --git a/bin/main.ml b/bin/main.ml index 2e71466..a566c61 100644 --- a/bin/main.ml +++ b/bin/main.ml @@ -33,10 +33,10 @@ let mtime path = let get_token () = Token.load (home / ".github" / "github-activity-token") -let show ~from ~user json = - let* contribs = Contributions.of_json ~from ~user json in +let show ~period ~user json = + let* contribs = Contributions.of_json ~period ~user json in if Contributions.is_empty contribs then - Fmt.epr "(no activity found since %s)@." from + Fmt.epr "(no activity found since %s)@." (fst period) else Fmt.pr "%a@." Contributions.pp contribs let mode = `Normal @@ -96,7 +96,7 @@ let run period user : unit = let* token = get_token () in let request = Contributions.request ~period ~user ~token in let* contributions = Graphql.exec request in - show ~from:(fst period) ~user contributions) + show ~period ~user contributions) | `Save -> Period.with_period period ~last_fetch_file ~f:(fun period -> let* token = get_token () in @@ -105,10 +105,12 @@ let run period user : unit = Yojson.Safe.to_file "activity.json" contributions) | `Load -> (* When testing formatting changes, it is quicker to fetch the data once and then load it again for each test: *) - let from = - mtime last_fetch_file |> Option.value ~default:0.0 |> Period.to_8601 + let period = + let from = mtime last_fetch_file |> Option.value ~default:0.0 in + let to_ = Unix.time () in + (Period.to_8601 from, Period.to_8601 to_) in - show ~from ~user @@ Yojson.Safe.from_file "activity.json" + show ~period ~user @@ Yojson.Safe.from_file "activity.json" let term = Term.(const run $ period $ user) let cmd = Cmd.v info term diff --git a/lib/contributions.ml b/lib/contributions.ml index 3db81bd..6dc8b78 100644 --- a/lib/contributions.ml +++ b/lib/contributions.ml @@ -129,7 +129,7 @@ let read_repos = let title = "Created new repository" in { kind = `New_repo; date; url; title; body = ""; repo }) -let of_json ~from ~user json = +let of_json ~period:(from, to_) ~user json = let* json = match Json.t_of_yojson json with | x -> Ok x @@ -156,7 +156,7 @@ let of_json ~from ~user json = let activity = (* GitHub seems to ignore the time part, so do the filtering here. *) items - |> List.filter (fun item -> item.date >= from) + |> List.filter (fun item -> item.date >= from && item.date <= to_) |> List.fold_left (fun acc item -> let items = diff --git a/lib/contributions.mli b/lib/contributions.mli index c47b518..660c0e8 100644 --- a/lib/contributions.mli +++ b/lib/contributions.mli @@ -19,8 +19,11 @@ val request : period:string * string -> user:User.t -> token:Token.t -> Graphql.request val of_json : - from:string -> user:User.t -> Yojson.Safe.t -> (t, [ `Msg of string ]) result -(** We pass [from] again here so we can filter out anything that GitHub included by accident. *) + period:string * string -> + user:User.t -> + Yojson.Safe.t -> + (t, [ `Msg of string ]) result +(** We pass [period] again here so we can filter out anything that GitHub included by accident. *) val is_empty : t -> bool diff --git a/test/lib/test_contributions.ml b/test/lib/test_contributions.ml index fee29eb..201151b 100644 --- a/test/lib/test_contributions.ml +++ b/test/lib/test_contributions.ml @@ -327,113 +327,145 @@ let activity_example ~user = let activity_example_json ~user = Yojson.Safe.from_string (activity_example ~user) -let contributions_example ~user = +let contributions_example1 ~user = let open Contributions in { username = user |> or_viewer; activity = - Repo_map.empty - |> Repo_map.add "gpetiot/config.ml" - [ - { - repo = "gpetiot/config.ml"; - kind = `New_repo; - date = "2024-03-02T09:40:41Z"; - url = "https://github.com/gpetiot/config.ml"; - title = "Created new repository"; - body = ""; - }; - ] - |> Repo_map.add "gpetiot/js_of_ocaml" - [ - { - repo = "gpetiot/js_of_ocaml"; - kind = `New_repo; - date = "2024-03-01T10:43:33Z"; - url = "https://github.com/gpetiot/js_of_ocaml"; - title = "Created new repository"; - body = ""; - }; - ] - |> Repo_map.add "ocaml-ppx/ocamlformat" - [ - { - repo = "ocaml-ppx/ocamlformat"; - kind = `PR; - date = "2024-03-05T11:21:22Z"; - url = "https://github.com/ocaml-ppx/ocamlformat/pull/2533"; - title = "Represent the expr sequence as a list"; - body = "xxx"; - }; - ] - |> Repo_map.add "realworldocaml/mdx" - [ - { - repo = "realworldocaml/mdx"; - kind = `Review "APPROVED"; - date = "2024-03-05T11:43:04Z"; - url = - "https://github.com/realworldocaml/mdx/pull/449#pullrequestreview-1916654244"; - title = "Add upgrade instructions in the changelog for #446"; - body = "xxx"; - }; - { - repo = "realworldocaml/mdx"; - kind = `PR; - date = "2024-03-04T17:20:11Z"; - url = "https://github.com/realworldocaml/mdx/pull/450"; - title = "Add an 'exec' label to execute include OCaml blocks"; - body = "xxx"; - }; - ] - |> Repo_map.add "tarides/get-activity" - [ - { - repo = "tarides/get-activity"; - kind = `Issue; - date = "2024-03-04T11:55:37Z"; - url = "https://github.com/tarides/get-activity/issues/8"; - title = - "Add the PR/issues comments to the result of okra generate"; - body = "xxx"; - }; - ] - |> Repo_map.add "tarides/okra" - [ - { - repo = "tarides/okra"; - kind = `Review "APPROVED"; - date = "2024-02-28T11:09:41Z"; - url = - "https://github.com/tarides/okra/pull/166#pullrequestreview-1905972361"; - title = "Make README.md more precise"; - body = "xxx"; - }; - { - repo = "tarides/okra"; - kind = `Issue_comment; - date = "2024-03-13T11:09:56Z"; - url = - "https://github.com/tarides/okra/issues/114#issuecomment-1994130584"; - title = "Gitlab: exception when parsing Gitlab's JSON"; - body = "xxx"; - }; - { - repo = "tarides/okra"; - kind = `Issue; - date = "2024-02-27T12:05:04Z"; - url = "https://github.com/tarides/okra/issues/165"; - title = "Make the `get-activity` package known to ocaml-ci"; - body = "xxx"; - }; - ]; + Repo_map.of_seq @@ List.to_seq + @@ [ + ( "tarides/okra", + [ + { + repo = "tarides/okra"; + kind = `Review "APPROVED"; + date = "2024-02-28T11:09:41Z"; + url = + "https://github.com/tarides/okra/pull/166#pullrequestreview-1905972361"; + title = "Make README.md more precise"; + body = "xxx"; + }; + { + repo = "tarides/okra"; + kind = `Issue; + date = "2024-02-27T12:05:04Z"; + url = "https://github.com/tarides/okra/issues/165"; + title = "Make the `get-activity` package known to ocaml-ci"; + body = "xxx"; + }; + ] ); + ]; + } + +let contributions_example2 ~user = + let open Contributions in + { + username = user |> or_viewer; + activity = + Repo_map.of_seq @@ List.to_seq + @@ [ + ( "gpetiot/config.ml", + [ + { + repo = "gpetiot/config.ml"; + kind = `New_repo; + date = "2024-03-02T09:40:41Z"; + url = "https://github.com/gpetiot/config.ml"; + title = "Created new repository"; + body = ""; + }; + ] ); + ( "gpetiot/js_of_ocaml", + [ + { + repo = "gpetiot/js_of_ocaml"; + kind = `New_repo; + date = "2024-03-01T10:43:33Z"; + url = "https://github.com/gpetiot/js_of_ocaml"; + title = "Created new repository"; + body = ""; + }; + ] ); + ( "ocaml-ppx/ocamlformat", + [ + { + repo = "ocaml-ppx/ocamlformat"; + kind = `PR; + date = "2024-03-05T11:21:22Z"; + url = "https://github.com/ocaml-ppx/ocamlformat/pull/2533"; + title = "Represent the expr sequence as a list"; + body = "xxx"; + }; + ] ); + ( "realworldocaml/mdx", + [ + { + repo = "realworldocaml/mdx"; + kind = `Review "APPROVED"; + date = "2024-03-05T11:43:04Z"; + url = + "https://github.com/realworldocaml/mdx/pull/449#pullrequestreview-1916654244"; + title = "Add upgrade instructions in the changelog for #446"; + body = "xxx"; + }; + { + repo = "realworldocaml/mdx"; + kind = `PR; + date = "2024-03-04T17:20:11Z"; + url = "https://github.com/realworldocaml/mdx/pull/450"; + title = "Add an 'exec' label to execute include OCaml blocks"; + body = "xxx"; + }; + ] ); + ( "tarides/get-activity", + [ + { + repo = "tarides/get-activity"; + kind = `Issue; + date = "2024-03-04T11:55:37Z"; + url = "https://github.com/tarides/get-activity/issues/8"; + title = + "Add the PR/issues comments to the result of okra generate"; + body = "xxx"; + }; + ] ); + ( "tarides/okra", + [ + { + repo = "tarides/okra"; + kind = `Review "APPROVED"; + date = "2024-02-28T11:09:41Z"; + url = + "https://github.com/tarides/okra/pull/166#pullrequestreview-1905972361"; + title = "Make README.md more precise"; + body = "xxx"; + }; + { + repo = "tarides/okra"; + kind = `Issue_comment; + date = "2024-03-13T11:09:56Z"; + url = + "https://github.com/tarides/okra/issues/114#issuecomment-1994130584"; + title = "Gitlab: exception when parsing Gitlab's JSON"; + body = "xxx"; + }; + { + repo = "tarides/okra"; + kind = `Issue; + date = "2024-02-27T12:05:04Z"; + url = "https://github.com/tarides/okra/issues/165"; + title = "Make the `get-activity` package known to ocaml-ci"; + body = "xxx"; + }; + ] ); + ]; } let test_of_json = - let make_test name ~from ~user json ~expected = + let make_test name ~period ~user json ~expected = let name = Printf.sprintf "of_json: %s" name in let test_fun () = - let actual = Contributions.of_json ~from ~user json in + let actual = Contributions.of_json ~period ~user json in Alcotest.(check (Alcotest_ext.or_msg Testable.contributions)) name expected actual in @@ -441,13 +473,26 @@ let test_of_json = in [ (let user = User.Viewer in - make_test "no token" ~from:"" ~user + make_test "no token" + ~period:("2024-02-27T12:05:04Z", "2024-02-28T11:09:41Z") + ~user + (activity_example_json ~user) + ~expected:(Ok (contributions_example1 ~user))); + (let user = User.User "gpetiot" in + make_test "no token" + ~period:("2024-02-27T12:05:04Z", "2024-03-13T11:09:56Z") + ~user (activity_example_json ~user) - ~expected:(Ok (contributions_example ~user))); + ~expected:(Ok (contributions_example2 ~user))); (let user = User.User "gpetiot" in - make_test "no token" ~from:"" ~user + make_test "no token" ~period:("", "") ~user (activity_example_json ~user) - ~expected:(Ok (contributions_example ~user))); + ~expected: + (Ok + { + username = user |> or_viewer; + activity = Contributions.Repo_map.empty; + })); ] let test_is_empty = @@ -465,7 +510,7 @@ let test_is_empty = { Contributions.username = ""; activity = Contributions.Repo_map.empty } ~expected:true; make_test "not empty" - ~input:(contributions_example ~user:Viewer) + ~input:(contributions_example1 ~user:Viewer) ~expected:false; ]