From 7cdd7a6851ae9137d4aa8f944ad495217549ef7c Mon Sep 17 00:00:00 2001 From: Robin Neatherway Date: Thu, 7 Mar 2024 23:53:17 +0000 Subject: [PATCH] Add support for fetching subthreads --- cmd/gh-slack/cmd/read.go | 37 ++++++++++++++++++--------- cmd/gh-slack/cmd/read_test.go | 46 +++++++++++++++++++++++++++++----- internal/slackclient/client.go | 32 +++++++++++++++++------ 3 files changed, 89 insertions(+), 26 deletions(-) diff --git a/cmd/gh-slack/cmd/read.go b/cmd/gh-slack/cmd/read.go index 0842843..ae411e4 100644 --- a/cmd/gh-slack/cmd/read.go +++ b/cmd/gh-slack/cmd/read.go @@ -8,6 +8,7 @@ import ( "net/url" "os" "regexp" + "strings" "github.com/rneatherway/gh-slack/internal/gh" "github.com/rneatherway/gh-slack/internal/markdown" @@ -28,29 +29,41 @@ var readCmd = &cobra.Command{ } var ( - permalinkRE = regexp.MustCompile("https://([^./]+).slack.com/archives/([A-Z0-9]+)/p([0-9]+)([0-9]{6})") - nwoRE = regexp.MustCompile("^/[^/]+/[^/]+/?$") - issueRE = regexp.MustCompile("^/[^/]+/[^/]+/(issues|pull)/[0-9]+/?$") + nwoRE = regexp.MustCompile("^/[^/]+/[^/]+/?$") + issueRE = regexp.MustCompile("^/[^/]+/[^/]+/(issues|pull)/[0-9]+/?$") ) type linkParts struct { team string channelID string timestamp string + thread string } -// https://github.slack.com/archives/CP9GMKJCE/p1648028606962719 -// returns (github, CP9GMKJCE, 1648028606.962719, nil) func parsePermalink(link string) (linkParts, error) { - result := permalinkRE.FindStringSubmatch(link) - if result == nil { - return linkParts{}, fmt.Errorf("not a permalink: %q", link) + u, err := url.Parse(link) + if err != nil { + return linkParts{}, err + } + + team, ok := strings.CutSuffix(u.Hostname(), ".slack.com") + if !ok { + return linkParts{}, fmt.Errorf("expected slack.com subdomain: %q", link) } + pathSegments := strings.Split(strings.TrimPrefix(u.Path, "/"), "/") + if len(pathSegments) != 3 || pathSegments[0] != "archives" { + return linkParts{}, fmt.Errorf("expected path of the form /archives//p: %q", link) + } + + channel := pathSegments[1] + timestamp := pathSegments[2][1:len(pathSegments[2])-6] + "." + pathSegments[2][len(pathSegments[2])-6:] + return linkParts{ - team: result[1], - channelID: result[2], - timestamp: result[3] + "." + result[4], + team: team, + channelID: channel, + timestamp: timestamp, + thread: u.Query().Get("thread_ts"), }, nil } @@ -123,7 +136,7 @@ func readSlack(args []string) error { return err } - history, err := client.History(linkParts.channelID, linkParts.timestamp, opts.Limit) + history, err := client.History(linkParts.channelID, linkParts.timestamp, linkParts.thread, opts.Limit) if err != nil { return err } diff --git a/cmd/gh-slack/cmd/read_test.go b/cmd/gh-slack/cmd/read_test.go index 0b84f0c..8954500 100644 --- a/cmd/gh-slack/cmd/read_test.go +++ b/cmd/gh-slack/cmd/read_test.go @@ -2,12 +2,46 @@ package cmd import "testing" -func TestPermalinkRE(t *testing.T) { - result := permalinkRE.FindStringSubmatch("https://github.slack.com/archives/CP9GMKJCE/p1648028606962719") - if len(result) != 5 { - t.Errorf("result had length %d", len(result)) +func TestParsePermalink(t *testing.T) { + tests := []struct { + link string + expected linkParts + }{ + { + link: "https://github.slack.com/archives/CP9GMKJCE/p1648028606962719", + expected: linkParts{ + team: "github", + channelID: "CP9GMKJCE", + timestamp: "1648028606.962719", + }, + }, + { + link: "https://sanity-io-land.slack.com/archives/C9Y51FDGA/p1709663536325529", + expected: linkParts{ + team: "sanity-io-land", + channelID: "C9Y51FDGA", + timestamp: "1709663536.325529", + }, + }, + { + link: "https://example.slack.com/archives/ABC123/p1709663536325529?thread_ts=1234567890.123456&cid=ABC123", + expected: linkParts{ + team: "example", + channelID: "ABC123", + thread: "1234567890.123456", + timestamp: "1709663536.325529", + }, + }, } - if result[1] != "github" || result[2] != "CP9GMKJCE" || result[3] != "1648028606" || result[4] != "962719" { - t.Fail() + + for _, test := range tests { + actual, err := parsePermalink(test.link) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if actual.team != test.expected.team || actual.channelID != test.expected.channelID || actual.timestamp != test.expected.timestamp { + t.Errorf("unexpected result for link %s, got %+v, want %+v", test.link, actual, test.expected) + } } } diff --git a/internal/slackclient/client.go b/internal/slackclient/client.go index fc020fc..6babf0d 100644 --- a/internal/slackclient/client.go +++ b/internal/slackclient/client.go @@ -38,6 +38,7 @@ type Message struct { Attachments []Attachment Ts string Type string + ReplyCount int `json:"reply_count"` } type SendMessage struct { @@ -291,12 +292,20 @@ func (c *SlackClient) loadCache() error { return json.Unmarshal(content, &c.cache) } -func (c *SlackClient) History(channelID string, startTimestamp string, limit int) (*HistoryResponse, error) { - body, err := c.get("conversations.replies", - map[string]string{ - "channel": channelID, - "ts": startTimestamp, - "inclusive": "true"}) +func (c *SlackClient) History(channelID string, startTimestamp string, thread string, limit int) (*HistoryResponse, error) { + params := map[string]string{ + "channel": channelID, + "ts": startTimestamp, + "inclusive": "true", + "limit": strconv.Itoa(limit), + } + + if thread != "" { + params["ts"] = thread + params["oldest"] = startTimestamp + } + + body, err := c.get("conversations.replies", params) if err != nil { return nil, err } @@ -311,8 +320,15 @@ func (c *SlackClient) History(channelID string, startTimestamp string, limit int return nil, fmt.Errorf("conversations.replies response not OK: %s", body) } - if len(historyResponse.Messages) > 1 { - // This was a thread, so we can return immediately + // If thread was specified, then we are fetching only part of a thread and + // should remove the first message if it has a reply count as we don't want + // the root message. + if thread != "" && historyResponse.Messages[0].ReplyCount != 0 && len(historyResponse.Messages) > 1 { + historyResponse.Messages = historyResponse.Messages[1:] + } + + if thread != "" || historyResponse.Messages[0].ReplyCount != 0 { + // Either we are deliberately fetching a subthread, or an entire thread. return historyResponse, nil }