Allow target_ref to be modified by subsequent patchsets with logging

appengine/chromium_rietveld/codereview/views.py

 # Copyright 2008 Google Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
 #     http://www.apache.org/licenses/LICENSE-2.0
 #
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 1318 matching lines @@
   if request.POST.get('send_mail') == 'yes' or request.POST.get('message'):
     msg = make_message(request, request.issue, request.POST.get('message', ''),
                        send_mail=(request.POST.get('send_mail', '') == 'yes'))
     request.issue.put()
     msg.put()
     notify_xmpp.notify_issue(request, request.issue, 'Mailed')
 
   return HttpTextResponse('OK')
 
 
+def _get_target_ref(form, issue_key, project):
+  target_ref = form.cleaned_data.get('target_ref', None)
+  # The following is a hack to ensure that all target_refs for chromium and v8
+  # start with 'refs/pending/'. More context here:
+  # https://code.google.com/p/chromium/issues/detail?id=435702#c10
+  # TODO(rmistry): Remove the below hack when the logged warning stops showing
+  # up.
+  if (target_ref and not target_ref.startswith('refs/pending/') and
+      project in ('chromium', 'v8')):
+    target_ref = target_ref.replace('refs/', 'refs/pending/')
+    logging.warn('Issue %d for %s did not start with refs/pending/',

[agable, 2014/12/19 17:52:56]

nit: "Target ref %s for issue %d in project %s did not start..."

+                 issue_key.id(), project)
+  return target_ref
+
+
 def _make_new(request, form):
   """Creates new issue and fill relevant fields from given form data.
 
   Sends notification about created issue (if requested with send_mail param).
 
   Returns (Issue, PatchSet) or (None, None).
   """
   if not form.is_valid():
     return (None, None)
   account = models.Account.get_account_for_user(request.user)
@@ skipping 15 matching lines @@
     return (None, None)
 
   base = form.get_base()
   if base is None:
     return (None, None)
 
   first_issue_id, _ = models.Issue.allocate_ids(1)
   issue_key = ndb.Key(models.Issue, first_issue_id)
 
   project = form.cleaned_data['project']
-  target_ref = form.cleaned_data.get('target_ref', None)
-  # The following is a hack to ensure that all target_refs for chromium and v8
-  # start with 'refs/pending/'. More context here:
-  # https://code.google.com/p/chromium/issues/detail?id=435702#c10
-  # TODO(rmistry): Remove the below hack when the logged warning stops showing
-  # up.
-  if (target_ref and not target_ref.startswith('refs/pending/') and
-      project in ('chromium', 'v8')):
-    target_ref = target_ref.replace('refs/', 'refs/pending/')
-    logging.warn('Issue %d for %s did not start with refs/pending/',
-                 issue_key.id(), project)
+  target_ref = _get_target_ref(form, issue_key, project)
 
   issue = models.Issue(subject=form.cleaned_data['subject'],
                        description=form.cleaned_data['description'],
                        project=project,
                        base=base,
                        target_ref=target_ref,
                        repo_guid=form.cleaned_data.get('repo_guid', None),
                        reviewers=reviewers,
                        required_reviewers=required_reviewers,
                        cc=cc,
@@ skipping 125 matching lines @@
     issue.required_reviewers += [reviewer for reviewer in required_reviewers
                                  if reviewer not in issue.required_reviewers]
     emails, _ = _get_emails(form, 'cc')
     if not form.is_valid():
       return None
     issue.cc += [cc for cc in emails if cc not in issue.cc]
   else:
     issue.reviewers, issue.required_reviewers = _get_emails(form, 'reviewers')
     issue.cc, _ = _get_emails(form, 'cc')
   issue.commit = False
+  issue.target_ref = _get_target_ref(form, issue.key, issue.project)
   issue.calculate_updates_for()
   issue.put()
+  # Log for auditing purposes.
+  logging.info("Patchset id %s for issue %s has target_ref %s",
+               patchset.key.id(), issue.key.id(), issue.target_ref)
 
   if form.cleaned_data.get('send_mail'):
     msg = make_message(request, issue, message, '', True)
     issue.put()
     msg.put()
     notify_xmpp.notify_issue(request, issue, 'Updated')
   return patchset
 
 def _get_emails(form, label):
   """Returns (reviewers, required_reviewers) or (None, None) for error."""
@@ skipping 3647 matching lines @@
     return HttpResponseNotFound()
   tops = []
   shame = []
   for i in data:
     if i.score == models.AccountStatsBase.NULL_SCORE:
       shame.append(i)
     else:
       tops.append(i)
   return respond(
       request, 'leaderboard.html', {'tops': tops, 'shame': shame, 'when': when})