From b3caf48ffa15f851f64c3da6159aac01df49f7e9 Mon Sep 17 00:00:00 2001
From: Andrew Bonventre <andybons@chromium.org>
Date: Wed, 6 Jan 2016 10:43:30 -0500
Subject: [PATCH] Persist diff context length on server

This code will eventually live within a diff preference pane
once options other than context are supported.

Change-Id: I3c752a18d163a55cb3398c0a46544593b0a9faed
---
 polygerrit-ui/app/elements/gr-app.html       | 16 +++-
 polygerrit-ui/app/elements/gr-diff-view.html |  5 ++
 polygerrit-ui/app/elements/gr-diff.html      | 88 +++++++++++++++-----
 polygerrit-ui/app/test/gr-diff-test.html     | 40 ++++++++-
 4 files changed, 123 insertions(+), 26 deletions(-)

diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index 3b4efaa81e..c3c520b7f5 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -87,6 +87,9 @@ limitations under the License.
     <gr-ajax auto url="/accounts/self/detail" last-response="{{account}}"></gr-ajax>
     <gr-ajax auto url="/config/server/info" last-response="{{config}}"></gr-ajax>
     <gr-ajax auto url="/config/server/version" last-response="{{version}}"></gr-ajax>
+    <gr-ajax id="diffPreferencesXHR"
+        url="/accounts/self/preferences.diff"
+        last-response="{{_diffPreferences}}"></gr-ajax>
     <header role="banner">
       <a href="/" class="bigTitle">PolyGerrit</a>
       <div class="headerRightItems">
@@ -109,7 +112,7 @@ limitations under the License.
             params="[[params]]"></gr-change-view>
       </template>
       <template is="dom-if" if="{{_showDiffView}}" restamp="true">
-        <gr-diff-view params="[[params]]"></gr-diff-view>
+        <gr-diff-view prefs="{{_diffPreferences}}" params="[[params]]"></gr-diff-view>
       </template>
     </main>
     <footer role="contentinfo">
@@ -175,6 +178,8 @@ limitations under the License.
           type: String,
           observer: '_routeChanged',
         },
