]> Cypherpunks repositories - gostls13.git/commitdiff
misc/dashboard/codereview: send mail to assigned reviewers if they aren't already...
authorDavid Symonds <dsymonds@golang.org>
Mon, 30 Apr 2012 12:03:56 +0000 (22:03 +1000)
committerDavid Symonds <dsymonds@golang.org>
Mon, 30 Apr 2012 12:03:56 +0000 (22:03 +1000)
R=golang-dev, bradfitz, r
CC=golang-dev
https://golang.org/cl/6128054

misc/dashboard/codereview/dashboard/cl.go
misc/dashboard/codereview/dashboard/people.go

index 80493aa5bdae54828920524ee8231ac559058893..fe20eb8e6d6d96d4c8c90c40579d6ca55503ba08 100644 (file)
@@ -16,6 +16,8 @@ import (
 
        "appengine"
        "appengine/datastore"
+       "appengine/delay"
+       "appengine/mail"
        "appengine/taskqueue"
        "appengine/urlfetch"
        "appengine/user"
@@ -42,6 +44,10 @@ type CL struct {
        FirstLine   string `datastore:",noindex"`
        LGTMs       []string
 
+       // Mail information.
+       Subject    string   `datastore:",noindex"`
+       Recipients []string `datastore:",noindex"`
+
        // These are person IDs (e.g. "rsc"); they may be empty
        Author   string
        Reviewer string
@@ -98,6 +104,8 @@ func (cl *CL) ModifiedAgo() string {
        return "just now"
 }
 
+var sendMailLater = delay.Func("send-mail", mail.Send)
+
 func handleAssign(w http.ResponseWriter, r *http.Request) {
        c := appengine.NewContext(r)
 
@@ -106,7 +114,8 @@ func handleAssign(w http.ResponseWriter, r *http.Request) {
                return
        }
 
-       if _, ok := emailToPerson[user.Current(c).Email]; !ok {
+       u := user.Current(c)
+       if _, ok := emailToPerson[u.Email]; !ok {
                http.Error(w, "Not allowed", http.StatusUnauthorized)
                return
        }
@@ -117,8 +126,79 @@ func handleAssign(w http.ResponseWriter, r *http.Request) {
                http.Error(w, "Bad CL", 400)
                return
        }
+       if _, ok := preferredEmail[rev]; !ok && rev != "" {
+               c.Errorf("Unknown reviewer %q", rev)
+               http.Error(w, "Unknown reviewer", 400)
+               return
+       }
 
        key := datastore.NewKey(c, "CL", n, 0, nil)
+
+       if rev != "" {
+               // Make sure the reviewer is listed in Rietveld as a reviewer.
+               url := codereviewBase + "/" + n + "/fields"
+               resp, err := urlfetch.Client(c).Get(url + "?field=reviewers")
+               if err != nil {
+                       c.Errorf("Retrieving CL reviewer list failed: %v", err)
+                       http.Error(w, err.Error(), 500)
+                       return
+               }
+               defer resp.Body.Close()
+               if resp.StatusCode != 200 {
+                       c.Errorf("Retrieving CL reviewer list failed: got HTTP response %d", resp.StatusCode)
+                       http.Error(w, "Failed contacting Rietveld", 500)
+                       return
+               }
+
+               var apiResp struct {
+                       Reviewers []string `json:"reviewers"`
+               }
+               if err := json.NewDecoder(resp.Body).Decode(&apiResp); err != nil {
+                       // probably can't be retried
+                       msg := fmt.Sprintf("Malformed JSON from %v: %v", url, err)
+                       c.Errorf("%s", msg)
+                       http.Error(w, msg, 500)
+                       return
+               }
+               found := false
+               for _, r := range apiResp.Reviewers {
+                       if emailToPerson[r] == rev {
+                               found = true
+                               break
+                       }
+               }
+               if !found {
+                       c.Infof("Adding %v as a reviewer of CL %v", rev, n)
+
+                       // We can't do this easily, as we need authentication to edit
+                       // an issue on behalf of a user, which is non-trivial. For now,
+                       // just send a mail with the body "R=<reviewer>", Cc'ing that person,
+                       // and rely on social convention.
+                       cl := new(CL)
+                       err := datastore.Get(c, key, cl)
+                       if err != nil {
+                               c.Errorf("%s", err)
+                               http.Error(w, err.Error(), 500)
+                               return
+                       }
+                       // The current data does not have the subject/recipient information.
+                       // TODO(dsymonds): Remove this if when all the CLs have subject lines.
+                       if cl.Subject != "" {
+                               msg := &mail.Message{
+                                       Sender: u.Email,
+                                       To:     []string{preferredEmail[rev]},
+                                       Cc:     cl.Recipients,
+                                       // Take care to match Rietveld's subject line
+                                       // so that Gmail will correctly thread mail.
+                                       Subject: cl.Subject + " (issue " + n + ")",
+                                       Body:    "R=" + rev + "\n\n(sent by gocodereview)",
+                               }
+                               sendMailLater.Call(c, msg)
+                       }
+               }
+       }
+
+       // Update our own record.
        err := datastore.RunInTransaction(c, func(c appengine.Context) error {
                cl := new(CL)
                err := datastore.Get(c, key, cl)
@@ -187,10 +267,12 @@ func updateCL(c appengine.Context, n string) error {
                OwnerEmail  string `json:"owner_email"`
                Modified    string `json:"modified"`
                Closed      bool   `json:"closed"`
+               Subject     string `json:"subject"`
                Messages    []struct {
-                       Text     string `json:"text"`
-                       Sender   string `json:"sender"`
-                       Approval bool   `json:"approval"`
+                       Text       string   `json:"text"`
+                       Sender     string   `json:"sender"`
+                       Recipients []string `json:"recipients"`
+                       Approval   bool     `json:"approval"`
                } `json:"messages"`
        }
        if err := json.NewDecoder(resp.Body).Decode(&apiResp); err != nil {
@@ -206,6 +288,7 @@ func updateCL(c appengine.Context, n string) error {
                Owner:       apiResp.OwnerEmail,
                Description: []byte(apiResp.Description),
                FirstLine:   apiResp.Description,
+               Subject:     apiResp.Subject,
                Author:      emailToPerson[apiResp.OwnerEmail],
        }
        cl.Created, err = time.Parse("2006-01-02 15:04:05.000000", apiResp.Created)
@@ -219,6 +302,7 @@ func updateCL(c appengine.Context, n string) error {
        if i := strings.Index(cl.FirstLine, "\n"); i >= 0 {
                cl.FirstLine = cl.FirstLine[:i]
        }
+       rcpt := make(map[string]bool)
        for _, msg := range apiResp.Messages {
                s, rev := msg.Sender, false
                if p, ok := emailToPerson[s]; ok {
@@ -234,10 +318,19 @@ func updateCL(c appengine.Context, n string) error {
                }
 
                if msg.Approval {
+                       // TODO(dsymonds): De-dupe LGTMs.
                        cl.LGTMs = append(cl.LGTMs, s)
                }
+
+               for _, r := range msg.Recipients {
+                       rcpt[r] = true
+               }
+       }
+       for r := range rcpt {
+               cl.Recipients = append(cl.Recipients, r)
        }
        sort.Strings(cl.LGTMs)
+       sort.Strings(cl.Recipients)
 
        key := datastore.NewKey(c, "CL", n, 0, nil)
        err = datastore.RunInTransaction(c, func(c appengine.Context) error {
index a9a40c34d6ac972ba591efaa487675e592113fd3..d4a8a8aa58170c259ccf7e36fa72b6264d3bf1b9 100644 (file)
@@ -7,12 +7,14 @@ import (
 )
 
 var (
-       emailToPerson = make(map[string]string)
-       personList    []string
+       emailToPerson  = make(map[string]string) // email => person
+       preferredEmail = make(map[string]string) // person => email
+       personList     []string
 )
 
 func init() {
-       // People we assume have golang.org and google.com accounts.
+       // People we assume have golang.org and google.com accounts,
+       // and prefer to use their golang.org address for code review.
        gophers := [...]string{
                "adg",
                "bradfitz",
@@ -27,6 +29,7 @@ func init() {
                personList = append(personList, p)
                emailToPerson[p+"@golang.org"] = p
                emailToPerson[p+"@google.com"] = p
+               preferredEmail[p] = p + "@golang.org"
        }
 
        sort.Strings(personList)