Use Zulip ID instead of email for most purposes (#62)

Squash commit due to a not-great history.


- We already use Zulip ID as the database key.

  Ignore the "id" value in the database *value*, since it's redundant with the record ID.
  Alongside this, I slightly refactored how the record-to-struct conversion happens.

- Use the Zulip ID when sending messages.

  Empirically, list of IDs has to be given as `[123,456]`,
  i.e. with square brackets. It's fine to send a single-receiver message as `[123]`
  (Jeremy got the "odd one out" message).

- Retrieve the Zulip ID from the RC API, to use when handling end-of-batch processing

- Misc other cleanups

Co-authored-by: Jeremy Kaplan <[email protected]>

api.go

@@ -3,7 +3,7 @@ package main
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"log"
 	"net/http"
 	"strings"
@@ -15,31 +15,36 @@ type RecurseAPI struct {
 }
 
 type RecurserProfile struct {
-	Email string
+	Name    string
+	ZulipId int64 `json:"zulip_id"`
 }
 
-func (ra *RecurseAPI) userIsCurrentlyAtRC(accessToken string, email string) bool {
-	emailsOfPeopleAtRC := ra.getCurrentlyActiveEmails(accessToken)
+func (ra *RecurseAPI) userIsCurrentlyAtRC(accessToken string, id int64) (bool, error) {
+	// TODO: Unfortunately this is not a query parameter, but it could be.
+	ids, err := ra.getCurrentlyActiveZulipIds(accessToken)
 
-	return contains(emailsOfPeopleAtRC, email)
+	return contains(ids, id), err
 }
 
-//The profile API endpoint is updated at midnight on the last day (Friday) of a batch.
-func (ra *RecurseAPI) getCurrentlyActiveEmails(accessToken string) []string {
-	var emailsOfPeopleAtRC []string
+// The profile API endpoint is updated at midnight on the last day (Friday) of a batch.
+func (ra *RecurseAPI) getCurrentlyActiveZulipIds(accessToken string) ([]int64, error) {
+	var ids []int64
 	offset := 0
 	limit := 50
 	apiHasMoreResults := true
 
 	for apiHasMoreResults {
-		emailsStartingFromOffset := ra.getCurrentlyActiveEmailsWithOffset(accessToken, offset, limit)
+		idsStartingFromOffset, err := ra.getCurrentlyActiveZulipIdsWithOffset(accessToken, offset, limit)
+		if err != nil {
+			return nil, fmt.Errorf("while reading from offset %d: %w", offset, err)
+		}
 
-		emailsOfPeopleAtRC = append(emailsOfPeopleAtRC, emailsStartingFromOffset...)
+		ids = append(ids, idsStartingFromOffset...)
 
-		log.Printf("The API returned %v profiles from the offset of %v", len(emailsStartingFromOffset), offset)
+		log.Printf("The API returned %v profiles from the offset of %v", len(idsStartingFromOffset), offset)
 
 		//The API limits respones to 50 total profiles. Keep querying the API until there are no more Recurser Profiles remaining
-		if len(emailsStartingFromOffset) == limit {
+		if len(idsStartingFromOffset) == limit {
 			apiHasMoreResults = true
 			offset += limit
 
@@ -49,45 +54,54 @@ func (ra *RecurseAPI) getCurrentlyActiveEmails(accessToken string) []string {
 		}
 	}
 
-	log.Println("The API returned this many TOTAL profiles", len(emailsOfPeopleAtRC))
-	log.Println("Here are the emails of people currently at RC", emailsOfPeopleAtRC)
+	log.Println("The API returned this many TOTAL profiles", len(ids))
+	log.Println("Here are the Zulip IDs of people currently at RC", ids)
 
-	return emailsOfPeopleAtRC
+	return ids, nil
 }
 
 /*
-	The RC API limits queries to the profiles endpoint to 50 results. However, there may be more than 50 people currently at RC.
-	The RC API takes in an "offset" query param that allows us to grab records beyond that limit of 50 results by performing multiple api calls.
+The RC API limits queries to the profiles endpoint to 50 results. However, there may be more than 50 people currently at RC.
+The RC API takes in an "offset" query param that allows us to grab records beyond that limit of 50 results by performing multiple api calls.
 */
-func (ra *RecurseAPI) getCurrentlyActiveEmailsWithOffset(accessToken string, offset int, limit int) []string {
-	var emailsOfPeopleAtRC []string
+func (ra *RecurseAPI) getCurrentlyActiveZulipIdsWithOffset(accessToken string, offset int, limit int) ([]int64, error) {
+	var ids []int64
 
 	endpointString := fmt.Sprintf("/profiles?scope=current&offset=%v&limit=%v&role=recurser&access_token=%v", offset, limit, accessToken)
 
 	resp, err := http.Get(ra.rcAPIURL + endpointString)
 	if err != nil {
-		log.Printf("Got the following error while getting the RC batches from the RC API: %s\n", err)
+		return nil, fmt.Errorf("error while getting active RCers from the RC API: %w", err)
+	}
+	if resp.StatusCode >= 400 {
+		err = fmt.Errorf("HTTP error while getting active RCers from the RC API: %s", resp.Status)
+		log.Print(err)
 	}
 
 	defer resp.Body.Close()
 
-	body, err := ioutil.ReadAll(resp.Body)
-
+	body, bodyErr := io.ReadAll(resp.Body)
+	if bodyErr != nil {
+		log.Printf("Unable to get the Zulip IDs of people currently at RC due to the following error: %s", err)
+	}
+	// Return the first error encountered: the HTTP status error, or the body error.
+	// We've logged both of them in either case.
 	if err != nil {
-		log.Printf("Unable to get the emails of people currently at RC due to the following error: %s", err)
-		return emailsOfPeopleAtRC
+		return nil, err
+	} else if bodyErr != nil {
+		return nil, bodyErr
 	}
 
 	//Parse the json response from the API
 	recursers := []RecurserProfile{}
 	json.Unmarshal([]byte(body), &recursers)
 
 	for i := range recursers {
-		email := recursers[i].Email
-		emailsOfPeopleAtRC = append(emailsOfPeopleAtRC, email)
+		zid := recursers[i].ZulipId
+		ids = append(ids, zid)
 	}
 
-	return emailsOfPeopleAtRC
+	return ids, nil
 }
 
 func (ra *RecurseAPI) isSecondWeekOfBatch(accessToken string) bool {
@@ -98,7 +112,7 @@ func (ra *RecurseAPI) isSecondWeekOfBatch(accessToken string) bool {
 
 	defer resp.Body.Close()
 
-	body, err := ioutil.ReadAll(resp.Body)
+	body, err := io.ReadAll(resp.Body)
 
 	//Parse the json response from the API
 	var batches []map[string]interface{}

client.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"strconv"
 	"strings"
 )
 
@@ -20,15 +19,15 @@ type incomingJSON struct {
 	Token   string `json:"token"`
 	Trigger string `json:"trigger"`
 	Message struct {
-		SenderID         int         `json:"sender_id"`
+		SenderID         int64       `json:"sender_id"`
 		DisplayRecipient interface{} `json:"display_recipient"`
 		SenderEmail      string      `json:"sender_email"`
 		SenderFullName   string      `json:"sender_full_name"`
 	} `json:"message"`
 }
 
 type UserDataFromJSON struct {
-	userID    string
+	userID    int64
 	userEmail string
 	userName  string
 }
@@ -56,7 +55,7 @@ type userRequest interface {
 }
 
 type userNotification interface {
-	sendUserMessage(ctx context.Context, botPassword, user, message string) error
+	sendUserMessage(ctx context.Context, botPassword string, userIDs []int64, message string) error
 }
 
 type streamMessage interface {
@@ -120,12 +119,16 @@ func (zsm *zulipStreamMessage) postToTopic(ctx context.Context, botPassword, mes
 	return nil
 }
 
-func (zun *zulipUserNotification) sendUserMessage(ctx context.Context, botPassword, user, message string) error {
+func (zun *zulipUserNotification) sendUserMessage(ctx context.Context, botPassword string, userIDs []int64, message string) error {
 
 	zulipClient := &http.Client{}
 	messageRequest := url.Values{}
 	messageRequest.Add("type", "private")
-	messageRequest.Add("to", user)
+	users := []string{}
+	for _, id := range userIDs {
+		users = append(users, fmt.Sprint(id))
+	}
+	messageRequest.Add("to", "["+strings.Join(users, ",")+"]")
 	messageRequest.Add("content", message)
 
 	req, err := http.NewRequestWithContext(ctx, "POST", zun.zulipAPIURL, strings.NewReader(messageRequest.Encode()))
@@ -200,7 +203,7 @@ func (zur *zulipUserRequest) getCommandString() string {
 
 func (zur *zulipUserRequest) extractUserData() *UserDataFromJSON {
 	return &UserDataFromJSON{
-		userID:    strconv.Itoa(zur.json.Message.SenderID),
+		userID:    int64(zur.json.Message.SenderID),
 		userEmail: zur.json.Message.SenderEmail,
 		userName:  zur.json.Message.SenderFullName,
 	}
@@ -216,7 +219,7 @@ type mockUserRequest struct {
 type mockUserNotification struct {
 }
 
-func (mun *mockUserNotification) sendUserMessage(ctx context.Context, botPassword, user, message string) error {
+func (mun *mockUserNotification) sendUserMessage(ctx context.Context, botPassword string, userIDs []int64, message string) error {
 	return nil
 }