remotefiles: add tests

slack-go · Jan 1, 2021 · 56feeb9 · 56feeb9
1 parent 46dc7e1
commit 56feeb9
Show file tree

Hide file tree

Showing 2 changed files with 223 additions and 13 deletions.
diff --git a/remotefiles.go b/remotefiles.go
@@ -2,6 +2,7 @@ package slack
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/url"
 	"strconv"
@@ -90,11 +91,8 @@ func (api *Client) AddRemoteFile(params RemoteFileParameters) (remotefile *Remot
 
 // AddRemoteFileContext adds a remote file and setting a custom context
 func (api *Client) AddRemoteFileContext(ctx context.Context, params RemoteFileParameters) (remotefile *RemoteFile, err error) {
-	// Test if user token is valid. This helps because client.Do doesn't like this for some reason. XXX: More
-	// investigation needed, but for now this will do.
-	_, err = api.AuthTest()
-	if err != nil {
-		return nil, err
+	if params.ExternalID == "" || params.ExternalURL == "" || params.Title == "" {
+		return nil, ErrParametersMissing
 	}
 	response := &remoteFileResponseFull{}
 	values := url.Values{
@@ -167,6 +165,12 @@ func (api *Client) GetRemoteFileInfo(externalID, fileID string) (remotefile *Rem
 
 // GetRemoteFileInfoContext retrieves the complete remote file information given with a custom context.
 func (api *Client) GetRemoteFileInfoContext(ctx context.Context, externalID, fileID string) (remotefile *RemoteFile, err error) {
+	if fileID == "" && externalID == "" {
+		return nil, fmt.Errorf("either externalID or fileID is required")
+	}
+	if fileID != "" && externalID != "" {
+		return nil, fmt.Errorf("don't provide both externalID and fileID")
+	}
 	values := url.Values{
 		"token": {api.token},
 	}
@@ -190,6 +194,12 @@ func (api *Client) ShareRemoteFile(channels []string, externalID, fileID string)
 
 // ShareRemoteFileContext shares a remote file to channels with a custom context
 func (api *Client) ShareRemoteFileContext(ctx context.Context, channels []string, externalID, fileID string) (file *RemoteFile, err error) {
+	if channels == nil || len(channels) == 0 {
+		return nil, ErrParametersMissing
+	}
+	if fileID == "" && externalID == "" {
+		return nil, fmt.Errorf("either externalID or fileID is required")
+	}
 	values := url.Values{
 		"token":    {api.token},
 		"channels": {strings.Join(channels, ",")},
@@ -214,12 +224,6 @@ func (api *Client) UpdateRemoteFile(fileID string, params RemoteFileParameters)
 
 // UpdateRemoteFileContext updates a remote file with a custom context
 func (api *Client) UpdateRemoteFileContext(ctx context.Context, fileID string, params RemoteFileParameters) (remotefile *RemoteFile, err error) {
-	// Test if user token is valid. This helps because client.Do doesn't like this for some reason. XXX: More
-	// investigation needed, but for now this will do.
-	_, err = api.AuthTest()
-	if err != nil {
-		return nil, err
-	}
 	response := &remoteFileResponseFull{}
 	values := url.Values{
 		"token": {api.token},
@@ -243,9 +247,9 @@ func (api *Client) UpdateRemoteFileContext(ctx context.Context, fileID string, p
 		values.Add("indexable_file_contents", params.IndexableFileContents)
 	}
 	if params.PreviewImageReader != nil {
-		err = postWithMultipartResponse(ctx, api.httpclient, api.endpoint+"files.remote.add", "preview.png", "preview_image", values, params.PreviewImageReader, response, api)
+		err = postWithMultipartResponse(ctx, api.httpclient, api.endpoint+"files.remote.update", "preview.png", "preview_image", values, params.PreviewImageReader, response, api)
 	} else {
-		response, err = api.remoteFileRequest(ctx, "files.remote.add", values)
+		response, err = api.remoteFileRequest(ctx, "files.remote.update", values)
 	}
 
 	if err != nil {
@@ -262,6 +266,12 @@ func (api *Client) RemoveRemoteFile(externalID, fileID string) (err error) {
 
 // RemoveRemoteFileContext removes a remote file with a custom context
 func (api *Client) RemoveRemoteFileContext(ctx context.Context, externalID, fileID string) (err error) {
+	if fileID == "" && externalID == "" {
+		return fmt.Errorf("either externalID or fileID is required")
+	}
+	if fileID != "" && externalID != "" {
+		return fmt.Errorf("don't provide both externalID and fileID")
+	}
 	values := url.Values{
 		"token": {api.token},
 	}

diff --git a/remotefiles_test.go b/remotefiles_test.go
@@ -0,0 +1,200 @@
+package slack
+
+import (
+	"encoding/json"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func addRemoteFileHandler(rw http.ResponseWriter, r *http.Request) {
+	rw.Header().Set("Content-Type", "application/json")
+	response, _ := json.Marshal(remoteFileResponseFull{
+		SlackResponse: SlackResponse{Ok: true}})
+	rw.Write(response)
+}
+
+func TestAddRemoteFile(t *testing.T) {
+	http.HandleFunc("/files.remote.add", addRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	params := RemoteFileParameters{
+		ExternalID:  "externalID",
+		ExternalURL: "http://example.com/",
+		Title:       "example",
+	}
+	if _, err := api.AddRemoteFile(params); err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
+func TestAddRemoteFileWithoutTitle(t *testing.T) {
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	params := RemoteFileParameters{
+		ExternalID:  "externalID",
+		ExternalURL: "http://example.com/",
+	}
+	if _, err := api.AddRemoteFile(params); err != ErrParametersMissing {
+		t.Errorf("Expected ErrParametersMissing. got %s", err)
+	}
+}
+
+func listRemoteFileHandler(rw http.ResponseWriter, r *http.Request) {
+	rw.Header().Set("Content-Type", "application/json")
+	response, _ := json.Marshal(remoteFileResponseFull{
+		SlackResponse: SlackResponse{Ok: true}})
+	rw.Write(response)
+}
+
+func TestListRemoteFile(t *testing.T) {
+	http.HandleFunc("/files.remote.list", listRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	params := ListRemoteFilesParameters{}
+	if _, _, err := api.ListRemoteFiles(params); err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
+func getRemoteFileInfoHandler(rw http.ResponseWriter, r *http.Request) {
+	rw.Header().Set("Content-Type", "application/json")
+	response, _ := json.Marshal(remoteFileResponseFull{
+		SlackResponse: SlackResponse{Ok: true}})
+	rw.Write(response)
+}
+
+func TestGetRemoteFileInfo(t *testing.T) {
+	http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	if _, err := api.GetRemoteFileInfo("ExternalID", ""); err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
+func TestGetRemoteFileInfoWithoutID(t *testing.T) {
+	http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	_, err := api.GetRemoteFileInfo("", "")
+	if err == nil {
+		t.Fatal("Expected error when both externalID and fileID is not provided, instead got nil")
+	}
+	if !strings.Contains(err.Error(), "either externalID or fileID is required") {
+		t.Errorf("Error message should mention a required field")
+	}
+}
+
+func TestGetRemoteFileInfoWithFileIDAndExternalID(t *testing.T) {
+	http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	_, err := api.GetRemoteFileInfo("ExternalID", "FileID")
+	if err == nil {
+		t.Fatal("Expected error when both externalID and fileID are both provided, instead got nil")
+	}
+	if !strings.Contains(err.Error(), "don't provide both externalID and fileID") {
+		t.Errorf("Error message should mention don't providing both externalID and fileID")
+	}
+}
+
+func shareRemoteFileHandler(rw http.ResponseWriter, r *http.Request) {
+	rw.Header().Set("Content-Type", "application/json")
+	response, _ := json.Marshal(remoteFileResponseFull{
+		SlackResponse: SlackResponse{Ok: true}})
+	rw.Write(response)
+}
+
+func TestShareRemoteFile(t *testing.T) {
+	http.HandleFunc("/files.remote.share", shareRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	if _, err := api.ShareRemoteFile([]string{"channel"}, "ExternalID", ""); err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
+func TestShareRemoteFileWithoutChannels(t *testing.T) {
+	http.HandleFunc("/files.remote.share", shareRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	if _, err := api.ShareRemoteFile([]string{}, "ExternalID", ""); err != ErrParametersMissing {
+		t.Errorf("Expected ErrParametersMissing. got %s", err)
+	}
+}
+
+func TestShareRemoteFileWithoutID(t *testing.T) {
+	http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	_, err := api.ShareRemoteFile([]string{"channel"}, "", "")
+	if err == nil {
+		t.Fatal("Expected error when both externalID and fileID is not provided, instead got nil")
+	}
+	if !strings.Contains(err.Error(), "either externalID or fileID is required") {
+		t.Errorf("Error message should mention a required field")
+	}
+}
+
+func updateRemoteFileHandler(rw http.ResponseWriter, r *http.Request) {
+	rw.Header().Set("Content-Type", "application/json")
+	response, _ := json.Marshal(remoteFileResponseFull{
+		SlackResponse: SlackResponse{Ok: true}})
+	rw.Write(response)
+}
+
+func TestUpdateRemoteFile(t *testing.T) {
+	http.HandleFunc("/files.remote.update", updateRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	params := RemoteFileParameters{
+		ExternalURL: "http://example.com/",
+		Title:       "example",
+	}
+	if _, err := api.UpdateRemoteFile("fileID", params); err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
+func removeRemoteFileHandler(rw http.ResponseWriter, r *http.Request) {
+	rw.Header().Set("Content-Type", "application/json")
+	response, _ := json.Marshal(remoteFileResponseFull{
+		SlackResponse: SlackResponse{Ok: true}})
+	rw.Write(response)
+}
+
+func TestRemoveRemoteFile(t *testing.T) {
+	http.HandleFunc("/files.remote.remove", removeRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	if err := api.RemoveRemoteFile("ExternalID", ""); err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
+func TestRemoveRemoteFileWithoutID(t *testing.T) {
+	http.HandleFunc("/files.remote.remove", removeRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	err := api.RemoveRemoteFile("", "")
+	if err == nil {
+		t.Fatal("Expected error when both externalID and fileID is not provided, instead got nil")
+	}
+	if !strings.Contains(err.Error(), "either externalID or fileID is required") {
+		t.Errorf("Error message should mention a required field")
+	}
+}
+
+func TestRemoveRemoteFileWithFileIDAndExternalID(t *testing.T) {
+	http.HandleFunc("/files.remote.remove", removeRemoteFileHandler)
+	once.Do(startServer)
+	api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/"))
+	err := api.RemoveRemoteFile("ExternalID", "FileID")
+	if err == nil {
+		t.Fatal("Expected error when both externalID and fileID are both provided, instead got nil")
+	}
+	if !strings.Contains(err.Error(), "don't provide both externalID and fileID") {
+		t.Errorf("Error message should mention don't providing both externalID and fileID")
+	}
+}