Fix bug 32: Check for (and prevent) duplicate comments if URL contains 'resume=1'
authorGraeme McCutcheon <graememcc_firefox@graeme-online.co.uk>
Sun, 13 Jan 2013 15:49:57 +0000
changeset 1196 35e708ef89bcb4d6ec8974436d614a0ade6981f2
parent 1195 e445582c5dceab56c95e5d9d8a77bda9aaa67dc8
child 1197 f1e6133469b7d9184dfc450fcd809b1177014054
push id659
push useremorley@mozilla.com
push dateTue, 18 Jun 2013 13:49:17 +0000
bugs32
Fix bug 32: Check for (and prevent) duplicate comments if URL contains 'resume=1'
mcmerge/js/BugData.js
mcmerge/js/Remapper.js
mcmerge/js/Step.js
mcmerge/js/ViewerController.js
mcmerge/js/mcMerge.js
--- a/mcmerge/js/BugData.js
+++ b/mcmerge/js/BugData.js
@@ -3,22 +3,24 @@
 var BugData = {
   bugs: {},
   trackingFlag: null,
   statusFlag: null,
   fields: 'id,resolution,status,whiteboard,keywords,target_milestone,summary,product,component,flags,assigned_to',
   notYetLoaded: [],
   loadCallback: null,
   errorCallback: null,
+  checkComments: false,
   bugzilla: bz.createClient(),
 
-  load: function BD_load(bugs, loadCallback, errorCallback) {
+  load: function BD_load(bugs, checkComments, loadCallback, errorCallback) {
     this.notYetLoaded = bugs;
     this.loadCallback = loadCallback;
     this.errorCallback = errorCallback;
+    this.checkComments = checkComments;
     this.loadMore();    
   },
 
 
   loadMore: function BD_loadMore() {
     var batch = [];
     var limit = 500; // to avoid URI too long errors from BZAPI
     if (this.notYetLoaded.length < limit)
@@ -31,16 +33,18 @@ var BugData = {
 
   _realLoad: function BD_realLoad(bugs) {
     if (mcMerge.trackingFlag)
       this.trackingFlag = 'cf_' + mcMerge.trackingFlag;
     if (mcMerge.statusFlag)
       this.statusFlag = 'cf_' + mcMerge.statusFlag;
 
     var includeFields = this.fields;
+    if (this.checkComments)
+      includeFields += ',comments';
     if (this.trackingFlag)
       includeFields += ',' + this.trackingFlag;
     if (this.statusFlag)
       includeFields += ',' + this.statusFlag;
 
     bugs = {id : bugs, include_fields: includeFields};
 
     var self = this;
@@ -77,16 +81,18 @@ var BugData = {
     bug.status = UI.htmlEncode(bugObj.status);
     bug.milestone = UI.htmlEncode(bugObj.target_milestone);
     bug.summary = UI.htmlEncode(bugObj.summary);
     bug.product = bugObj.product;
     bug.id = bugObj.id;
     if (typeof bug.id == 'string')
       bug.id = UI.htmlEncode(bug.id);
 
+    if (this.checkComments)
+      bug.comments = bugObj.comments;
 
     bug.canResolve = !(bug.status == 'RESOLVED' || bug.status == 'VERIFIED');
     bug.canReopen = bug.resolution == 'FIXED';
 
     bug.isTracked = false;
     if (this.trackingFlag && bugObj[this.trackingFlag] == '+')
       bug.isTracked = true;
 
--- a/mcmerge/js/Remapper.js
+++ b/mcmerge/js/Remapper.js
@@ -28,16 +28,18 @@ var Remapper = {
    html += '<div class="grid-12">Set bug statuses to NEW (to allow you testing of bug resolution)';
    html += '<input type="checkbox" id="new"></div>';
    html += '<div class="grid-12">Add checkin-needed into keywords (to allow testing of checkin-needed removal)';
    html += '<input type="checkbox" id="checkin"></div>';
    html += '<div class="grid-12">Add [inbound] into whiteboard (to allow testing of [inbound] removal)';
    html += '<input type="checkbox" id="inbound"></div>';
    html += '<div class="grid-12">Add [fixed-in-fx-team] into whiteboard (to allow testing of [fixed-in-fx-team] removal)';
    html += '<input type="checkbox" id="fxteam"></div>';
+   html += '<div class="grid-12">Ignore comments (i.e. override the "Don\'t duplicate comments" mechanism)';
+   html += '<input type="checkbox" id="comments"></div>';
    html += '<hr>';
    return html;
   },
 
   makeHTMLForSubmit: function rm_makeHTMLForSubmit() {
     var html = '<div class="grid-12 divRight" id="remsubmit">';
     html += '  <button type="button" id="remapSubmit">Submit</button>';
     html += '</div>';
@@ -149,16 +151,26 @@ var Remapper = {
             BugData.bugs[b].whiteboard += '[fixed-in-fx-team]';
             BugData.bugs[b].summary += ' [fixed-in-fx-team]';
             fxteamMultiple = true;
           }
         }
       }
     }
 
+    if ($('#comments').prop('checked')) {
+      for (b in remaps) {
+        if (b == 'items')
+          continue;
+        if (b in BugData.bugs) {
+          BugData.bugs[b].comments = [{text: 'Dummy comment'}];
+        }
+      }
+    }
+
     if ($('#midair').prop('checked')) {
       if (remaps.items == 0) {
         this.error('You need to redirect at least 1 bug for debug midairs to work!');
         return;
       }
       remaps.midair = true;
     } else
      remaps.midair = false;
--- a/mcmerge/js/Step.js
+++ b/mcmerge/js/Step.js
@@ -40,16 +40,17 @@ function Step(name, callbacks, isBackout
   this.prependText = '';
 
   // The following are used for additional help text
   this.leaveOpenBugs = [];
   this.multiBugs = [];
   this.securityBugs = [];
   this.hasMilestones = [];
   this.trackedBugs = [];
+  this.haveComment = [];
 
   var options = {};
   if (Step.remaps.items > 0)
     options.test = true;
   this.unprivilegedLoader = bz.createClient(options);
 
   constructAttachedBugs(false);
   if (this.hasBackouts)
@@ -392,42 +393,42 @@ Step.prototype.postSubmit = function Ste
       BugData.bugs[bugID].milestone = sent.target_milestone;
     info.canResolve = false;
     info.shouldResolve = false;
     BugData.bugs[bugID].canReopen = false;
     info.canReopen = false;
   }
 
 
-  // Disallow comments if we just sent a comment
-  if ('comments' in sent) {
-    for (var j = 0; j < info.linkedChangesets.length; j++) {
-      var index = info.linkedChangesets[j];
+  // Update the intestsuite flag if we sent it
+  if ('flags' in sent) {
+    info.intestsuite = sent.flags[0].status;
+    BugData.bugs[bugID].intestsuite = sent.flags[0].status;
+  }
+
+  for (var j = 0; j < info.linkedChangesets.length; j++) {
+    var index = info.linkedChangesets[j];
+    // Disallow comments if we just sent a comment
+    if ('comments' in sent) {
       var attached = this.attachedBugs[index][bugID];
-      if (attached.canComment)
+      if (attached.canComment && attached.shouldComment) {
         attached.canComment = false;
-      if (attached.shouldComment)
         attached.shouldComment = false;
-      this.callbacks.uiUpdate(index, bugID);
+      }
     }
+    this.callbacks.uiUpdate(index, bugID);
   }
 
   // Disallow setting status- if we just sent it
   if ('cf_' + mcMerge.statusFlag in sent) {
     info.canSetStatus = false;
     info.shouldSetStatus = false;
     BugData.bugs[bugID].statusFlag = sent['cf_' + mcMerge.statusFlag];
   }
 
-  // Update the intestsuite flag if we sent it
-  if ('flags' in sent) {
-    info.intestsuite = sent.flags[0].status;
-    BugData.bugs[bugID].intestsuite = sent.flags[0].status;
-  }
-
   this.continueSubmit(i);
 };
 
 
 Step.prototype.continueSubmit = function Step_continueSubmit(i) {
   if (i + 1 >= this.sendData.length) {
     UI.updateProgressModal(100);
     if (this.retries.length > 0) {
@@ -571,17 +572,35 @@ Step.prototype.attachBugToCset = functio
     if (bug && bug.canSetTestsuite)
       this.bugInfo[bugID].intestsuite = bug.intestsuite;
 
     // Adjust the whiteboard the first time we see this bug
     if (bug)
       bug.whiteboard = this.adjustWhiteboard(bug.whiteboard, this.bugInfo[bugID].canReopen);
   }
 
-  attached.canComment = !!bug; // reserved for future use :-)
+  if (bug && bug.comments) {
+    var hasThisComment = false;
+    // Iterate backwards: if commented already, the changeset will be in one of the last comments
+    for (var i = bug.comments.length - 1; i > -1; i--) {
+      if (bug.comments[i].text && bug.comments[i].text.indexOf(attached.comment) != -1) {
+        hasThisComment = true;
+        if (this.haveComment.indexOf(bug.id) == -1)
+          this.haveComment.push(bug.id);
+        break;
+      }
+    }
+    attached.canComment = !hasThisComment;
+  } else if (bug && bug.comments === undefined)
+    attached.canComment = true;
+  else 
+    attached.canComment = false;
+
+  if (!attached.canComment)
+    attached.shouldComment = false;
 
   // Maintain indices of changesets linked to a particular bug, in order to coalesce comments
   this.bugInfo[bugID].linkedChangesets.push(index);
   this.bugInfo[bugID].linkedChangesets.sort(function compare(a, b) {return a-b;});
 
   if (!(index in this.attachedBugs))
     this.attachedBugs[index] = {};
 
@@ -929,35 +948,40 @@ Step.prototype.constructTextFor = functi
 //   - "has milestone" bugs (bugs that can be resolved, but already had a milestone)
 Step.prototype.getAdditionalHelpText = function Step_getAdditionalHelpText() {
   var text = '';
 
   var multiPost = ' associated with multiple changesets: the individual comments will be coalesced into a single comment.';
   var leaveOpenPost = ' "leave open" in the whiteboard, so the resolve flag has not been set.';
   var securityPost = ' restricted - m-cMerge was unable to load the relevant information from Bugzilla.';
   var milestonePost = ' a milestone set. You may wish to check it is correct before submitting.';
+  var alreadyCommentPost = ' to have already been commented with the correct changeset URL, so commenting there has been disabled.';
   var trackedPost = ' ' + mcMerge.trackingFlag + '+, so ' + mcMerge.statusFlag + ' will be set to "fixed".';
 
   var hashave = {singular: 'has', plural: 'have'};
+  var appearTo = {singular: 'appears', plural: 'appear'};
   var isare = {singular: 'is', plural: 'are'};
   var already = {singular: 'already has', plural: 'already have'};
 
   if (this.multiBugs.length > 0 || this.leaveOpenBugs.length > 0 ||
       this.securityBugs.length > 0)
     text += '<br>';
 
   if (this.multiBugs.length > 0)
     text += this.constructTextFor(this.multiBugs, multiPost);
 
   if (this.leaveOpenBugs.length > 0)
     text += this.constructTextFor(this.leaveOpenBugs, leaveOpenPost, hashave);
 
   if (this.securityBugs.length > 0)
     text += this.constructTextFor(this.securityBugs, securityPost);
 
+  if (this.haveComment.length > 0)
+    text += this.constructTextFor(this.haveComment, alreadyCommentPost, appearTo, true);
+
   if (this.hasMilestones.length > 0)
     text += this.constructTextFor(this.hasMilestones, milestonePost, already, true);
 
   if (this.trackedBugs.length > 0 && !Config.treeInfo[Config.treeName].unconditionalFlag)
     text += this.constructTextFor(this.trackedBugs, trackedPost, isare);
 
   return text;
 };
--- a/mcmerge/js/ViewerController.js
+++ b/mcmerge/js/ViewerController.js
@@ -1,16 +1,17 @@
 "use strict";
 
 var ViewerController = {
-  init: function vc_Init(remap) {
+  init: function vc_Init(remap, resume) {
     this.remap = remap;
     this.currentStep = -1;
     this.maxStep = -1;
     this.steps = [];
+    this.resume = resume;
   },
 
 
   getCurrentStep: function vc_getCurrentStep() {
     if (this.currentStep != -1)
       return this.steps[this.currentStep];
   },
 
@@ -106,17 +107,17 @@ var ViewerController = {
       return;
     }
 
     // Kick off load if not
     var self = this;
     var loadCallback = function vc_onAddBugLoadCallback() {
       self.addBug(index, input);
     };
-    BugData.load([input], loadCallback, null);
+    BugData.load([input], this.resume, loadCallback, null);
   },
 
 
   onChangeBug: function vc_onAddBug(index, bug, input) {
     UI.showLoadingOverlay();
 
     // Verify input is valid
     input = input.trim();
--- a/mcmerge/js/mcMerge.js
+++ b/mcmerge/js/mcMerge.js
@@ -1,14 +1,15 @@
 "use strict";
 
 var mcMerge = {
   debug: false,
   expand: false,
   remap: false,
+  resume: false,
   tree: null,
   trackingFlag: null,
   statusFlag: null,
 
   stageTypes: [{name: 'foundBackouts'},
     {name: 'notFoundBackouts'},
     {name: 'merges'},
     {name: 'others'},
@@ -281,17 +282,17 @@ var mcMerge = {
     var loadCallback = function mcM_loadBugsLoadCallback() {
      self.onBugLoad();
     };
 
     var errorCallback = function mcM_loadBugsErrorCallback(jqResponse, textStatus, errorThrown) {
       self.ajaxError(jqResponse, textStatus, errorThrown);
     };
 
-    BugData.load(bugArray, loadCallback, errorCallback);
+    BugData.load(bugArray, this.resume, loadCallback, errorCallback);
   },
 
 
   // Load options for options menu from Bugzilla config
   loadConfigurationFromBZ: function mcM_loadConfigurationFromBZ() {
     this.loading = 'version';
     UI.showLoadingMessage('Loading Bugzilla configuration...');
     var self = this;
@@ -382,17 +383,17 @@ var mcMerge = {
     this.remaps = remaps;
     this.showSteps();
   },
 
 
   showSteps: function mcM_showSteps() {
     Step.remaps = this.remaps;
     Viewer.expand = this.expand;
-    ViewerController.init(this.remap);
+    ViewerController.init(this.remap, this.resume);
     Viewer.init();
 
     // How many stages do we have?
     for (var i = 0; i < this.stageTypes.length; i++) {
       var stageName = this.stageTypes[i].name;
 
       if (PushData[stageName].length == 0)
         continue;
@@ -467,16 +468,18 @@ var mcMerge = {
         paramsObj[p[0]] = p[1];
       }
       if ('debug' in paramsObj)
         this.debug = (paramsObj['debug'] == '1');
       if ('expand' in paramsObj)
         this.expand = (paramsObj['expand'] == '1');
       if ('remap' in paramsObj)
         this.remap = (paramsObj['remap'] == '1');
+      if ('resume' in paramsObj)
+        this.resume = (paramsObj['resume'] == '1');
 
       if ('error' in paramsObj)
         return self.errorPage(paramsObj);
 
       if ('cset' in paramsObj) {
         var cset = paramsObj['cset'];
 
         if ('tree' in paramsObj) {
@@ -530,16 +533,20 @@ var mcMerge = {
     // Maintain expand state across page load
     if (this.expand)
       maintained.push('expand=1');
 
     // Maintain remap state across page load
     if (this.remap)
       maintained.push('remap=1');
 
+    // Maintain resume state across page load
+    if (this.resume)
+      maintained.push('resume=1');
+
     var maintainedQuery = maintained.join('&');
     if (!query && maintainedQuery.length > 0)
       newURL += '?';
     else if (query && maintainedQuery.length > 0)
       newURL += '&';
     newURL += maintainedQuery;
 
     if (Config.supportsHistory) {