Repository: slack-go/slack
Pull request #865 (2020-12-10 10:56:40)

Reminders list fix
##### PR preparation
Run `make pr-prep` from the root of the repository to run formatting, linting and tests.

Done.

##### Should this be an issue instead

Fix for a bug that appeared in https://github.com/slack-go/slack/pull/812

##### API changes

At the moment the reminders list array is being populated with the same data, this is because of Golang behaviour inside range loops - it copies the values inside its loop and gets rid of it when it's done. This PR fixes it so the data returned is correct.

Updated the test to check for this issue (that's how I confirmed the fix works)



Patch:
diff --git a/reminders.go b/reminders.go
index f312af3..ae1da86 100644
--- a/reminders.go
+++ b/reminders.go
@@ -22,7 +22,7 @@ type reminderResp struct {
 
 type remindersResp struct {
 	SlackResponse
-	Reminders []Reminder `json:"reminders"`
+	Reminders []*Reminder `json:"reminders"`
 }
 
 func (api *Client) doReminder(ctx context.Context, path string, values url.Values) (*Reminder, error) {
@@ -42,7 +42,7 @@ func (api *Client) doReminders(ctx context.Context, path string, values url.Valu
 	// create an array of pointers to reminders
 	var reminders = make([]*Reminder, 0, len(response.Reminders))
 	for _, reminder := range response.Reminders {
-		reminders = append(reminders, &reminder)
+		reminders = append(reminders, reminder)
 	}
 
 	return reminders, response.Err()


Test patch:
diff --git a/reminders_test.go b/reminders_test.go
index 919ba5b..25291b5 100644
--- a/reminders_test.go
+++ b/reminders_test.go
@@ -1,6 +1,8 @@
 package slack
 
 import (
+	"bytes"
+	"io/ioutil"
 	"net/http"
 	"reflect"
 	"testing"
@@ -156,17 +158,54 @@ func TestSlack_DeleteReminder(t *testing.T) {
 	}
 }
 
+type mockRemindersListHTTPClient struct{}
+
+func (m *mockRemindersListHTTPClient) Do(*http.Request) (*http.Response, error) {
+	responseString := `{
+		"ok": true,
+		"reminders": [
+	        {
+				"id": "Rm12345678",
+				"creator": "U18888888",
+				"user": "U18888888",
+				"text": "eat a banana",
+				"recurring": false,
+				"time": 1458678068,
+				"complete_ts": 0
+			},
+			{
+				"id": "Gm12345678",
+				"creator": "U18888888",
+				"user": "U18888888",
+				"text": "drink some water",
+				"recurring": false,
+				"time": 1458678090,
+				"complete_ts": 0
+			}
+		]
+	}`
+
+	return &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewBufferString(responseString))}, nil
+}
+
 func TestSlack_ListReminders(t *testing.T) {
-	// as the ListReminders takes only token parameter
-	// we just test if the call doesn't return an error
+	expectedIDs := []string{"Rm12345678", "Gm12345678"}
+
 	once.Do(startServer)
-	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
-	var rh *remindersHandler
-	http.HandleFunc("/reminders.list", func(w http.ResponseWriter, r *http.Request) { rh.handler(w, r) })
-	rh = newRemindersHandler()
+	api := &Client{
+		endpoint:   "http://" + serverAddr + "/",
+		token:      "testing-token",
+		httpclient: &mockRemindersListHTTPClient{},
+	}
 
-	_, err := api.ListReminders()
+	reminders, err := api.ListReminders()
 	if err != nil {
 		t.Fatalf("Unexpected error: %s", err)
 	}
+
+	for i := range reminders {
+		if reminders[i].ID != expectedIDs[i] {
+			t.Fatalf("List Reminders data wasn't correctly populated: wanted %v, got %v", expectedIDs[i], reminders[i].ID)
+		}
+	}
 }