internal/github: fix parsing of times

The format for parsing times differed from the one used by the GitHub
API. The error was silently lost. Instead, a zero time was returned,
meaning all issues and comments appeared to be before the
commentfix.Fixer's cutoff and were not processed.

This CL should fix the problem for subsequent issues, but we still
have to go back and manually fix the ones that were missed somehow.

Besides using the right format, and testing it against actual values
from GH, also panic on error so gaby fails fast. It was only because
Cherry was observant that this was noticed at all.

For #54.

Change-Id: Ib639dd68cae950ccba1af8e798ea6ae2c9ba5e19
Reviewed-on: https://go-review.googlesource.com/c/oscar/+/629797
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>

Author: jba

internal/github/data.go

@@ -297,8 +297,8 @@ func (x *IssueComment) CommentID() int64 {
 func (x *IssueComment) ID() string                 { return x.URL }
 func (x *IssueComment) Title_() string             { return "" }
 func (x *IssueComment) Body_() string              { return x.Body }
-func (x *IssueComment) CreatedAt_() time.Time      { return parseTimeOrZero(x.CreatedAt) }
-func (x *IssueComment) UpdatedAt_() time.Time      { return parseTimeOrZero(x.UpdatedAt) }
+func (x *IssueComment) CreatedAt_() time.Time      { return mustParseTime(x.CreatedAt) }
+func (x *IssueComment) UpdatedAt_() time.Time      { return mustParseTime(x.UpdatedAt) }
 func (x *IssueComment) Author() *model.Identity    { panic("TODO: convert User to Identity") }
 func (x *IssueComment) CanEdit() bool              { return true }
 func (x *IssueComment) CanHaveChildren() bool      { return false }
@@ -346,8 +346,8 @@ func (i *Issue) DocID() string {
 func (x *Issue) ID() string                 { return x.URL }
 func (x *Issue) Title_() string             { return x.Title }
 func (x *Issue) Body_() string              { return x.Body }
-func (x *Issue) CreatedAt_() time.Time      { return parseTimeOrZero(x.CreatedAt) }
-func (x *Issue) UpdatedAt_() time.Time      { return parseTimeOrZero(x.UpdatedAt) }
+func (x *Issue) CreatedAt_() time.Time      { return mustParseTime(x.CreatedAt) }
+func (x *Issue) UpdatedAt_() time.Time      { return mustParseTime(x.UpdatedAt) }
 func (x *Issue) Author() *model.Identity    { panic("TODO: convert User to Identity") }
 func (x *Issue) CanEdit() bool              { return true }
 func (x *Issue) CanHaveChildren() bool      { return false }
@@ -358,7 +358,13 @@ func (x *Issue) Updates() model.PostUpdates { return &IssueChanges{} }
 
 var _ model.Post = (*Issue)(nil)
 
-func parseTimeOrZero(s string) time.Time {
-	t, _ := time.Parse("2006-01-02 15:04:05", s)
+func mustParseTime(s string) time.Time {
+	if s == "" {
+		return time.Time{}
+	}
+	t, err := time.Parse(time.RFC3339, s)
+	if err != nil {
+		storage.Panic("bad time: %v", err)
+	}
 	return t
 }

internal/github/data_test.go

@@ -0,0 +1,26 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package github
+
+import (
+	"testing"
+	"time"
+)
+
+func TestMustParseTime(t *testing.T) {
+	for _, tc := range []struct {
+		in   string
+		want time.Time
+	}{
+		{"", time.Time{}},
+		{"2015-08-05T01:55:29Z", time.Date(2015, 8, 5, 1, 55, 29, 0, time.UTC)},
+		{"2024-11-11T18:41:58Z", time.Date(2024, 11, 11, 18, 41, 58, 0, time.UTC)},
+	} {
+		got := mustParseTime(tc.in)
+		if !got.Equal(tc.want) {
+			t.Errorf("%q: got %s, want %s", tc.in, got, tc.want)
+		}
+	}
+}