Merge "Fix index out of bounds exception when nothing able to merge"
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 8553634..b7311d6 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -187,6 +187,19 @@
the link:access-control.html#category_submit[Submit] access right is
assigned.
+** [[revert]]`Revert`:
++
+Reverts the change via creating a new one.
++
+The `Revert` button is available if the change has been submitted.
++
+When the `Revert` button is pressed, a panel will appear to allow
+the user to enter a commit message for the reverting change.
++
+Once a revert change is created, the original author and any reviewers
+of the original change are added as reviewers and a message is posted
+to the original change linking to the revert.
+
** [[abandon]]`Abandon`:
+
Abandons the change.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
index 5f48daa..86e068c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
@@ -24,12 +24,14 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
@@ -66,6 +68,8 @@
import java.io.IOException;
import java.sql.Timestamp;
import java.text.MessageFormat;
+import java.util.HashSet;
+import java.util.Set;
@Singleton
public class Revert implements RestModifyView<ChangeResource, RevertInput>,
@@ -82,6 +86,7 @@
private final RevertedSender.Factory revertedSenderFactory;
private final ChangeJson.Factory json;
private final PersonIdent serverIdent;
+ private final ApprovalsUtil approvalsUtil;
@Inject
Revert(Provider<ReviewDb> db,
@@ -93,7 +98,8 @@
PatchSetUtil psUtil,
RevertedSender.Factory revertedSenderFactory,
ChangeJson.Factory json,
- @GerritPersonIdent PersonIdent serverIdent) {
+ @GerritPersonIdent PersonIdent serverIdent,
+ ApprovalsUtil approvalsUtil) {
this.db = db;
this.repoManager = repoManager;
this.changeInserterFactory = changeInserterFactory;
@@ -104,6 +110,7 @@
this.revertedSenderFactory = revertedSenderFactory;
this.json = json;
this.serverIdent = serverIdent;
+ this.approvalsUtil = approvalsUtil;
}
@Override
@@ -182,6 +189,13 @@
.setTopic(changeToRevert.getTopic());
ins.setMessage("Uploaded patch set 1.");
+ Set<Account.Id> reviewers = new HashSet<>();
+ reviewers.add(changeToRevert.getOwner());
+ reviewers.addAll(
+ approvalsUtil.getReviewers(db.get(), ctl.getNotes()).all());
+ reviewers.remove(user.getAccountId());
+ ins.setReviewers(reviewers);
+
try (BatchUpdate bu = updateFactory.create(
db.get(), project, user, now)) {
bu.setRepository(git, revWalk, oi);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index b55b416..0e62600 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -43,10 +44,12 @@
protected final NotesMigration migration;
protected final ChangeNoteUtil noteUtil;
protected final String anonymousCowardName;
- protected final ChangeNotes notes;
protected final Account.Id accountId;
protected final PersonIdent authorIdent;
protected final Date when;
+
+ @Nullable private final ChangeNotes notes;
+ private final Change change;
private final PersonIdent serverIdent;
protected PatchSet.Id psId;
@@ -59,15 +62,16 @@
String anonymousCowardName,
ChangeNoteUtil noteUtil,
Date when) {
- this(
- migration,
- noteUtil,
- serverIdent,
- anonymousCowardName,
- ctl.getNotes(),
- accountId(ctl.getUser()),
- ident(noteUtil, serverIdent, anonymousCowardName, ctl.getUser(), when),
- when);
+ this.migration = migration;
+ this.noteUtil = noteUtil;
+ this.serverIdent = new PersonIdent(serverIdent, when);
+ this.anonymousCowardName = anonymousCowardName;
+ this.notes = ctl.getNotes();
+ this.change = notes.getChange();
+ this.accountId = accountId(ctl.getUser());
+ this.authorIdent =
+ ident(noteUtil, serverIdent, anonymousCowardName, ctl.getUser(), when);
+ this.when = when;
}
protected AbstractChangeUpdate(
@@ -75,15 +79,21 @@
ChangeNoteUtil noteUtil,
PersonIdent serverIdent,
String anonymousCowardName,
- ChangeNotes notes,
+ @Nullable ChangeNotes notes,
+ @Nullable Change change,
Account.Id accountId,
PersonIdent authorIdent,
Date when) {
+ checkArgument(
+ (notes != null && change == null)
+ || (notes == null && change != null),
+ "exactly one of notes or change required");
this.migration = migration;
this.noteUtil = noteUtil;
this.serverIdent = new PersonIdent(serverIdent, when);
this.anonymousCowardName = anonymousCowardName;
this.notes = notes;
+ this.change = change != null ? change : notes.getChange();
this.accountId = accountId;
this.authorIdent = authorIdent;
this.when = when;
@@ -114,15 +124,16 @@
}
public Change.Id getId() {
- return notes.getChangeId();
+ return change.getId();
}
+ @Nullable
public ChangeNotes getNotes() {
return notes;
}
public Change getChange() {
- return notes.getChange();
+ return change;
}
public Date getWhen() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 84e647e..7b59a47 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -21,6 +21,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -62,6 +63,8 @@
public interface Factory {
ChangeDraftUpdate create(ChangeNotes notes, Account.Id accountId,
PersonIdent authorIdent, Date when);
+ ChangeDraftUpdate create(Change change, Account.Id accountId,
+ PersonIdent authorIdent, Date when);
}
@AutoValue
@@ -76,8 +79,8 @@
private final AllUsersName draftsProject;
- private List<PatchLineComment> put;
- private Set<Key> delete;
+ private List<PatchLineComment> put = new ArrayList<>();
+ private Set<Key> delete = new HashSet<>();
@AssistedInject
private ChangeDraftUpdate(
@@ -90,11 +93,25 @@
@Assisted Account.Id accountId,
@Assisted PersonIdent authorIdent,
@Assisted Date when) {
- super(migration, noteUtil, serverIdent, anonymousCowardName, notes,
+ super(migration, noteUtil, serverIdent, anonymousCowardName, notes, null,
accountId, authorIdent, when);
this.draftsProject = allUsers;
- this.put = new ArrayList<>();
- this.delete = new HashSet<>();
+ }
+
+ @AssistedInject
+ private ChangeDraftUpdate(
+ @GerritPersonIdent PersonIdent serverIdent,
+ @AnonymousCowardName String anonymousCowardName,
+ NotesMigration migration,
+ AllUsersName allUsers,
+ ChangeNoteUtil noteUtil,
+ @Assisted Change change,
+ @Assisted Account.Id accountId,
+ @Assisted PersonIdent authorIdent,
+ @Assisted Date when) {
+ super(migration, noteUtil, serverIdent, anonymousCowardName, null, change,
+ accountId, authorIdent, when);
+ this.draftsProject = allUsers;
}
public void putComment(PatchLineComment c) {
@@ -179,14 +196,17 @@
// If reading from changes is enabled, then the old DraftCommentNotes
// already parsed the revision notes. We can reuse them as long as the ref
// hasn't advanced.
- DraftCommentNotes draftNotes =
- getNotes().load().getDraftCommentNotes();
- if (draftNotes != null) {
- ObjectId idFromNotes =
- firstNonNull(draftNotes.getRevision(), ObjectId.zeroId());
- RevisionNoteMap rnm = draftNotes.getRevisionNoteMap();
- if (idFromNotes.equals(curr) && rnm != null) {
- return rnm;
+ ChangeNotes changeNotes = getNotes();
+ if (changeNotes != null) {
+ DraftCommentNotes draftNotes =
+ changeNotes.load().getDraftCommentNotes();
+ if (draftNotes != null) {
+ ObjectId idFromNotes =
+ firstNonNull(draftNotes.getRevision(), ObjectId.zeroId());
+ RevisionNoteMap rnm = draftNotes.getRevisionNoteMap();
+ if (idFromNotes.equals(curr) && rnm != null) {
+ return rnm;
+ }
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
index 5f16eb4..5105fc0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
@@ -116,7 +116,6 @@
private final AccountCache accountCache;
private final ChangeDraftUpdate.Factory draftUpdateFactory;
- private final ChangeNotes.Factory notesFactory;
private final ChangeNoteUtil changeNoteUtil;
private final ChangeUpdate.Factory updateFactory;
private final NoteDbUpdateManager.Factory updateManagerFactory;
@@ -129,7 +128,6 @@
ChangeRebuilderImpl(SchemaFactory<ReviewDb> schemaFactory,
AccountCache accountCache,
ChangeDraftUpdate.Factory draftUpdateFactory,
- ChangeNotes.Factory notesFactory,
ChangeNoteUtil changeNoteUtil,
ChangeUpdate.Factory updateFactory,
NoteDbUpdateManager.Factory updateManagerFactory,
@@ -140,7 +138,6 @@
super(schemaFactory);
this.accountCache = accountCache;
this.draftUpdateFactory = draftUpdateFactory;
- this.notesFactory = notesFactory;
this.changeNoteUtil = changeNoteUtil;
this.updateFactory = updateFactory;
this.updateManagerFactory = updateManagerFactory;
@@ -388,8 +385,7 @@
labelNameComparator = Ordering.natural();
}
ChangeUpdate update = updateFactory.create(
- notesFactory.createWithAutoRebuildingDisabled(
- change, manager.getChangeRepo().cmds),
+ change,
events.getAccountId(),
events.newAuthorIdent(),
events.getWhen(),
@@ -406,13 +402,12 @@
private void flushEventsToDraftUpdate(NoteDbUpdateManager manager,
EventList<PatchLineCommentEvent> events, Change change)
- throws OrmException, IOException {
+ throws OrmException {
if (events.isEmpty()) {
return;
}
ChangeDraftUpdate update = draftUpdateFactory.create(
- notesFactory.createWithAutoRebuildingDisabled(
- change, manager.getChangeRepo().cmds),
+ change,
events.getAccountId(),
events.newAuthorIdent(),
events.getWhen());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 3c7f909..2af7bbf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -95,7 +95,7 @@
public interface Factory {
ChangeUpdate create(ChangeControl ctl);
ChangeUpdate create(ChangeControl ctl, Date when);
- ChangeUpdate create(ChangeNotes notes, @Nullable Account.Id accountId,
+ ChangeUpdate create(Change change, @Nullable Account.Id accountId,
PersonIdent authorIdent, Date when,
Comparator<String> labelNameComparator);
@@ -204,12 +204,12 @@
NoteDbUpdateManager.Factory updateManagerFactory,
ChangeDraftUpdate.Factory draftUpdateFactory,
ChangeNoteUtil noteUtil,
- @Assisted ChangeNotes notes,
+ @Assisted Change change,
@Assisted @Nullable Account.Id accountId,
@Assisted PersonIdent authorIdent,
@Assisted Date when,
@Assisted Comparator<String> labelNameComparator) {
- super(migration, noteUtil, serverIdent, anonymousCowardName, notes,
+ super(migration, noteUtil, serverIdent, anonymousCowardName, null, change,
accountId, authorIdent, when);
this.accountCache = accountCache;
this.draftUpdateFactory = draftUpdateFactory;
@@ -328,8 +328,14 @@
@VisibleForTesting
ChangeDraftUpdate createDraftUpdateIfNull() {
if (draftUpdate == null) {
- draftUpdate =
- draftUpdateFactory.create(getNotes(), accountId, authorIdent, when);
+ ChangeNotes notes = getNotes();
+ if (notes != null) {
+ draftUpdate =
+ draftUpdateFactory.create(notes, accountId, authorIdent, when);
+ } else {
+ draftUpdate = draftUpdateFactory.create(
+ getChange(), accountId, authorIdent, when);
+ }
}
return draftUpdate;
}
@@ -428,10 +434,13 @@
// If reading from changes is enabled, then the old ChangeNotes already
// parsed the revision notes. We can reuse them as long as the ref hasn't
// advanced.
- ObjectId idFromNotes =
- firstNonNull(getNotes().load().getRevision(), ObjectId.zeroId());
- if (idFromNotes.equals(curr)) {
- return checkNotNull(getNotes().revisionNoteMap);
+ ChangeNotes notes = getNotes();
+ if (notes != null) {
+ ObjectId idFromNotes =
+ firstNonNull(notes.load().getRevision(), ObjectId.zeroId());
+ if (idFromNotes.equals(curr)) {
+ return checkNotNull(getNotes().revisionNoteMap);
+ }
}
}
NoteMap noteMap = NoteMap.read(rw.getObjectReader(), rw.parseCommit(curr));
diff --git a/plugins/replication b/plugins/replication
index 945c842..43af15b 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 945c842f9c884469ec0fb2d883d6cda552e97747
+Subproject commit 43af15baab9b2312677000c47313026f34352abb
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index dd63764..544ad4c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -62,10 +62,7 @@
},
changeNum: String,
patchNum: String,
- commitInfo: {
- type: Object,
- readOnly: true,
- },
+ commitInfo: Object,
_loading: {
type: Boolean,
value: true,
@@ -237,11 +234,6 @@
_handleRevertDialogConfirm: function() {
var el = this.$.confirmRevertDialog;
- if (!el.message) {
- // TODO(viktard): Fix validation.
- alert('The revert commit message can’t be empty.');
- return;
- }
this.$.overlay.close();
el.hidden = false;
this._fireAction(
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 098e097..b0f5a5c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -221,12 +221,6 @@
fireActionStub.restore();
});
- test('validation', function() {
- element._handleRevertDialogConfirm();
- assert.notOk(fireActionStub.called);
- assert.ok(alertStub.called);
- });
-
test('works', function() {
var revertButton = element.$$('gr-button[data-action-key="revert"]');
MockInteractions.tap(revertButton);
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index d416e14..5f76a79 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -104,6 +104,18 @@
<span class="value">[[change.branch]]</span>
</section>
<section>
+ <span class="title">Commit</span>
+ <span class="value">
+ <template is="dom-if" if="[[_computeShowWebLink(commitInfo)]]">
+ <a target="_blank"
+ href$="[[_computeWebLink(commitInfo)]]">[[_computeShortHash(change)]]</a>
+ </template>
+ <template is="dom-if" if="[[!_computeShowWebLink(commitInfo)]]">
+ [[_computeShortHash(change)]]
+ </template>
+ </span>
+ </section>
+ <section>
<span class="title">Topic</span>
<span class="value">[[change.topic]]</span>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 4fa98ca..8e2e000 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -27,6 +27,7 @@
properties: {
change: Object,
+ commitInfo: Object,
mutable: Boolean,
serverConfig: Object,
},
@@ -35,6 +36,18 @@
Gerrit.RESTClientBehavior,
],
+ _computeShowWebLink: function(commitInfo) {
+ return commitInfo.web_links && commitInfo.web_links.length;
+ },
+
+ _computeWebLink: function(commitInfo) {
+ return '../../' + commitInfo.web_links[0].url;
+ },
+
+ _computeShortHash: function(change) {
+ return change.current_revision.slice(0, 6);
+ },
+
_computeHideStrategy: function(change) {
return !this.changeIsOpen(change.status);
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 3175517..78d4123 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -248,6 +248,7 @@
<div class="changeInfo-column changeMetadata">
<gr-change-metadata
change="[[_change]]"
+ commit-info="[[_commitInfo]]"
server-config="[[serverConfig]]"
mutable="[[_loggedIn]]"></gr-change-metadata>
<gr-change-actions id="actions"
@@ -272,6 +273,7 @@
</div>
</section>
<gr-file-list id="fileList"
+ change="[[_change]]"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
comments="[[_comments]]"
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
index a3ab470..979a06a 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
@@ -23,7 +23,6 @@
<style>
:host {
display: block;
- width: 30em;
}
:host([disabled]) {
opacity: .5;
@@ -31,19 +30,18 @@
}
label {
cursor: pointer;
+ display: block;
+ width: 100%;
}
iron-autogrow-textarea {
+ font-family: var(--monospace-font-family);
padding: 0;
- }
- .main label,
- .main input[type="text"] {
- display: block;
- font: inherit;
- width: 100%;
- }
- .main .message {
- border: groove;
- width: 100%;
+ width: 73ch; /* Add a char to account for the border. */
+
+ --iron-autogrow-textarea {
+ border: 1px solid #ddd;
+ font-family: var(--monospace-font-family);
+ }
}
</style>
<gr-confirm-dialog
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
index 5371ce5..ff0fc32 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
@@ -34,7 +34,6 @@
message: String,
commitInfo: {
type: Object,
- readOnly: true,
observer: '_commitInfoChanged',
},
},
@@ -49,7 +48,8 @@
new RegExp('\n{1,2}' + revertCommitText + '\\w+.\n*', 'gm');
commitMessage = commitMessage.replace(previousRevertText, '');
this.message = 'Revert "' + commitMessage + '"\n\n' +
- revertCommitText + commitInfo.commit + '.';
+ revertCommitText + commitInfo.commit + '.\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>\n\n';
},
_handleConfirmTap: function(e) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 976cf9b..f15c654 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -170,6 +170,8 @@
</div>
</div>
<gr-diff hidden
+ project="[[change.project]]"
+ commit="[[change.current_revision]]"
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
path="[[file.__path]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 3257469..82c47d6 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -36,6 +36,7 @@
type: Object,
value: function() { return document.body; },
},
+ change: Object,
_files: {
type: Array,
@@ -332,11 +333,14 @@
for (var node = el; node; node = node.offsetParent) {
top += node.offsetTop;
}
- if (this._showInlineDiffs) {
- window.scrollTo(0, top - this.topMargin);
- } else {
- window.scrollTo(0, top - document.body.clientHeight / 2);
+
+ // Don't scroll if it's already in view.
+ if (top > window.pageYOffset + this.topMargin &&
+ top < window.pageYOffset + window.innerHeight - el.clientHeight) {
+ return;
}
+
+ window.scrollTo(0, top - document.body.clientHeight / 2);
},
_computeFileSelected: function(index, selectedIndex) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index aa5d8bd..7304ac7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -61,10 +61,8 @@
}
var draft = this._newDraft(opt_lineNum);
+ draft.__editing = true;
this.push('comments', draft);
- this.async(function() {
- this._commentElWithDraftID(draft.__draftID).editing = true;
- }.bind(this), 1);
},
_getLoggedIn: function() {
@@ -121,13 +119,8 @@
function(line) { return ' > ' + line; }).join('\n') + '\n\n';
}
var reply = this._newReply(comment.id, comment.line, quoteStr);
+ reply.__editing = true;
this.push('comments', reply);
-
- // Allow the reply to render in the dom-repeat.
- this.async(function() {
- var commentEl = this._commentElWithDraftID(reply.__draftID);
- commentEl.editing = true;
- }.bind(this), 1);
},
_handleCommentDone: function(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 525bac0..7a15754 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -15,6 +15,7 @@
'use strict';
var STORAGE_DEBOUNCE_INTERVAL = 400;
+ var UPDATE_DEBOUNCE_INTERVAL = 500;
Polymer({
is: 'gr-diff-comment',
@@ -43,11 +44,18 @@
* @event comment-save
*/
+ /**
+ * Fired when this comment is updated.
+ *
+ * @event comment-update
+ */
+
properties: {
changeNum: String,
comment: {
type: Object,
notify: true,
+ observer: '_commentChanged',
},
disabled: {
type: Boolean,
@@ -81,6 +89,10 @@
'_loadLocalDraft(changeNum, patchNum, comment)',
],
+ detached: function() {
+ this.flushDebouncer('fire-update');
+ },
+
save: function() {
this.comment.message = this._messageText;
this.disabled = true;
@@ -106,8 +118,7 @@
}
this.comment = comment;
this.editing = false;
- this.fire('comment-save');
-
+ this.fire('comment-save', {comment: this.comment});
return obj;
}.bind(this));
}.bind(this)).catch(function(err) {
@@ -116,6 +127,16 @@
}.bind(this));
},
+ _commentChanged: function(comment) {
+ this.editing = !!comment.__editing;
+ },
+
+ _fireUpdate: function() {
+ this.debounce('fire-update', function() {
+ this.fire('comment-update', {comment: this.comment});
+ }, UPDATE_DEBOUNCE_INTERVAL);
+ },
+
_draftChanged: function(draft) {
this.$.container.classList.toggle('draft', draft);
},
@@ -134,6 +155,10 @@
if (this.comment && this.comment.id) {
this.$$('.cancel').hidden = !editing;
}
+ if (this.comment) {
+ this.comment.__editing = this.editing;
+ }
+ this._fireUpdate();
},
_computeLinkToComment: function(comment) {
@@ -174,6 +199,7 @@
} else {
this.$.storage.setDraftComment(commentLocation, message);
}
+ this._fireUpdate();
}, STORAGE_DEBOUNCE_INTERVAL);
},
@@ -218,7 +244,7 @@
_handleCancel: function(e) {
this._preventDefaultAndBlur(e);
if (this.comment.message == null || this.comment.message.length == 0) {
- this.fire('comment-discard');
+ this.fire('comment-discard', {comment: this.comment});
return;
}
this._messageText = this.comment.message;
@@ -234,20 +260,20 @@
this.disabled = true;
if (!this.comment.id) {
this.disabled = false;
- this.fire('comment-discard');
+ this.fire('comment-discard', {comment: this.comment});
return;
}
- this._xhrPromise =
- this._deleteDraft(this.comment).then(function(response) {
- this.disabled = false;
- if (!response.ok) { return response; }
+ this._xhrPromise = this._deleteDraft(this.comment).then(
+ function(response) {
+ this.disabled = false;
+ if (!response.ok) { return response; }
- this.fire('comment-discard');
- }.bind(this)).catch(function(err) {
- this.disabled = false;
- throw err;
- }.bind(this));;
+ this.fire('comment-discard', {comment: this.comment});
+ }.bind(this)).catch(function(err) {
+ this.disabled = false;
+ throw err;
+ }.bind(this));
},
_preventDefaultAndBlur: function(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index 8ef222e..6c27a36 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -204,16 +204,50 @@
});
test('draft saving/editing', function(done) {
+ var fireStub = sinon.stub(element, 'fire');
+
element.draft = true;
MockInteractions.tap(element.$$('.edit'));
element._messageText = 'good news, everyone!';
+ element.flushDebouncer('fire-update');
+ element.flushDebouncer('store');
+ assert(fireStub.calledWith('comment-update'),
+ 'comment-update should be sent');
+ assert.deepEqual(fireStub.lastCall.args, [
+ 'comment-update', {
+ comment: {
+ __draft: true,
+ __draftID: 'temp_draft_id',
+ __editing: true,
+ line: 5,
+ path: '/path/to/file',
+ },
+ },
+ ]);
MockInteractions.tap(element.$$('.save'));
+
assert.isTrue(element.disabled,
'Element should be disabled when creating draft.');
element._xhrPromise.then(function(draft) {
+ assert(fireStub.calledWith('comment-save'),
+ 'comment-save should be sent');
+ assert.deepEqual(fireStub.lastCall.args, [
+ 'comment-save', {
+ comment: {
+ __draft: true,
+ __draftID: 'temp_draft_id',
+ __editing: false,
+ id: 'baf0414d_40572e03',
+ line: 5,
+ message: 'saved!',
+ path: '/path/to/file',
+ updated: '2015-12-08 21:52:36.177000000',
+ },
+ },
+ ]);
assert.isFalse(element.disabled,
- 'Element should be enabled when done creating draft.');
+ 'Element should be enabled when done creating draft.');
assert.equal(draft.message, 'saved!');
assert.isFalse(element.editing);
}).then(function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index 3f71892..3de3ae4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -172,9 +172,9 @@
},
_rowHasSide: function(row) {
- var selector = '.content';
- selector += this.side === DiffSides.LEFT ? '.left' : '.right';
- return row.querySelector(selector);
+ var selector = (this.side === DiffSides.LEFT ? '.left' : '.right') +
+ ' + .content';
+ return !!row.querySelector(selector);
},
_isFirstRowOfChunk: function(row) {
@@ -278,7 +278,7 @@
}
for (i = 0;
- i < splice.removed && splicee.removed.length;
+ i < splice.removed && splice.removed.length;
i++) {
this.unlisten(splice.removed[i], 'render', 'handleDiffUpdate');
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 0e99a15..903eabf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -174,12 +174,16 @@
available-patches="[[_computeAvailablePatches(_change.revisions)]]">
</gr-patch-range-select>
<div>
- <select id="modeSelect" on-change="_handleModeChange">
+ <select
+ id="modeSelect"
+ on-change="_handleModeChange"
+ hidden$="[[_computeModeSelectHidden(_isImageDiff)]]">
<option value="SIDE_BY_SIDE">Side By Side</option>
<option value="UNIFIED_DIFF">Unified</option>
</select>
<span hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]">
- /
+ <span
+ hidden$="[[_computeModeSelectHidden(_isImageDiff)]]">/</span>
<gr-button link
class="prefsButton"
on-tap="_handlePrefsTap">Preferences</gr-button>
@@ -193,6 +197,9 @@
on-cancel="_handlePrefsCancel"></gr-diff-preferences>
</gr-overlay>
<gr-diff id="diff"
+ project="[[_change.project]]"
+ commit="[[_change.current_revision]]"
+ is-image-diff="{{_isImageDiff}}"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
path="[[_path]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index c75cb72..17aa0c4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -74,6 +74,7 @@
type: String,
computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)'
},
+ _isImageDiff: Boolean,
},
behaviors: [
@@ -293,11 +294,11 @@
this._userPrefs = prefs;
}.bind(this)));
- promises.push(this.$.diff.reload());
+ promises.push(this._getChangeDetail(this._changeNum));
- Promise.all(promises).then(function() {
- this._loading = false;
- }.bind(this));
+ Promise.all(promises)
+ .then(function() { return this.$.diff.reload(); }.bind(this))
+ .then(function() { this._loading = false; }.bind(this));
},
_pathChanged: function(path) {
@@ -462,5 +463,9 @@
this.$.modeSelect.value = mode;
}
},
+
+ _computeModeSelectHidden: function() {
+ return this._isImageDiff;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js
new file mode 100644
index 0000000..b897708
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js
@@ -0,0 +1,100 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// 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,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+(function(window, GrDiffBuilderSideBySide) {
+ 'use strict';
+
+ function GrDiffBuilderImage(diff, comments, prefs, outputEl, baseImage,
+ revisionImage) {
+ GrDiffBuilderSideBySide.call(this, diff, comments, prefs, outputEl);
+ this._baseImage = baseImage;
+ this._revisionImage = revisionImage;
+ }
+
+ GrDiffBuilderImage.prototype = Object.create(
+ GrDiffBuilderSideBySide.prototype);
+ GrDiffBuilderImage.prototype.constructor = GrDiffBuilderImage;
+
+ GrDiffBuilderImage.prototype.emitDiff = function() {
+ this.emitGroup(this._groups[0]);
+
+ var section = this._createElement('tbody', 'image-diff');
+
+ this._emitImagePair(section);
+ this._emitImageLabels(section);
+
+ this._outputEl.appendChild(section);
+ };
+
+ GrDiffBuilderImage.prototype._emitImagePair = function(section) {
+ var tr = this._createElement('tr');
+
+ tr.appendChild(this._createElement('td'));
+ tr.appendChild(this._createImageCell(this._baseImage, 'left'));
+
+ tr.appendChild(this._createElement('td'));
+ tr.appendChild(this._createImageCell(this._revisionImage, 'right'));
+
+ section.appendChild(tr);
+ };
+
+ GrDiffBuilderImage.prototype._createImageCell = function(image, className) {
+ var td = this._createElement('td', className);
+ if (image) {
+ var imageEl = this._createElement('img');
+ imageEl.src = 'data:' + image.type + ';base64, ' + image.body;
+ image._height = imageEl.naturalHeight;
+ image._width = imageEl.naturalWidth;
+ imageEl.addEventListener('error', function(e) {
+ imageEl.remove();
+ td.textContent = '[Image failed to load]';
+ });
+ td.appendChild(imageEl);
+ }
+ return td;
+ };
+
+ GrDiffBuilderImage.prototype._emitImageLabels = function(section) {
+ var tr = this._createElement('tr');
+
+ tr.appendChild(this._createElement('td'));
+ var td = this._createElement('td', 'left');
+ var label = this._createElement('label');
+ label.textContent = this._getImageLabel(this._baseImage);
+ td.appendChild(label);
+ tr.appendChild(td);
+
+ tr.appendChild(this._createElement('td'));
+ td = this._createElement('td', 'right');
+ label = this._createElement('label');
+ label.textContent = this._getImageLabel(this._revisionImage);
+ td.appendChild(label);
+ tr.appendChild(td);
+
+ section.appendChild(tr);
+ };
+
+ GrDiffBuilderImage.prototype._getImageLabel = function(image) {
+ if (image) {
+ var type = image.type || image._expectedType;
+ if (image._width && image._height) {
+ return image._width + '⨉' + image._height + ' ' + type;
+ } else {
+ return type;
+ }
+ }
+ return 'No image';
+ };
+
+ window.GrDiffBuilderImage = GrDiffBuilderImage;
+})(window, GrDiffBuilderSideBySide);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js
index bf93112..7e8779f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js
@@ -20,7 +20,7 @@
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderSideBySide.prototype.constructor = GrDiffBuilderSideBySide;
- GrDiffBuilderSideBySide.prototype.emitGroup = function(group,
+ GrDiffBuilderSideBySide.prototype.buildSectionElement = function(group,
opt_beforeSection) {
var sectionEl = this._createElement('tbody', 'section');
sectionEl.classList.add(group.type);
@@ -29,7 +29,7 @@
sectionEl.appendChild(this._createRow(sectionEl, pairs[i].left,
pairs[i].right));
}
- this._outputEl.insertBefore(sectionEl, opt_beforeSection);
+ return sectionEl;
};
GrDiffBuilderSideBySide.prototype._createRow = function(section, leftLine,
@@ -48,13 +48,14 @@
GrDiffBuilderSideBySide.prototype._appendPair = function(section, row, line,
lineNumber, side) {
- row.appendChild(this._createLineEl(line, lineNumber, line.type, side));
+ var lineEl = this._createLineEl(line, lineNumber, line.type, side);
+ lineEl.classList.add(side);
+ row.appendChild(lineEl);
var action = this._createContextControl(section, line);
if (action) {
row.appendChild(action);
} else {
var textEl = this._createTextEl(line);
- textEl.classList.add(side);
var threadEl = this._commentThreadForLine(line, side);
if (threadEl) {
textEl.appendChild(threadEl);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js
index 86340bd..2f1aac6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js
@@ -20,23 +20,26 @@
GrDiffBuilderUnified.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderUnified.prototype.constructor = GrDiffBuilderUnified;
- GrDiffBuilderUnified.prototype.emitGroup = function(group,
- opt_beforeSection) {
+ GrDiffBuilderUnified.prototype.buildSectionElement = function(group) {
var sectionEl = this._createElement('tbody', 'section');
sectionEl.classList.add(group.type);
for (var i = 0; i < group.lines.length; ++i) {
sectionEl.appendChild(this._createRow(sectionEl, group.lines[i]));
}
- this._outputEl.insertBefore(sectionEl, opt_beforeSection);
+ return sectionEl;
};
GrDiffBuilderUnified.prototype._createRow = function(section, line) {
var row = this._createElement('tr', line.type);
- row.appendChild(this._createLineEl(line, line.beforeNumber,
- GrDiffLine.Type.REMOVE));
- row.appendChild(this._createLineEl(line, line.afterNumber,
- GrDiffLine.Type.ADD));
+ var lineEl = this._createLineEl(line, line.beforeNumber,
+ GrDiffLine.Type.REMOVE);
+ lineEl.classList.add('left');
+ row.appendChild(lineEl);
+ lineEl = this._createLineEl(line, line.afterNumber,
+ GrDiffLine.Type.ADD);
+ lineEl.classList.add('right');
+ row.appendChild(lineEl);
row.classList.add('diff-row', 'unified');
var action = this._createContextControl(section, line);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
index c2197eb..d2a7c25 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
@@ -15,6 +15,7 @@
'use strict';
function GrDiffBuilder(diff, comments, prefs, outputEl) {
+ this._diff = diff;
this._comments = comments;
this._prefs = prefs;
this._outputEl = outputEl;
@@ -56,8 +57,51 @@
}
};
+ GrDiffBuilder.prototype.buildSectionElement = function(
+ group, opt_beforeSection) {
+ throw Error('Subclasses must implement buildGroupElement');
+ };
+
GrDiffBuilder.prototype.emitGroup = function(group, opt_beforeSection) {
- throw Error('Subclasses must implement emitGroup');
+ var element = this.buildSectionElement(group);
+ this._outputEl.insertBefore(element, opt_beforeSection);
+ group.element = element;
+ };
+
+ GrDiffBuilder.prototype.renderSection = function(element) {
+ for (var i = 0; i < this._groups.length; i++) {
+ var group = this._groups[i];
+ if (group.element === element) {
+ var newElement = this.buildSectionElement(group);
+ group.element.parentElement.replaceChild(newElement, group.element);
+ group.element = newElement;
+ break;
+ }
+ }
+ };
+
+ GrDiffBuilder.prototype.getSectionsByLineRange = function(
+ startLine, endLine, opt_side) {
+ var sections = [];
+ for (var i = 0; i < this._groups.length; i++) {
+ var group = this._groups[i];
+ if (group.lines.length === 0) {
+ continue;
+ }
+ var groupStartLine;
+ var groupEndLine;
+ if (opt_side === GrDiffBuilder.Side.LEFT) {
+ groupStartLine = group.lines[0].beforeNumber;
+ groupEndLine = group.lines[group.lines.length - 1].beforeNumber;
+ } else if (opt_side === GrDiffBuilder.Side.RIGHT) {
+ groupStartLine = group.lines[0].afterNumber;
+ groupEndLine = group.lines[group.lines.length - 1].afterNumber;
+ }
+ if (startLine <= groupEndLine && endLine >= groupStartLine) {
+ sections.push(group.element);
+ }
+ }
+ return sections;
};
GrDiffBuilder.prototype._processContent = function(content, groups, context) {
@@ -182,6 +226,7 @@
currentChunk.ab.push(chunk[j]);
}
}
+ // != instead of !== because we want to cover both undefined and null.
if (currentChunk.ab != null && currentChunk.ab.length > 0) {
result.push(currentChunk);
}
@@ -260,7 +305,8 @@
}
var ctxLine = new GrDiffLine(GrDiffLine.Type.CONTEXT_CONTROL);
- ctxLine.contextLines = hiddenLines;
+ ctxLine.contextGroup =
+ new GrDiffGroup(GrDiffGroup.Type.BOTH, hiddenLines);
groups.push(new GrDiffGroup(GrDiffGroup.Type.CONTEXT_CONTROL,
[ctxLine]));
@@ -310,13 +356,14 @@
};
GrDiffBuilder.prototype._createContextControl = function(section, line) {
- if (!line.contextLines.length) {
+ if (!line.contextGroup || !line.contextGroup.lines.length) {
return null;
}
+ var contextLines = line.contextGroup.lines;
var td = this._createElement('td');
var button = this._createElement('gr-button', 'showContext');
button.setAttribute('link', true);
- var commonLines = line.contextLines.length;
+ var commonLines = contextLines.length;
var text = 'Show ' + commonLines + ' common line';
if (commonLines > 1) {
text += 's';
@@ -325,7 +372,7 @@
button.textContent = text;
button.addEventListener('tap', function(e) {
e.detail = {
- group: new GrDiffGroup(GrDiffGroup.Type.BOTH, line.contextLines),
+ group: line.contextGroup,
section: section,
};
// Let it bubble up the DOM tree.
@@ -382,7 +429,7 @@
}
var patchNum = this._comments.meta.patchRange.patchNum;
- var side = 'REVISION';
+ var side = comments[0].side || 'REVISION';
if (line.type === GrDiffLine.Type.REMOVE ||
opt_side === GrDiffBuilder.Side.LEFT) {
if (this._comments.meta.patchRange.basePatchNum === 'PARENT') {
@@ -412,7 +459,7 @@
} else if (line.type === GrDiffLine.Type.CONTEXT_CONTROL) {
td.classList.add('contextLineNum');
td.setAttribute('data-value', '@@');
- } else if (line.type === GrDiffLine.Type.BOTH || line.type == type) {
+ } else if (line.type === GrDiffLine.Type.BOTH || line.type === type) {
td.classList.add('lineNum');
td.setAttribute('data-value', number);
}
@@ -475,18 +522,18 @@
// Tags don't count as characters
while (index < html.length &&
- html.charCodeAt(index) == GrDiffBuilder.LESS_THAN_CODE) {
+ html.charCodeAt(index) === GrDiffBuilder.LESS_THAN_CODE) {
while (index < html.length &&
- html.charCodeAt(index) != GrDiffBuilder.GREATER_THAN_CODE) {
+ html.charCodeAt(index) !== GrDiffBuilder.GREATER_THAN_CODE) {
index++;
}
index++; // skip the ">" itself
}
// An HTML entity (e.g., <) counts as one character.
if (index < html.length &&
- html.charCodeAt(index) == GrDiffBuilder.AMPERSAND_CODE) {
+ html.charCodeAt(index) === GrDiffBuilder.AMPERSAND_CODE) {
while (index < html.length &&
- html.charCodeAt(index) != GrDiffBuilder.SEMICOLON_CODE) {
+ html.charCodeAt(index) !== GrDiffBuilder.SEMICOLON_CODE) {
index++;
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html
index 22b9072..4392de0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html
@@ -144,8 +144,9 @@
assert.equal(groups[0].lines[0].afterNumber, GrDiffLine.FILE);
assert.equal(groups[1].type, GrDiffGroup.Type.CONTEXT_CONTROL);
- assert.equal(groups[1].lines[0].contextLines.length, 90);
- groups[1].lines[0].contextLines.forEach(function(l) {
+ assert.instanceOf(groups[1].lines[0].contextGroup, GrDiffGroup);
+ assert.equal(groups[1].lines[0].contextGroup.lines.length, 90);
+ groups[1].lines[0].contextGroup.lines.forEach(function(l) {
assert.equal(l.text, content[0].ab[0]);
});
@@ -179,8 +180,9 @@
});
assert.equal(groups[7].type, GrDiffGroup.Type.CONTEXT_CONTROL);
- assert.equal(groups[7].lines[0].contextLines.length, 90);
- groups[7].lines[0].contextLines.forEach(function(l) {
+ assert.instanceOf(groups[7].lines[0].contextGroup, GrDiffGroup);
+ assert.equal(groups[7].lines[0].contextGroup.lines.length, 90);
+ groups[7].lines[0].contextGroup.lines.forEach(function(l) {
assert.equal(l.text, content[4].ab[0]);
});
@@ -215,8 +217,9 @@
});
assert.equal(groups[3].type, GrDiffGroup.Type.CONTEXT_CONTROL);
- assert.equal(groups[3].lines[0].contextLines.length, 30);
- groups[3].lines[0].contextLines.forEach(function(l) {
+ assert.instanceOf(groups[3].lines[0].contextGroup, GrDiffGroup);
+ assert.equal(groups[3].lines[0].contextGroup.lines.length, 30);
+ groups[3].lines[0].contextGroup.lines.forEach(function(l) {
assert.equal(l.text, content[1].ab[0]);
});
@@ -517,5 +520,54 @@
}
]);
});
+
+ suite('rendering', function() {
+ var content;
+ var outputEl;
+
+ setup(function() {
+ var prefs = {
+ line_length: 10,
+ show_tabs: true,
+ tab_size: 4,
+ context: -1
+ };
+ content = [
+ {ab: []},
+ {a: ['all work and no play make andybons a dull boy']},
+ {ab: []},
+ {b: ['elgoog elgoog elgoog']},
+ {ab: []},
+ ];
+ outputEl = document.createElement('out');
+ builder =
+ new GrDiffBuilder(
+ {content: content}, {left: [], right: []}, prefs, outputEl);
+ builder.buildSectionElement = function(group) {
+ var section = document.createElement('stub');
+ section.textContent = group.lines.reduce(function(acc, line) {
+ return acc + line.text;
+ }, '');
+ return section;
+ };
+ builder.emitDiff();
+ });
+
+ test('renderSection', function() {
+ var section = outputEl.querySelector('stub:nth-of-type(2)');
+ var prevInnerHTML = section.innerHTML;
+ section.innerHTML = 'wiped';
+ builder.renderSection(section);
+ section = outputEl.querySelector('stub:nth-of-type(2)');
+ assert.equal(section.innerHTML, prevInnerHTML);
+ });
+
+ test('getSectionsByLineRange', function() {
+ var section = outputEl.querySelector('stub:nth-of-type(2)');
+ var sections = builder.getSectionsByLineRange(1, 1, 'left');
+ assert.equal(sections.length, 1);
+ assert.strictEqual(sections[0], section);
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js
index 750f7da..7c7c508 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js
@@ -25,6 +25,8 @@
}
}
+ GrDiffGroup.prototype.element = null;
+
GrDiffGroup.Type = {
BOTH: 'both',
CONTEXT_CONTROL: 'contextControl',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
index ea00a3d..4acde0c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
@@ -16,13 +16,14 @@
function GrDiffLine(type) {
this.type = type;
- this.contextLines = [];
this.highlights = [];
}
+ GrDiffLine.prototype.afterNumber = 0;
+
GrDiffLine.prototype.beforeNumber = 0;
- GrDiffLine.prototype.afterNumber = 0;
+ GrDiffLine.prototype.contextGroup = null;
GrDiffLine.prototype.text = '';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index e4603b8..826a21e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -43,6 +43,17 @@
.section {
background-color: #eee;
}
+ .image-diff .gr-diff {
+ text-align: center;
+ }
+ .image-diff img {
+ max-width: 50em;
+ outline: 1px solid #ccc;
+ }
+ .image-diff label {
+ font-family: var(--font-family);
+ font-style: italic;
+ }
.diff-row.target-row.target-side-left .lineNum.left,
.diff-row.target-row.target-side-right .lineNum.right,
.diff-row.target-row.unified .lineNum {
@@ -151,5 +162,6 @@
<script src="gr-diff-builder.js"></script>
<script src="gr-diff-builder-side-by-side.js"></script>
<script src="gr-diff-builder-unified.js"></script>
+ <script src="gr-diff-builder-image.js"></script>
<script src="gr-diff.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 660b86f..d813933 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -42,6 +42,13 @@
type: Object,
observer: '_projectConfigChanged',
},
+ project: String,
+ commit: String,
+ isImageDiff: {
+ type: Boolean,
+ computed: '_computeIsImageDiff(_diff)',
+ notify: true,
+ },
_loggedIn: {
type: Boolean,
@@ -64,15 +71,17 @@
'_prefsChanged(prefs.*, viewMode)',
],
+ listeners: {
+ 'thread-discard': '_handleThreadDiscard',
+ 'comment-discard': '_handleCommentDiscard',
+ 'comment-update': '_handleCommentUpdate',
+ 'comment-save': '_handleCommentSave',
+ },
+
attached: function() {
this._getLoggedIn().then(function(loggedIn) {
this._loggedIn = loggedIn;
}.bind(this));
-
- this.addEventListener('thread-discard',
- this._handleThreadDiscard.bind(this));
- this.addEventListener('comment-discard',
- this._handleCommentDiscard.bind(this));
},
reload: function() {
@@ -82,6 +91,7 @@
promises.push(this._getDiff().then(function(diff) {
this._diff = diff;
+ return this._loadDiffAssets();
}.bind(this)));
promises.push(this._getDiffCommentsAndDrafts().then(function(comments) {
@@ -205,7 +215,7 @@
} else {
var patchNum = this.patchRange.patchNum;
var side = 'REVISION';
- if (contentEl.classList.contains(DiffSide.LEFT) ||
+ if (lineEl.classList.contains(DiffSide.LEFT) ||
contentEl.classList.contains('remove')) {
if (this.patchRange.basePatchNum === 'PARENT') {
side = 'PARENT';
@@ -226,29 +236,71 @@
},
_handleCommentDiscard: function(e) {
- var comment = Polymer.dom(e).rootTarget.comment;
- this._removeComment(comment);
+ var comment = e.detail.comment;
+ this._removeComment(comment, e.target.patchNum);
},
- _removeComment: function(comment) {
- if (!comment.id) { return; }
- this._removeCommentFromSide(comment, DiffSide.LEFT) ||
- this._removeCommentFromSide(comment, DiffSide.RIGHT);
+ _removeComment: function(comment, opt_patchNum) {
+ var side = this._findCommentSide(comment, opt_patchNum);
+ this._removeCommentFromSide(comment, side);
+ },
+
+ _findCommentSide: function(comment, opt_patchNum) {
+ if (comment.side === 'PARENT') {
+ return DiffSide.LEFT;
+ } else {
+ return this._comments.meta.patchRange.basePatchNum === opt_patchNum ?
+ DiffSide.LEFT : DiffSide.RIGHT;
+ }
+ },
+
+ _handleCommentSave: function(e) {
+ var comment = e.detail.comment;
+ var side = this._findCommentSide(comment, e.target.patchNum);
+ var idx = this._findDraftIndex(comment, side);
+ this.set(['_comments', side, idx], comment);
+ },
+
+ _handleCommentUpdate: function(e) {
+ var comment = e.detail.comment;
+ var side = this._findCommentSide(comment, e.target.patchNum);
+ var idx = this._findCommentIndex(comment, side);
+ if (idx === -1) {
+ idx = this._findDraftIndex(comment, side);
+ }
+ if (idx !== -1) { // Update draft or comment.
+ this.set(['_comments', side, idx], comment);
+ } else { // Create new draft.
+ this.push(['_comments', side], comment);
+ }
},
_removeCommentFromSide: function(comment, side) {
- var idx = -1;
- for (var i = 0; i < this._comments[side].length; i++) {
- if (this._comments[side][i].id === comment.id) {
- idx = i;
- break;
- }
+ var idx = this._findCommentIndex(comment, side);
+ if (idx === -1) {
+ idx = this._findDraftIndex(comment, side);
}
if (idx !== -1) {
this.splice('_comments.' + side, idx, 1);
- return true;
}
- return false;
+ },
+
+ _findCommentIndex: function(comment, side) {
+ if (!comment.id || !this._comments[side]) {
+ return -1;
+ }
+ return this._comments[side].findIndex(function(item) {
+ return item.id === comment.id;
+ });
+ },
+
+ _findDraftIndex: function(comment, side) {
+ if (!comment.__draftID || !this._comments[side]) {
+ return -1;
+ }
+ return this._comments[side].findIndex(function(item) {
+ return item.__draftID === comment.__draftID;
+ });
},
_handleMouseDown: function(e) {
@@ -312,6 +364,13 @@
},
_showContext: function(group, sectionEl) {
+ var groups = this._builder._groups;
+ // TODO(viktard): Polyfill findIndex for IE10.
+ var contextIndex = groups.findIndex(function(group) {
+ return group.element == sectionEl;
+ });
+ groups[contextIndex] = group;
+
this._builder.emitGroup(group, sectionEl);
sectionEl.parentNode.removeChild(sectionEl);
@@ -331,14 +390,17 @@
},
_render: function() {
- this._clearDiffContent();
- this._builder = this._getDiffBuilder(this._diff, this._comments,
- this.prefs);
- this._builder.emitDiff(this._diff.content);
+ this._builder =
+ this._getDiffBuilder(this._diff, this._comments, this.prefs);
+ this._renderDiff();
+ },
+ _renderDiff: function() {
+ this._clearDiffContent();
+ this._builder.emitDiff();
this.async(function() {
this.fire('render', null, {bubbles: false});
- }.bind(this), 1);
+ }, 1);
},
_clearDiffContent: function() {
@@ -414,8 +476,40 @@
return this.$.restAPI.getLoggedIn();
},
+ _computeIsImageDiff: function() {
+ if (!this._diff) { return false; }
+
+ var isA = this._diff.meta_a &&
+ this._diff.meta_a.content_type.indexOf('image/') === 0;
+ var isB = this._diff.meta_b &&
+ this._diff.meta_b.content_type.indexOf('image/') === 0;
+
+ return this._diff.binary && (isA || isB);
+ },
+
+ _loadDiffAssets: function() {
+ if (this.isImageDiff) {
+ return this._getImages().then(function(images) {
+ this._baseImage = images.baseImage;
+ this._revisionImage = images.revisionImage;
+ }.bind(this));
+ } else {
+ this._baseImage = null;
+ this._revisionImage = null;
+ return Promise.resolve();
+ }
+ },
+
+ _getImages: function() {
+ return this.$.restAPI.getImagesForDiff(this.project, this.commit,
+ this.changeNum, this._diff, this.patchRange);
+ },
+
_getDiffBuilder: function(diff, comments, prefs) {
- if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ if (this.isImageDiff) {
+ return new GrDiffBuilderImage(diff, comments, prefs, this.$.diffTable,
+ this._baseImage, this._revisionImage);
+ } else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
return new GrDiffBuilderSideBySide(diff, comments, prefs,
this.$.diffTable);
} else if (this.viewMode === DiffViewMode.UNIFIED) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index f02b861..aec32b6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -20,6 +20,7 @@
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../../scripts/util.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-diff.html">
@@ -34,85 +35,29 @@
suite('gr-diff tests', function() {
var element;
- setup(function() {
- stub('gr-rest-api-interface', {
- getLoggedIn: function() { return Promise.resolve(false); },
+ suite('not logged in', function() {
+
+ setup(function() {
+ stub('gr-rest-api-interface', {
+ getLoggedIn: function() { return Promise.resolve(false); },
+ });
+ element = fixture('basic');
});
- element = fixture('basic');
- });
- test('get drafts logged out', function(done) {
- element.patchRange = {basePatchNum: 0, patchNum: 0};
+ test('get drafts', function(done) {
+ element.patchRange = {basePatchNum: 0, patchNum: 0};
- var getDraftsStub = sinon.stub(element.$.restAPI, 'getDiffDrafts');
- var loggedInStub = sinon.stub(element, '_getLoggedIn',
- function() { return Promise.resolve(false); });
- element._getDiffDrafts().then(function(result) {
- assert.deepEqual(result, {baseComments: [], comments: []});
- sinon.assert.notCalled(getDraftsStub);
- loggedInStub.restore();
- getDraftsStub.restore();
- done();
+ var getDraftsStub = sinon.stub(element.$.restAPI, 'getDiffDrafts');
+ element._getDiffDrafts().then(function(result) {
+ assert.deepEqual(result, {baseComments: [], comments: []});
+ sinon.assert.notCalled(getDraftsStub);
+ getDraftsStub.restore();
+ done();
+ });
});
- });
- test('get drafts logged in', function(done) {
- element.patchRange = {basePatchNum: 0, patchNum: 0};
- var draftsResponse = {
- baseComments: [{id: 'foo'}],
- comments: [{id: 'bar'}],
- };
- var getDraftsStub = sinon.stub(element.$.restAPI, 'getDiffDrafts',
- function() { return Promise.resolve(draftsResponse); });
- var loggedInStub = sinon.stub(element, '_getLoggedIn',
- function() { return Promise.resolve(true); });
- element._getDiffDrafts().then(function(result) {
- assert.deepEqual(result, draftsResponse);
- loggedInStub.restore();
- getDraftsStub.restore();
- done();
- });
- });
-
- test('get comments and drafts', function(done) {
- var loggedInStub = sinon.stub(element, '_getLoggedIn',
- function() { return Promise.resolve(true); });
- var comments = {
- baseComments: [
- {id: 'bc1'},
- {id: 'bc2'},
- ],
- comments: [
- {id: 'c1'},
- {id: 'c2'},
- ],
- };
- var diffCommentsStub = sinon.stub(element, '_getDiffComments',
- function() { return Promise.resolve(comments); });
-
- var drafts = {
- baseComments: [
- {id: 'bd1'},
- {id: 'bd2'},
- ],
- comments: [
- {id: 'd1'},
- {id: 'd2'},
- ],
- };
- var diffDraftsStub = sinon.stub(element, '_getDiffDrafts',
- function() { return Promise.resolve(drafts); });
-
- element.changeNum = '42';
- element.patchRange = {
- basePatchNum: 'PARENT',
- patchNum: 3,
- };
- element.path = '/path/to/foo';
- element.projectConfig = {foo: 'bar'};
-
- element._getDiffCommentsAndDrafts().then(function(result) {
- assert.deepEqual(result, {
+ test('remove comment', function() {
+ element._comments = {
meta: {
changeNum: '42',
patchRange: {
@@ -123,10 +68,10 @@
projectConfig: {foo: 'bar'},
},
left: [
- {id: 'bc1'},
- {id: 'bc2'},
- {id: 'bd1', __draft: true},
- {id: 'bd2', __draft: true},
+ {id: 'bc1', side: 'PARENT'},
+ {id: 'bc2', side: 'PARENT'},
+ {id: 'bd1', __draft: true, side: 'PARENT'},
+ {id: 'bd2', __draft: true, side: 'PARENT'},
],
right: [
{id: 'c1'},
@@ -134,113 +79,348 @@
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
],
- });
+ };
- diffCommentsStub.restore();
- diffDraftsStub.restore();
- loggedInStub.restore();
- done();
+ element._removeComment({});
+ // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem
+ // to believe that one object deepEquals another even when they do :-/.
+ assert.equal(JSON.stringify(element._comments), JSON.stringify({
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT'},
+ {id: 'bc2', side: 'PARENT'},
+ {id: 'bd1', __draft: true, side: 'PARENT'},
+ {id: 'bd2', __draft: true, side: 'PARENT'},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ }));
+
+ element._removeComment({id: 'bc2', side: 'PARENT'});
+ assert.equal(JSON.stringify(element._comments), JSON.stringify({
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT'},
+ {id: 'bd1', __draft: true, side: 'PARENT'},
+ {id: 'bd2', __draft: true, side: 'PARENT'},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ }));
+
+ element._removeComment({id: 'd2'});
+ assert.deepEqual(JSON.stringify(element._comments), JSON.stringify({
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT'},
+ {id: 'bd1', __draft: true, side: 'PARENT'},
+ {id: 'bd2', __draft: true, side: 'PARENT'},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ ],
+ }));
+ });
+
+ test('renders image diffs', function(done) {
+ var mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ var mockFile1 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAEwsA' +
+ 'AAAAAAAAAAAAAAAA/w==',
+ type: 'image/bmp',
+ };
+ var mockFile2 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAEwsA' +
+ 'AAAAAAAAAAAA/////w==',
+ type: 'image/bmp'
+ };
+ var mockCommit = {
+ commit: '9a1a1d10baece5efbba10bc4ccf808a67a50ac0a',
+ parents: [{
+ commit: '7338aa9adfe57909f1fdaf88975cdea467d3382f',
+ subject: 'Added a carrot',
+ }],
+ author: {
+ name: 'Wyatt Allen',
+ email: 'wyatta@google.com',
+ date: '2016-05-23 21:44:51.000000000',
+ tz: -420,
+ },
+ committer: {
+ name: 'Wyatt Allen',
+ email: 'wyatta@google.com',
+ date: '2016-05-25 00:25:41.000000000',
+ tz: -420,
+ },
+ subject: 'Updated the carrot',
+ message: 'Updated the carrot\n\nChange-Id: Iabcd123\n',
+ };
+ var mockComments = {baseComments: [], comments: []};
+
+ var stubs = [];
+ stubs.push(sinon.stub(element, '_getDiff',
+ function() { return Promise.resolve(mockDiff); }));
+ stubs.push(sinon.stub(element.$.restAPI, 'getCommitInfo',
+ function() { return Promise.resolve(mockCommit); }));
+ stubs.push(sinon.stub(element.$.restAPI,
+ 'getCommitFileContents',
+ function() { return Promise.resolve(mockFile1); }));
+ stubs.push(sinon.stub(element.$.restAPI,
+ 'getChangeFileContents',
+ function() { return Promise.resolve(mockFile2); }));
+ stubs.push(sinon.stub(element.$.restAPI, '_getDiffComments',
+ function() { return Promise.resolve(mockComments); }));
+ stubs.push(sinon.stub(element.$.restAPI, 'getDiffDrafts',
+ function() { return Promise.resolve(mockComments); }));
+
+ element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
+
+ var rendered = function() {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(element._getDiffBuilder(element._diff,
+ element._comments, element.prefs), GrDiffBuilderImage);
+
+ // Left image rendered with the parent commit's version of the file.
+ var leftInmage = element.$.diffTable.querySelector('td.left img');
+ assert.isOk(leftInmage);
+ assert.equal(leftInmage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile1.body);
+
+ // Right image rendered with this change's revision of the image.
+ var rightInmage = element.$.diffTable.querySelector('td.right img');
+ assert.isOk(rightInmage);
+ assert.equal(rightInmage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile2.body);
+
+ // Cleanup.
+ element.removeEventListener('render', rendered);
+ stubs.forEach(function(stub) { stub.restore(); });
+
+ done();
+ };
+
+ element.addEventListener('render', rendered);
+
+ element.$.restAPI.getDiffPreferences().then(function(prefs) {
+ element.prefs = prefs;
+ element.reload();
+ });
});
});
- test('remove comment', function() {
- element._comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1'},
- {id: 'bc2'},
- {id: 'bd1', __draft: true},
- {id: 'bd2', __draft: true},
- ],
- right: [
- {id: 'c1'},
- {id: 'c2'},
- {id: 'd1', __draft: true},
- {id: 'd2', __draft: true},
- ],
- };
+ suite('logged in', function() {
- element._removeComment({});
- // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem to
- // believe that one object deepEquals another even when they do :-/.
- assert.equal(JSON.stringify(element._comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1'},
- {id: 'bc2'},
- {id: 'bd1', __draft: true},
- {id: 'bd2', __draft: true},
- ],
- right: [
- {id: 'c1'},
- {id: 'c2'},
- {id: 'd1', __draft: true},
- {id: 'd2', __draft: true},
- ],
- }));
+ setup(function() {
+ stub('gr-rest-api-interface', {
+ getLoggedIn: function() { return Promise.resolve(true); },
+ });
+ element = fixture('basic');
+ });
- element._removeComment({id: 'bc2'});
- assert.equal(JSON.stringify(element._comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1'},
- {id: 'bd1', __draft: true},
- {id: 'bd2', __draft: true},
- ],
- right: [
- {id: 'c1'},
- {id: 'c2'},
- {id: 'd1', __draft: true},
- {id: 'd2', __draft: true},
- ],
- }));
+ test('get drafts', function(done) {
+ element.patchRange = {basePatchNum: 0, patchNum: 0};
+ var draftsResponse = {
+ baseComments: [{id: 'foo'}],
+ comments: [{id: 'bar'}],
+ };
+ var getDraftsStub = sinon.stub(element.$.restAPI, 'getDiffDrafts',
+ function() { return Promise.resolve(draftsResponse); });
+ element._getDiffDrafts().then(function(result) {
+ assert.deepEqual(result, draftsResponse);
+ getDraftsStub.restore();
+ done();
+ });
+ });
- element._removeComment({id: 'd2'});
- assert.deepEqual(JSON.stringify(element._comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1'},
- {id: 'bd1', __draft: true},
- {id: 'bd2', __draft: true},
- ],
- right: [
- {id: 'c1'},
- {id: 'c2'},
- {id: 'd1', __draft: true},
- ],
- }));
+ test('get comments and drafts', function(done) {
+ var comments = {
+ baseComments: [
+ {id: 'bc1'},
+ {id: 'bc2'},
+ ],
+ comments: [
+ {id: 'c1'},
+ {id: 'c2'},
+ ],
+ };
+ var diffCommentsStub = sinon.stub(element, '_getDiffComments',
+ function() { return Promise.resolve(comments); });
+
+ var drafts = {
+ baseComments: [
+ {id: 'bd1'},
+ {id: 'bd2'},
+ ],
+ comments: [
+ {id: 'd1'},
+ {id: 'd2'},
+ ],
+ };
+ var diffDraftsStub = sinon.stub(element, '_getDiffDrafts',
+ function() { return Promise.resolve(drafts); });
+
+ element.changeNum = '42';
+ element.patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ };
+ element.path = '/path/to/foo';
+ element.projectConfig = {foo: 'bar'};
+
+ element._getDiffCommentsAndDrafts().then(function(result) {
+ assert.deepEqual(result, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1'},
+ {id: 'bc2'},
+ {id: 'bd1', __draft: true},
+ {id: 'bd2', __draft: true},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ });
+
+ diffCommentsStub.restore();
+ diffDraftsStub.restore();
+ done();
+ });
+ });
+
+ suite('handle comment-update', function() {
+
+ setup(function() {
+ element._comments = {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT'},
+ {id: 'bc2', side: 'PARENT'},
+ {id: 'bd1', __draft: true, side: 'PARENT'},
+ {id: 'bd2', __draft: true, side: 'PARENT'},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ };
+ });
+
+ test('creating a draft', function() {
+ var comment = {__draft: true, __draftID: 'tempID', side: 'PARENT'};
+ element.fire('comment-update', {comment: comment});
+ assert.include(element._comments.left, comment);
+ });
+
+ test('saving a draft', function() {
+ var draftID = 'tempID';
+ var id = 'savedID';
+ element._comments.left.push(
+ {__draft: true, __draftID: draftID, side: 'PARENT'});
+ element.fire('comment-update', {comment:
+ {id: id, __draft: true, __draftID: draftID, side: 'PARENT'},
+ });
+ var drafts = element._comments.left.filter(function(item) {
+ return item.__draftID === draftID;
+ });
+ assert.equal(drafts.length, 1);
+ assert.equal(drafts[0].id, id);
+ });
+ });
+ });
+
+ suite('renderDiff', function() {
+ setup(function(done) {
+ sinon.stub(element, 'fire');
+ element._builder = {
+ emitDiff: sinon.stub(),
+ };
+ element._renderDiff();
+ flush(function() {
+ done();
+ });
+ });
+
+ teardown(function() {
+ element.fire.restore();
+ });
+
+ test('fires render', function() {
+ assert(element.fire.calledWithExactly(
+ 'render', null, {bubbles: false}));
+ });
+ test('calls emitDiff on builder', function() {
+ assert(element._builder.emitDiff.calledOnce);
+ });
});
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
index 10c8a29..72ae4e4 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
@@ -18,7 +18,6 @@
<script src="../../../bower_components/fetch/fetch.js"></script>
<dom-module id="gr-rest-api-interface">
- <template></template>
<script src="gr-rest-api-interface.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index e681bf9..9604ded 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -682,5 +682,84 @@
return '';
},
+ getCommitInfo: function(project, commit) {
+ return this.fetchJSON(
+ '/projects/' + encodeURIComponent(project) +
+ '/commits/' + encodeURIComponent(commit));
+ },
+
+ _fetchB64File: function(url) {
+ return fetch(url).then(function(response) {
+ var type = response.headers.get('X-FYI-Content-Type');
+ return response.text()
+ .then(function(text) {
+ return {body: text, type: type};
+ });
+ });
+ },
+
+ getChangeFileContents: function(changeId, patchNum, path) {
+ return this._fetchB64File(
+ '/changes/' + encodeURIComponent(changeId) +
+ '/revisions/' + encodeURIComponent(patchNum) +
+ '/files/' + encodeURIComponent(path) +
+ '/content');
+ },
+
+ getCommitFileContents: function(projectName, commit, path) {
+ return this._fetchB64File(
+ '/projects/' + encodeURIComponent(projectName) +
+ '/commits/' + encodeURIComponent(commit) +
+ '/files/' + encodeURIComponent(path) +
+ '/content');
+ },
+
+ getImagesForDiff: function(project, commit, changeNum, diff, patchRange) {
+ var promiseA;
+ var promiseB;
+
+ if (diff.meta_a && diff.meta_a.content_type.indexOf('image/') === 0) {
+ if (patchRange.basePatchNum === 'PARENT') {
+ // Need the commit info know the parent SHA.
+ promiseA = this.getCommitInfo(project, commit).then(function(info) {
+ if (info.parents.length !== 1) {
+ return Promise.reject('Change commit has multiple parents.');
+ }
+ var parent = info.parents[0].commit;
+ return this.getCommitFileContents(project, parent,
+ diff.meta_a.name);
+ }.bind(this));
+
+ } else {
+ promiseA = this.getChangeFileContents(changeNum,
+ patchRange.basePatchNum, diff.meta_a.name);
+ }
+ } else {
+ promiseA = Promise.resolve(null);
+ }
+
+ if (diff.meta_b && diff.meta_b.content_type.indexOf('image/') === 0) {
+ promiseB = this.getChangeFileContents(changeNum, patchRange.patchNum,
+ diff.meta_b.name);
+ } else {
+ promiseB = Promise.resolve(null);
+ }
+
+ return Promise.all([promiseA, promiseB])
+ .then(function(results) {
+ var baseImage = results[0];
+ var revisionImage = results[1];
+
+ // Sometimes the server doesn't send back the content type.
+ if (baseImage) {
+ baseImage._expectedType = diff.meta_a.content_type;
+ }
+ if (revisionImage) {
+ revisionImage._expectedType = diff.meta_b.content_type;
+ }
+
+ return {baseImage: baseImage, revisionImage: revisionImage};
+ }.bind(this));
+ },
});
})();
