Merge "Replace "attached" with observers"

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js

@@ -73,6 +73,7 @@
         type: Object,
         notify: true,
         value() { return {}; },
+        observer: '_viewStateChanged',
       },
       backPage: String,
       hasParent: Boolean,
@@ -87,6 +88,7 @@
       _diffPrefs: Object,
       _numFilesShown: {
         type: Number,
+        value: DEFAULT_NUM_FILES_SHOWN,
         observer: '_numFilesShownChanged',
       },
       _account: {
@@ -221,9 +223,6 @@
         }
       });
 
-      this._numFilesShown = this.viewState.numFilesShown ?
-          this.viewState.numFilesShown : DEFAULT_NUM_FILES_SHOWN;
-
       this.addEventListener('comment-save', this._handleCommentSave.bind(this));
       this.addEventListener('comment-discard',
           this._handleCommentDiscard.bind(this));
@@ -512,6 +511,11 @@
       }
     },
 
+    _viewStateChanged(viewState) {
+      this._numFilesShown = viewState.numFilesShown ?
+          viewState.numFilesShown : DEFAULT_NUM_FILES_SHOWN;
+    },
+
     _numFilesShownChanged(numFilesShown) {
       this.viewState.numFilesShown = numFilesShown;
     },

polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js

@@ -57,6 +57,7 @@
         type: Object,
         notify: true,
         value() { return {}; },
+        observer: '_changeViewStatehanged',
       },
 
       _patchRange: Object,
@@ -120,6 +121,7 @@
     observers: [
       '_getProjectConfig(_change.project)',
       '_getFiles(_changeNum, _patchRange.*)',
+      '_setReviewedObserver(_loggedIn, params.*)',
     ],
 
     keyBindings: {
@@ -141,21 +143,7 @@
     attached() {
       this._getLoggedIn().then(loggedIn => {
         this._loggedIn = loggedIn;
-        if (loggedIn) {
-          this._setReviewed(true);
-        }
       });
-      if (this.changeViewState.diffMode === null) {
-        // If screen size is small, always default to unified view.
-        this.$.restAPI.getPreferences().then(prefs => {
-          this.set('changeViewState.diffMode', prefs.default_diff_view);
-        });
-      }
-
-      if (this._path) {
-        this.fire('title-change',
-            {title: this._computeFileDisplayName(this._path)});
-      }
 
       this.$.cursor.push('diffs', this.$.diff);
     },
@@ -476,6 +464,21 @@
       });
     },
 
+    _changeViewStatehanged(changeViewState) {
+      if (changeViewState.diffMode === null) {
+        // If screen size is small, always default to unified view.
+        this.$.restAPI.getPreferences().then(prefs => {
+          this.set('changeViewState.diffMode', prefs.default_diff_view);
+        });
+      }
+    },
+
+    _setReviewedObserver(_loggedIn) {
+      if (_loggedIn) {
+        this._setReviewed(true);
+      }
+    },
+
     /**
      * If the URL hash is a diff address then configure the diff cursor.
      */
@@ -492,14 +495,15 @@
     },
 
     _pathChanged(path) {
+      if (path) {
+        this.fire('title-change',
+            {title: this._computeFileDisplayName(path)});
+      }
+
       if (this._fileList.length == 0) { return; }
 
       this.set('changeViewState.selectedFileIndex',
           this._fileList.indexOf(path));
-
-      if (this._loggedIn) {
-        this._setReviewed(true);
-      }
     },
 
     _getDiffURL(changeNum, patchRange, path) {
@@ -522,6 +526,7 @@
 
     _computeAvailablePatches(revisions) {
       const patchNums = [];
+      if (!revisions) { return patchNums; }
       for (const rev of Object.values(revisions)) {
         patchNums.push(rev._number);
       }

polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html

@@ -54,6 +54,8 @@ limitations under the License.
         getDiffChangeDetail() { return Promise.resolve(null); },
         getChangeFiles() { return Promise.resolve({}); },
         saveFileReviewed() { return Promise.resolve(); },
+        getDiffRobotComments() { return Promise.resolve(); },
+        getDiffDrafts() { return Promise.resolve(); },
       });
       element = fixture('basic');
     });
@@ -469,16 +471,21 @@ limitations under the License.
     });
 
     test('file review status', done => {
-      element._loggedIn = true;
-      element._changeNum = '42';
-      element._patchRange = {
-        basePatchNum: '1',
-        patchNum: '2',
-      };
-      element._fileList = ['/COMMIT_MSG'];
-      element._path = '/COMMIT_MSG';
+      stub('gr-rest-api-interface', {
+        getDiffComments() { return Promise.resolve({}); },
+      });
       const saveReviewedStub = sandbox.stub(element, '_saveReviewedState',
           () => Promise.resolve());
+      sandbox.stub(element.$.diff, 'reload');
+
+      element._loggedIn = true;
+      element.params = {
+        view: 'gr-diff-view',
+        changeNum: '42',
+        patchNum: '2',
+        basePatchNum: '1',
+        path: '/COMMIT_MSG',
+      };
 
       flush(() => {
         const commitMsg = Polymer.dom(element.root).querySelector(
@@ -625,7 +632,6 @@ limitations under the License.
     suite('_loadCommentMap', () => {
       test('empty', done => {
         stub('gr-rest-api-interface', {
-          getDiffRobotComments() { return Promise.resolve({}); },
           getDiffComments() { return Promise.resolve({}); },
         });
         element._loadCommentMap().then(map => {
@@ -636,7 +642,6 @@ limitations under the License.
 
       test('paths in patch range', done => {
         stub('gr-rest-api-interface', {
-          getDiffRobotComments() { return Promise.resolve({}); },
           getDiffComments() {
             return Promise.resolve({
               'path/to/file/one.cpp': [{patch_set: 3, message: 'lorem'}],
@@ -658,7 +663,6 @@ limitations under the License.
 
       test('empty for paths outside patch range', done => {
         stub('gr-rest-api-interface', {
-          getDiffRobotComments() { return Promise.resolve({}); },
           getDiffComments() {
             return Promise.resolve({
               'path/to/file/one.cpp': [{patch_set: 'PARENT', message: 'lorem'}],