+
+        _diffPreferences: Object,
       },
 
       get loggedIn() {
@@ -185,6 +190,15 @@ limitations under the License.
         this._resolveAccountReady();
         this.$.accountContainer.classList.toggle('loggedIn', this.loggedIn);
         this.$.accountContainer.classList.toggle('loggedOut', !this.loggedIn);
+        if (this.loggedIn) {
+          this.$.diffPreferencesXHR.generateRequest();
+        } else {
+          // These defaults should match the defaults in
+          // gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java
+          this._diffPreferences = {
+            context: 10,
+          };
+        }
       },
 
       _configChanged: function(config) {
diff --git a/polygerrit-ui/app/elements/gr-diff-view.html b/polygerrit-ui/app/elements/gr-diff-view.html
index fd8c5e23c7..e32e593586 100644
--- a/polygerrit-ui/app/elements/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/gr-diff-view.html
@@ -120,6 +120,7 @@ limitations under the License.
     <gr-diff id="diff"
         auto
         change-num="[[_changeNum]]"
+        prefs="{{prefs}}"
         patch-range="[[_patchRange]]"
         path="[[_path]]"
         available-patches="[[_computeAvailablePatches(_change.revisions)]]"
@@ -138,6 +139,10 @@ limitations under the License.
       ],
 
       properties: {
+        prefs: {
+          type: Object,
+          notify: true,
+        },
         keyEventTarget: {
           type: Object,
           value: function() {
diff --git a/polygerrit-ui/app/elements/gr-diff.html b/polygerrit-ui/app/elements/gr-diff.html
index 215ba8e427..454bf9b348 100644
--- a/polygerrit-ui/app/elements/gr-diff.html
+++ b/polygerrit-ui/app/elements/gr-diff.html
@@ -68,7 +68,7 @@ limitations under the License.
           change-num="[[changeNum]]"
           patch-range="[[patchRange]]"
           available-patches="[[availablePatches]]"></gr-patch-range-select>
-      <div class="contextControl">
+      <div class="contextControl" hidden$="[[!prefs.context]]" hidden>
         Context:
           <select id="contextSelect" on-change="_handleContextSelectChange">
             <option value="3">3 lines</option>
@@ -77,7 +77,7 @@ limitations under the License.
             <option value="50">50 lines</option>
             <option value="75">75 lines</option>
             <option value="100">100 lines</option>
-            <option value="ALL">Whole file</option>
+            <option value="-1">Whole file</option>
           </select>
       </div>
     </div>
@@ -138,10 +138,18 @@ limitations under the License.
           type: Number,
           value: 80,
         },
-        _context: {
-          type: Number,
-          value: 10,
-          observer: '_contextChanged',
+        prefs: {
+          type: Object,
+          notify: true,
+        },
+        _prefsReady: {
+          type: Object,
+          readOnly: true,
+          value: function() {
+            return new Promise(function(resolve) {
+              this._resolvePrefsReady = resolve;
+            }.bind(this));
+          },
         },
         _baseComments: Array,
         _comments: Array,
@@ -170,11 +178,17 @@ limitations under the License.
           type: Boolean,
           value: false,
         },
+        _initialRenderComplete: {
+          type: Boolean,
+          value: false,
+        },
         _diffRequestsPromise: Object,  // Used for testing.
+        _diffPreferencesPromise: Object,  // Used for testing.
       },
 
       observers: [
-        '_diffOptionsChanged(changeNum, patchRange, path)'
+        '_prefsChanged(prefs.*)',
+        '_diffOptionsChanged(changeNum, patchRange, path)',
       ],
 
       ready: function() {
@@ -188,18 +202,25 @@ limitations under the License.
         this.$.rightDiff.scrollToLine(lineNum);
       },
 
-      _contextChanged: function(context) {
-        if (context == Infinity) {
-          this.$.contextSelect.value = 'ALL';
-        } else {
-          this.$.contextSelect.value = context;
+      _prefsChanged: function(changeRecord) {
+        var prefs = changeRecord.base;
+
+        this.$.contextSelect.value = prefs.context;
+
+        if (this._initialRenderComplete) {
+          this._render();
         }
+
+        this._resolvePrefsReady(prefs);
       },
 
       _diffOptionsChanged: function(changeNum, patchRange, path) {
         if (!this.auto) { return; }
 
-        var promises = [this.$.diffXHR.generateRequest().completes];
+        var promises = [
+          this._prefsReady,
+          this.$.diffXHR.generateRequest().completes
+        ];
 
         var basePatchNum = patchRange.basePatchNum;
         var patchNum = patchRange.patchNum;
@@ -224,6 +245,8 @@ limitations under the License.
         this.async(function() {
           this.fire('render', null, {bubbles: false});
         }.bind(this), 1);
+
+        this._initialRenderComplete = true;
       },
 
       _getCommentsAndDrafts: function(basePatchNum, loggedIn) {
@@ -302,12 +325,28 @@ limitations under the License.
 
       _handleContextSelectChange: function(e) {
         var selectEl = Polymer.dom(e).rootTarget;
-        if (selectEl.value == 'ALL') {
-          this._context = Infinity;
-        } else {
-          this._context = parseInt(selectEl.value, 10);
-        }
-        this._render();
+        this.set('prefs.context', parseInt(selectEl.value, 10));
+
+        app.accountReady.then(function() {
+          if (!this._loggedIn) { return; }
+
+          this._diffPreferencesPromise =
+              this._saveDiffPreferences().catch(function(err) {
+            alert('Oops. Something went wrong. Check the console and bug the ' +
+                  'PolyGerrit team for assistance.');
+            throw err;
+          });
+        }.bind(this));
+      },
+
+      _saveDiffPreferences: function() {
+        var xhr = document.createElement('gr-request');
+        this._diffPreferencesPromise = xhr.send({
+          method: 'PUT',
+          url: '/accounts/self/preferences.diff',
+          body: this.prefs,
+        });
+        return this._diffPreferencesPromise;
       },
 
       _handleExpandContext: function(e) {
@@ -424,13 +463,18 @@ limitations under the License.
           }
         };
         var content = this._diffResponse.content;
+        var context = this.prefs.context;
+        if (context == -1) {
+          // Show the entire file.
+          context = Infinity;
+        }
         for (var i = 0; i < content.length; i++) {
           if (i == 0) {
-            ctx.skipRange = [0, this._context];
+            ctx.skipRange = [0, context];
           } else if (i == content.length - 1) {
-            ctx.skipRange = [this._context, 0];
+            ctx.skipRange = [context, 0];
           } else {
-            ctx.skipRange = [this._context, this._context];
+            ctx.skipRange = [context, context];
           }
           ctx.diffChunkIndex = i;
           this._addDiffChunk(ctx, content[i], leftSide, rightSide);
diff --git a/polygerrit-ui/app/test/gr-diff-test.html b/polygerrit-ui/app/test/gr-diff-test.html
index a1c158542f..6ed4a8ff25 100644
--- a/polygerrit-ui/app/test/gr-diff-test.html
+++ b/polygerrit-ui/app/test/gr-diff-test.html
@@ -48,6 +48,9 @@ limitations under the License.
       element = fixture('basic');
       element.changeNum = 42;
       element.path = 'sieve.go';
+      element.prefs = {
+        context: 10,
+      };
 
       server = sinon.fakeServer.create();
       server.respondWith(
@@ -184,6 +187,18 @@ limitations under the License.
           }),
         ]
       );
+
+      server.respondWith(
+        'PUT',
+        '/accounts/self/preferences.diff',
+        [
+          200,
+          { 'Content-Type': 'application/json' },
+          ')]}\'\n' +
+          JSON.stringify({ context: 25 }),
+        ]
+      );
+
     });
 
     teardown(function() {
@@ -225,7 +240,7 @@ limitations under the License.
     });
 
     test('comment rendering', function(done) {
-      element._context = Infinity;
+      element.prefs.context = -1;
       element._loggedIn = true;
       element.patchRange = {
         basePatchNum: 1,
@@ -394,7 +409,7 @@ limitations under the License.
     });
 
     test('context', function() {
-      element._context = 3;
+      element.prefs.context = 3;
       element._diffResponse = {
         content: [
           {
@@ -468,7 +483,26 @@ limitations under the License.
       assert.equal(leftContext.start, 17);
       assert.equal(leftContext.end, 19);
     });
+
+    test('save context', function(done) {
+      element._loggedIn = false;
+
+      var contextSelectEl = element.$.contextSelect;
+      assert.ok(contextSelectEl);
+      contextSelectEl.value = '50';
+      element.fire('change', {}, {node: contextSelectEl});
+      assert.isTrue(element._diffPreferencesPromise == null);
+
+      element._loggedIn = true;
+      contextSelectEl.value = '25';
+      element.fire('change', {}, {node: contextSelectEl});
+      server.respond();
+
+      element._diffPreferencesPromise.then(function(req) {
+        assert.equal(req.xhr.requestBody, JSON.stringify(element.prefs));
+        done();
+      });
+    });
   });
 
-
 </script>