[gold] Always pass changeListID and crs around in frontend
This renames all instances of "issue" to the more descriptive
"changeListID".
Change-Id: Ie18027b8536521a5b30b2ba8228824197a2a005c
Bug: skia:10532
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/306333
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/cmd/skiacorrectness/main.go b/golden/cmd/skiacorrectness/main.go
index 0b67f01..6932c70 100644
--- a/golden/cmd/skiacorrectness/main.go
+++ b/golden/cmd/skiacorrectness/main.go
@@ -624,7 +624,6 @@
URLTemplate: cfg.URLTemplate,
})
}
- sklog.Infof("Review systems %#v", rs)
return rs, nil
}
diff --git a/golden/frontend/index.html b/golden/frontend/index.html
index 7b2c5c2..cbfa02f 100644
--- a/golden/frontend/index.html
+++ b/golden/frontend/index.html
@@ -19,7 +19,6 @@
title: "{{.Title}}",
defaultCorpus: "{{.DefaultCorpus}}",
baseRepoURL: "{{.BaseRepoURL}}",
- crsTemplate: "{{.CodeReviewTemplate}}",
};
</script>
<link href="/dist/transitional-bundle.css" rel="stylesheet">
diff --git a/golden/go/search/search.go b/golden/go/search/search.go
index 66c57a3..54269a6 100644
--- a/golden/go/search/search.go
+++ b/golden/go/search/search.go
@@ -123,12 +123,11 @@
isChangeListSearch := q.ChangeListID != "" && q.ChangeListID != "0"
// Get the expectations and the current index, which we assume constant
// for the duration of this query.
- crs := q.CodeReviewSystemID
- if isChangeListSearch && crs == "" {
+ if isChangeListSearch && q.CodeReviewSystemID == "" {
// TODO(kjlubick) remove this default after the search page is converted to lit-html.
- crs = s.reviewSystems[0].ID
+ q.CodeReviewSystemID = s.reviewSystems[0].ID
}
- exp, err := s.getExpectations(ctx, q.ChangeListID, crs)
+ exp, err := s.getExpectations(ctx, q.ChangeListID, q.CodeReviewSystemID)
if err != nil {
return nil, skerr.Wrap(err)
}
@@ -138,7 +137,7 @@
var results []*frontend.SearchResult
// Find the digests (left hand side) we are interested in.
if isChangeListSearch {
- reviewSystem, err := s.reviewSystem(crs)
+ reviewSystem, err := s.reviewSystem(q.CodeReviewSystemID)
if err != nil {
return nil, skerr.Wrap(err)
}
@@ -191,7 +190,7 @@
// Sort the digests and fill the ones that are going to be displayed with
// additional data.
displayRet, offset := s.sortAndLimitDigests(ctx, q, results, int(q.Offset), int(q.Limit))
- s.addTriageHistory(ctx, s.makeTriageHistoryGetter(crs, q.ChangeListID), displayRet)
+ s.addTriageHistory(ctx, s.makeTriageHistoryGetter(q.CodeReviewSystemID, q.ChangeListID), displayRet)
traceComments := s.getTraceComments(ctx)
prepareTraceGroups(displayRet, exp, traceComments, isChangeListSearch)
diff --git a/golden/go/web/frontend/types.go b/golden/go/web/frontend/types.go
index 19b996c..91e528d 100644
--- a/golden/go/web/frontend/types.go
+++ b/golden/go/web/frontend/types.go
@@ -96,8 +96,7 @@
TestDigestStatus map[types.TestName]map[types.Digest]expectations.Label `json:"testDigestStatus"`
// ChangeListID is the id of the ChangeList for which we want to change the expectations.
- // "issue" is the JSON field for backwards compatibility.
- ChangeListID string `json:"issue"`
+ ChangeListID string `json:"changelist_id"`
// CodeReviewSystem is the id of the crs that the ChangeListID belongs. If ChangeListID is set,
// CodeReviewSystem should be also.
diff --git a/golden/go/web/web.go b/golden/go/web/web.go
index 6fdced7..a91fc3c 100644
--- a/golden/go/web/web.go
+++ b/golden/go/web/web.go
@@ -617,7 +617,7 @@
http.Error(w, "Some query parameters are wrong or missing", http.StatusBadRequest)
return
}
- clID := r.Form.Get("issue")
+ clID := r.Form.Get("changelist_id")
crs := r.Form.Get("crs")
if clID != "" {
if _, ok := wh.getCodeReviewSystem(crs); !ok {
@@ -657,7 +657,7 @@
http.Error(w, "invalid query params", http.StatusBadRequest)
return
}
- clID := r.Form.Get("issue")
+ clID := r.Form.Get("changelist_id")
crs := r.Form.Get("crs")
if clID != "" {
if _, ok := wh.getCodeReviewSystem(crs); !ok {
@@ -1170,7 +1170,7 @@
return
}
- clID := q.Get("issue")
+ clID := q.Get("changelist_id")
crs := q.Get("crs")
if clID != "" {
if _, ok := wh.getCodeReviewSystem(crs); !ok {
diff --git a/golden/modules/bulk-triage-sk/bulk-triage-sk-demo.js b/golden/modules/bulk-triage-sk/bulk-triage-sk-demo.js
index cc40175..b2efa8c 100644
--- a/golden/modules/bulk-triage-sk/bulk-triage-sk-demo.js
+++ b/golden/modules/bulk-triage-sk/bulk-triage-sk-demo.js
@@ -21,6 +21,7 @@
const eleCL = document.createElement('bulk-triage-sk');
eleCL.setDigests(examplePageData, exampleAllData);
eleCL.changeListID = '1234567';
+eleCL.crs = 'github';
eleCL.addEventListener('bulk_triage_invoked', handleTriaged);
ele.addEventListener('bulk_triage_cancelled', handleCancelled);
$$('#changelist').appendChild(eleCL);
diff --git a/golden/modules/bulk-triage-sk/bulk-triage-sk.js b/golden/modules/bulk-triage-sk/bulk-triage-sk.js
index 8e6a18c..96cb562 100644
--- a/golden/modules/bulk-triage-sk/bulk-triage-sk.js
+++ b/golden/modules/bulk-triage-sk/bulk-triage-sk.js
@@ -78,6 +78,7 @@
constructor() {
super(template);
this._changeListID = '';
+ this._crs = '';
this._value = CLOSEST;
this._triageAll = false;
@@ -117,6 +118,16 @@
}
/**
+ * @prop crs {string} The Code Review System (e.g. "gerrit") if changeListID is set.
+ */
+ get crs() { return this._crs; }
+
+ set crs(c) {
+ this._crs = c;
+ this._render();
+ }
+
+ /**
* Sets the data that would be used to make bulk triage requests. These the same shape as
* to frontend.TriageRequest.TestDigestStatus. Note that the labels should be set to
* the closest matching reference digest, so we can apply the "closest" bulk triage.
@@ -171,7 +182,8 @@
method: 'POST',
body: JSON.stringify({
testDigestStatus: triageStatuses,
- issue: this.changeListID,
+ changelist_id: this.changeListID,
+ crs: this.crs,
}),
}).then(() => {
// Even if we get back a non-200 code, we want to say we finished.
diff --git a/golden/modules/bulk-triage-sk/bulk-triage-sk_test.js b/golden/modules/bulk-triage-sk/bulk-triage-sk_test.js
index f9a6f9b..0f98e1e 100644
--- a/golden/modules/bulk-triage-sk/bulk-triage-sk_test.js
+++ b/golden/modules/bulk-triage-sk/bulk-triage-sk_test.js
@@ -55,6 +55,7 @@
it('POSTs for all results', async () => {
bulkTriageSk.changeListID = 'someCL';
+ bulkTriageSk.crs = 'gerrit';
$$('checkbox-sk.toggle_all', bulkTriageSk).click();
const finishedPromise = eventPromise('bulk_triage_finished');
fetchMock.post('/json/triage', (url, req) => {
diff --git a/golden/modules/bulk-triage-sk/test_data.js b/golden/modules/bulk-triage-sk/test_data.js
index 1433d0c..b920378 100644
--- a/golden/modules/bulk-triage-sk/test_data.js
+++ b/golden/modules/bulk-triage-sk/test_data.js
@@ -9,8 +9,7 @@
},
};
-export const expectedPageData = '{"testDigestStatus":{"alpha_test":{"aaaaaaaaaaaaaaaaaaaaaaaaaaa":"positive","bbbbbbbbbbbbbbbbbbbbbbbbbbb":"negative"},"beta_test":{"ccccccccccccccccccccccccccc":"positive"}},"issue":""}';
-
+export const expectedPageData = '{"testDigestStatus":{"alpha_test":{"aaaaaaaaaaaaaaaaaaaaaaaaaaa":"positive","bbbbbbbbbbbbbbbbbbbbbbbbbbb":"negative"},"beta_test":{"ccccccccccccccccccccccccccc":"positive"}},"changelist_id":"","crs":""}';
export const exampleAllData = {
alpha_test: {
@@ -29,4 +28,4 @@
},
};
-export const expectedAllData = '{"testDigestStatus":{"alpha_test":{"aaaaaaaaaaaaaaaaaaaaaaaaaaa":"positive","bbbbbbbbbbbbbbbbbbbbbbbbbbb":"negative","ddddddddddddddddddddddddddd":"positive"},"beta_test":{"ccccccccccccccccccccccccccc":"positive","ddddddddddddddddddddddddddd":"negative"},"gamma_test":{"eeeeeeeeeeeeeeeeeeeeeeeeeee":""}},"issue":"someCL"}';
+export const expectedAllData = '{"testDigestStatus":{"alpha_test":{"aaaaaaaaaaaaaaaaaaaaaaaaaaa":"positive","bbbbbbbbbbbbbbbbbbbbbbbbbbb":"negative","ddddddddddddddddddddddddddd":"positive"},"beta_test":{"ccccccccccccccccccccccccccc":"positive","ddddddddddddddddddddddddddd":"negative"},"gamma_test":{"eeeeeeeeeeeeeeeeeeeeeeeeeee":""}},"changelist_id":"someCL","crs":"gerrit"}';
diff --git a/golden/modules/changelist-controls-sk/changelist-controls-sk.js b/golden/modules/changelist-controls-sk/changelist-controls-sk.js
index 49f5e4f..ee3baf3 100644
--- a/golden/modules/changelist-controls-sk/changelist-controls-sk.js
+++ b/golden/modules/changelist-controls-sk/changelist-controls-sk.js
@@ -26,14 +26,14 @@
const ps = ele._selectedPS();
return html`
<div class=info>
- <span class=title>${cl.system} issue:</span>
+ <span class=title>${cl.system} changelist:</span>
<a href=${cl.url} target=_blank rel=noopener>
${limitString(cl.subject, 48)}
</a>
<span>${limitString(cl.owner, 32)}</span>
- <a href="/triagelog?issue=${cl.id}&system=${cl.system}">
+ <a href="/triagelog?changelist_id=${cl.id}&crs=${cl.system}">
<find-in-page-icon-sk></find-in-page-icon-sk>Triagelog
</a>
</div>
diff --git a/golden/modules/cluster-page-sk/cluster-page-sk-demo.js b/golden/modules/cluster-page-sk/cluster-page-sk-demo.js
index e969595..5711569 100644
--- a/golden/modules/cluster-page-sk/cluster-page-sk-demo.js
+++ b/golden/modules/cluster-page-sk/cluster-page-sk-demo.js
@@ -20,7 +20,7 @@
// Load the demo page with some params to load if there aren't any already. 4 is an arbitrary
// small number to check against to determine "no query params"
if (window.location.search.length < 4) {
- const query = '?grouping=dots-legend-sk_too-many-digests&issue=12353';
+ const query = '?grouping=dots-legend-sk_too-many-digests&changelist_id=12353&crs=gerrit';
history.pushState(null, '', window.location.origin + window.location.pathname + query);
}
diff --git a/golden/modules/cluster-page-sk/cluster-page-sk.js b/golden/modules/cluster-page-sk/cluster-page-sk.js
index 0995bd4..91fc887 100644
--- a/golden/modules/cluster-page-sk/cluster-page-sk.js
+++ b/golden/modules/cluster-page-sk/cluster-page-sk.js
@@ -119,12 +119,14 @@
};
this._grouping = '';
this._changeListID = '';
+ this._crs = '';
this._stateChanged = stateReflector(
/* getState */() => {
const state = SearchCriteriaToHintableObject(this._searchCriteria);
state.grouping = this._grouping;
state.changeListID = this._changeListID;
+ state.crs = this._crs;
return state;
},
/* setState */(newState) => {
@@ -134,6 +136,7 @@
this._searchCriteria = SearchCriteriaFromHintableObject(newState);
this._grouping = newState.grouping;
this._changeListID = newState.changeListID;
+ this._crs = newState.crs;
this._fetchClusterData();
this._render();
},
@@ -237,7 +240,8 @@
digest: [digest],
};
if (this._changeListID) {
- urlObj.issue = [this._changeListID];
+ urlObj.changelist_id = [this._changeListID];
+ urlObj.crs = [this._crs];
}
const url = `/json/details?${fromObject(urlObj)}`;
@@ -262,7 +266,8 @@
right: [rightDigest],
};
if (this._changeListID) {
- urlObj.issue = [this._changeListID];
+ urlObj.changelist_id = [this._changeListID];
+ urlObj.crs = [this._crs];
}
const url = `/json/diff?${fromObject(urlObj)}`;
diff --git a/golden/modules/common.js b/golden/modules/common.js
index ea7df37..0e8e145 100644
--- a/golden/modules/common.js
+++ b/golden/modules/common.js
@@ -75,13 +75,14 @@
* Returns a link to the details page for a given test-digest pair.
* @param test {string}
* @param digest {string}
- * @param issue {string} Optional, omit or use empty string for master branch.
+ * @param clID {string} Optional, omit or use empty string for master branch.
+ * @param crs {string} Optional, omit or use empty string for master branch.
* @return {string}
*/
-export function detailHref(test, digest, issue = '') {
+export function detailHref(test, digest, clID = '', crs = '') {
const u = `/detail?test=${test}&digest=${digest}`;
- if (issue) {
- return `${u}&issue=${issue}`;
+ if (clID) {
+ return `${u}&changelist_id=${clID}&crs=${crs}`;
}
return u;
}
@@ -91,17 +92,18 @@
* @param grouping {string}
* @param left {string}
* @param right {string}
- * @param issue {string} Optional, omit or use empty string for master branch.
+ * @param clID {string} Optional, omit or use empty string for master branch.
+ * @param crs {string} Optional, omit or use empty string for master branch.
* @return {string}
*/
-export function diffPageHref(grouping, left, right, issue = '') {
+export function diffPageHref(grouping, left, right, clID = '', crs = '') {
if (!left || !right) {
return '';
}
const u = `/diff?test=${grouping}&left=${left}&right=${right}`;
- if (issue) {
- return `${u}&issue=${issue}`;
+ if (clID) {
+ return `${u}&changelist_id=${clID}&crs=${crs}`;
}
return u;
}
diff --git a/golden/modules/common_test.js b/golden/modules/common_test.js
index 6e3e5e4..b776b05 100644
--- a/golden/modules/common_test.js
+++ b/golden/modules/common_test.js
@@ -66,12 +66,12 @@
});
describe('detailHref', () => {
- it('returns a path with and without an issue', () => {
+ it('returns a path with and without an changelist id', () => {
expect(detailHref('my-test', aDigest)).to.equal(
'/detail?test=my-test&digest=aaab78c9711cb79197d47f448ba51338',
);
- expect(detailHref('my-test', aDigest, '12345')).to.equal(
- '/detail?test=my-test&digest=aaab78c9711cb79197d47f448ba51338&issue=12345',
+ expect(detailHref('my-test', aDigest, '12345', 'gerrit')).to.equal(
+ '/detail?test=my-test&digest=aaab78c9711cb79197d47f448ba51338&changelist_id=12345&crs=gerrit',
);
});
});
@@ -87,8 +87,9 @@
);
});
it('supports an optional changelist id', () => {
- expect(diffPageHref('my-test', aDigest, bDigest, '12345')).to.equal(
- '/diff?test=my-test&left=aaab78c9711cb79197d47f448ba51338&right=bbb8b07beb4e1247c2cbafdb92b93e55&issue=12345',
+ expect(diffPageHref('my-test', aDigest, bDigest, '123456', 'github')).to.equal(
+ '/diff?test=my-test&left=aaab78c9711cb79197d47f448ba51338'
+ + '&right=bbb8b07beb4e1247c2cbafdb92b93e55&changelist_id=123456&crs=github',
);
});
});
diff --git a/golden/modules/details-page-sk/details-page-sk-demo.js b/golden/modules/details-page-sk/details-page-sk-demo.js
index f9901e5..e3679d2 100644
--- a/golden/modules/details-page-sk/details-page-sk-demo.js
+++ b/golden/modules/details-page-sk/details-page-sk-demo.js
@@ -20,7 +20,7 @@
// Load the demo page with some params to load if there aren't any already. 4 is an arbitrary
// small number to check against to determine "no query params"
if (window.location.search.length < 4) {
- const query = '?digest=6246b773851984c726cb2e1cb13510c2&test=My%20test%20has%20spaces&issue=12353';
+ const query = '?digest=6246b773851984c726cb2e1cb13510c2&test=My%20test%20has%20spaces&changelist_id=12353&crs=gerrit-internal';
history.pushState(null, '', window.location.origin + window.location.pathname + query);
}
diff --git a/golden/modules/details-page-sk/details-page-sk.js b/golden/modules/details-page-sk/details-page-sk.js
index a81e504..2aaa237 100644
--- a/golden/modules/details-page-sk/details-page-sk.js
+++ b/golden/modules/details-page-sk/details-page-sk.js
@@ -26,7 +26,8 @@
</div>`;
}
return html`
-<digest-details-sk .commits=${ele._commits} .issue=${ele._changeListID} .details=${ele._details}>
+<digest-details-sk .commits=${ele._commits} .changeListID=${ele._changeListID} .crs=${ele._crs}
+ .details=${ele._details}>
</digest-details-sk>
`;
};
@@ -37,18 +38,19 @@
this._grouping = '';
this._digest = '';
+ this._crs = '';
this._changeListID = '';
this._commits = [];
this._details = {};
this._didInitialLoad = false;
-
this._stateChanged = stateReflector(
/* getState */() => ({
// provide empty values
test: this._grouping, // TODO(kjlubick) rename test -> grouping
digest: this._digest,
- issue: this._changeListID, // TODO(kjlubick) rename issue -> changeListID
+ changelist_id: this._changeListID,
+ crs: this._crs,
}), /* setState */(newState) => {
if (!this._connected) {
return;
@@ -56,7 +58,8 @@
// default values if not specified.
this._grouping = newState.test || '';
this._digest = newState.digest || '';
- this._changeListID = newState.issue || '';
+ this._changeListID = newState.changelist_id || '';
+ this._crs = newState.crs || '';
this._fetch();
this._render();
},
@@ -86,7 +89,8 @@
sendBeginTask(this);
const url = `/json/details?test=${encodeURIComponent(this._grouping)}`
- + `&digest=${encodeURIComponent(this._digest)}&issue=${this._changeListID}`;
+ + `&digest=${encodeURIComponent(this._digest)}&changelist_id=${this._changeListID}`
+ + `&crs=${this._crs}`;
fetch(url, extra)
.then(jsonOrThrow)
.then((obj) => {
diff --git a/golden/modules/details-page-sk/details-page-sk_puppeteer_test.ts b/golden/modules/details-page-sk/details-page-sk_puppeteer_test.ts
index 428f785..52c3a49 100644
--- a/golden/modules/details-page-sk/details-page-sk_puppeteer_test.ts
+++ b/golden/modules/details-page-sk/details-page-sk_puppeteer_test.ts
@@ -56,18 +56,21 @@
expect(await getPropertyAsJSON(detailsPageSk!, '_grouping')).to.equal('My test has spaces');
expect(await getPropertyAsJSON(detailsPageSk!, '_digest')).to.equal('6246b773851984c726cb2e1cb13510c2');
expect(await getPropertyAsJSON(detailsPageSk!, '_changeListID')).to.equal('');
+ expect(await getPropertyAsJSON(detailsPageSk!, '_crs')).to.equal('');
});
- it('correctly extracts the changelistID (issue) if provided', async () => {
- await navigateTo(testBed.page, testBed.baseUrl, `${baseParams}&issue=65432`);
+ it('correctly extracts the changelistID if provided', async () => {
+ await navigateTo(testBed.page, testBed.baseUrl, `${baseParams}&changelist_id=65432&crs=gerrit-internal`);
const detailsPageSk = await testBed.page.$('details-page-sk');
expect(await getPropertyAsJSON(detailsPageSk!, '_grouping')).to.equal('My test has spaces');
expect(await getPropertyAsJSON(detailsPageSk!, '_digest')).to.equal('6246b773851984c726cb2e1cb13510c2');
expect(await getPropertyAsJSON(detailsPageSk!, '_changeListID')).to.equal('65432');
+ expect(await getPropertyAsJSON(detailsPageSk!, '_crs')).to.equal('gerrit-internal');
const digestDetails = await testBed.page.$('details-page-sk digest-details-sk');
- expect(await getPropertyAsJSON(digestDetails!, '_issue')).to.equal('65432');
+ expect(await getPropertyAsJSON(digestDetails!, 'changeListID')).to.equal('65432');
+ expect(await getPropertyAsJSON(digestDetails!, 'crs')).to.equal('gerrit-internal');
});
});
});
diff --git a/golden/modules/diff-page-sk/diff-page-sk-demo.js b/golden/modules/diff-page-sk/diff-page-sk-demo.js
index 18c8646..91da5b5 100644
--- a/golden/modules/diff-page-sk/diff-page-sk-demo.js
+++ b/golden/modules/diff-page-sk/diff-page-sk-demo.js
@@ -18,7 +18,8 @@
// Load the demo page with some params to load if there aren't any already.
if (window.location.search.length < 4) {
- const query = '?left=6246b773851984c726cb2e1cb13510c2&right=99c58c7002073346ff55f446d47d6311&test=My%20test%20has%20spaces&issue=12353';
+ const query = '?left=6246b773851984c726cb2e1cb13510c2&right=99c58c7002073346ff55f446d47d6311&'+
+ 'test=My%20test%20has%20spaces&changelist_id=12353&crs=gerrit';
history.pushState(null, '', window.location.origin + window.location.pathname + query);
}
diff --git a/golden/modules/diff-page-sk/diff-page-sk.js b/golden/modules/diff-page-sk/diff-page-sk.js
index 0699ef5..af0ace6 100644
--- a/golden/modules/diff-page-sk/diff-page-sk.js
+++ b/golden/modules/diff-page-sk/diff-page-sk.js
@@ -18,9 +18,9 @@
return html`<h1>Loading...</h1>`;
}
return html`
-<digest-details-sk .issue=${ele._changeListID} .details=${ele._leftDetails} .right=${ele._rightDetails}>
-</digest-details-sk>
- `;
+<digest-details-sk .details=${ele._leftDetails} .right=${ele._rightDetails}
+ .changeListID=${ele._changeListID} .crs=${ele._crs}>
+</digest-details-sk>`;
};
define('diff-page-sk', class extends ElementSk {
@@ -30,19 +30,20 @@
this._grouping = '';
this._leftDigest = '';
this._rightDigest = '';
+ this._crs = '';
this._changeListID = '';
this._leftDetails = {};
this._rightDetails = {};
this._didInitialLoad = false;
-
this._stateChanged = stateReflector(
/* getState */() => ({
// provide empty values
test: this._grouping, // TODO(kjlubick) rename test -> grouping
left: this._leftDigest,
right: this._rightDigest,
- issue: this._changeListID, // TODO(kjlubick) rename issue -> changeListID
+ changelist_id: this._changeListID,
+ crs: this._crs,
}), /* setState */(newState) => {
if (!this._connected) {
return;
@@ -51,7 +52,8 @@
this._grouping = newState.test || '';
this._leftDigest = newState.left || '';
this._rightDigest = newState.right || '';
- this._changeListID = newState.issue || '';
+ this._changeListID = newState.changelist_id || '';
+ this._crs = newState.crs || '';
this._fetch();
this._render();
},
@@ -82,7 +84,9 @@
const url = `/json/diff?test=${encodeURIComponent(this._grouping)}`
+ `&left=${encodeURIComponent(this._leftDigest)}`
- + `&right=${encodeURIComponent(this._rightDigest)}&issue=${this._changeListID}`;
+ + `&right=${encodeURIComponent(this._rightDigest)}`
+ + `&changelist_id=${this._changeListID}&crs=${this._crs}`;
+
fetch(url, extra)
.then(jsonOrThrow)
.then((obj) => {
diff --git a/golden/modules/diff-page-sk/diff-page-sk_puppeteer_test.ts b/golden/modules/diff-page-sk/diff-page-sk_puppeteer_test.ts
index 4cc34c0..a9bf35d 100644
--- a/golden/modules/diff-page-sk/diff-page-sk_puppeteer_test.ts
+++ b/golden/modules/diff-page-sk/diff-page-sk_puppeteer_test.ts
@@ -37,19 +37,22 @@
expect(await getPropertyAsJSON(diffPageSk!, '_leftDigest')).to.equal('6246b773851984c726cb2e1cb13510c2');
expect(await getPropertyAsJSON(diffPageSk!, '_rightDigest')).to.equal('99c58c7002073346ff55f446d47d6311');
expect(await getPropertyAsJSON(diffPageSk!, '_changeListID')).to.equal('');
+ expect(await getPropertyAsJSON(diffPageSk!, '_crs')).to.equal('');
});
- it('correctly extracts the changelistID (issue) if provided', async () => {
- await navigateTo(testBed.page, testBed.baseUrl, `${baseParams}&issue=65432`);
+ it('correctly extracts the changelistIDif provided', async () => {
+ await navigateTo(testBed.page, testBed.baseUrl, `${baseParams}&changelist_id=65432&crs=gerrit`);
const diffPageSk = await testBed.page.$('diff-page-sk');
expect(await getPropertyAsJSON(diffPageSk!, '_grouping')).to.equal('My test has spaces');
expect(await getPropertyAsJSON(diffPageSk!, '_leftDigest')).to.equal('6246b773851984c726cb2e1cb13510c2');
expect(await getPropertyAsJSON(diffPageSk!, '_rightDigest')).to.equal('99c58c7002073346ff55f446d47d6311');
expect(await getPropertyAsJSON(diffPageSk!, '_changeListID')).to.equal('65432');
+ expect(await getPropertyAsJSON(diffPageSk!, '_crs')).to.equal('gerrit');
const digestDetails = await testBed.page.$('diff-page-sk digest-details-sk');
- expect(await getPropertyAsJSON(digestDetails!, '_issue')).to.equal('65432');
+ expect(await getPropertyAsJSON(digestDetails!, 'changeListID')).to.equal('65432');
+ expect(await getPropertyAsJSON(digestDetails!, 'crs')).to.equal('gerrit');
});
});
});
diff --git a/golden/modules/digest-details-sk/digest-details-sk-demo.js b/golden/modules/digest-details-sk/digest-details-sk-demo.js
index 7bda77a..a38d5fb 100644
--- a/golden/modules/digest-details-sk/digest-details-sk-demo.js
+++ b/golden/modules/digest-details-sk/digest-details-sk-demo.js
@@ -32,7 +32,8 @@
ele = document.createElement('digest-details-sk');
ele.details = typicalDetails;
ele.commits = twoHundredCommits;
-ele.issue = '12345';
+ele.changeListID = '12345';
+ele.crs = 'gerrit';
$$('#changelist_id').appendChild(ele);
ele = document.createElement('digest-details-sk');
diff --git a/golden/modules/digest-details-sk/digest-details-sk.js b/golden/modules/digest-details-sk/digest-details-sk.js
index 8d37a6a..c50e1ec 100644
--- a/golden/modules/digest-details-sk/digest-details-sk.js
+++ b/golden/modules/digest-details-sk/digest-details-sk.js
@@ -90,7 +90,7 @@
return html`
<div class=metrics_and_triage>
<div>
- <a href=${diffPageHref(ele._grouping, ele._digest, ele.right.digest, ele.issue)}
+ <a href=${diffPageHref(ele._grouping, ele._digest, ele.right.digest, ele.changeListID, ele.crs)}
target=_blank rel=noopener class=diffpage_link>
Diff Details
</a>
@@ -121,7 +121,7 @@
const left = {
digest: ele._digest,
title: truncateWithEllipses(ele._digest),
- detail: detailHref(ele._grouping, ele._digest, ele.issue),
+ detail: detailHref(ele._grouping, ele._digest, ele.changeListID, ele.crs),
};
if (!ele.right) {
return html`<image-compare-sk .left=${left}></image-compare-sk>`;
@@ -129,7 +129,7 @@
const right = {
digest: ele.right.digest,
title: ele.right.status === 'positive' ? 'Closest Positive' : 'Closest Negative',
- detail: detailHref(ele._grouping, ele.right.digest, ele.issue),
+ detail: detailHref(ele._grouping, ele.right.digest, ele.changeListID, ele.crs),
};
return html`<image-compare-sk .left=${left} .right=${right}></image-compare-sk>`;
};
@@ -142,8 +142,8 @@
<div class=trace_info>
<dots-sk .value=${ele._traces} .commits=${ele._commits} @hover=${ele._hoverOverTrace}
@mouseleave=${ele._clearTraceHighlights} @showblamelist=${ele._showBlamelist}></dots-sk>
- <dots-legend-sk .digests=${ele._traces.digests} .issue=${ele.issue} .test=${ele._grouping}
-.totalDigests=${ele._traces.total_digests || 0}></dots-legend-sk>
+ <dots-legend-sk .digests=${ele._traces.digests} .changeListID=${ele.changeListID} .crs=${ele.crs}
+ .test=${ele._grouping} .totalDigests=${ele._traces.total_digests || 0}></dots-legend-sk>
</div>`;
};
@@ -188,7 +188,8 @@
this._params = null;
this._traces = null;
this._refDiffs = {};
- this._issue = '';
+ this._changeListID = '';
+ this._crs = '';
this._triageHistory = [];
this._commits = [];
@@ -234,13 +235,28 @@
}
/**
- * @prop issue {string} The changelist id (or empty string if this is the master branch).
- * TODO(kjlubick) rename this to changelistID.
+ * @prop changeListID {string} The changelist id (or empty string if this is the master branch).
*/
- get issue() { return this._issue; }
+ get changeListID() { return this._changeListID; }
+ set changeListID(id) {
+ this._changeListID = id;
+ this._render();
+ }
+
+ // For backwards compatibility with Polymer search page. synnonym for changeListID
set issue(id) {
- this._issue = id;
+ this._changeListID = id;
+ this._render();
+ }
+
+ /**
+ * @prop crs {string} The Code Review System (e.g. "gerrit") if changeListID is set.
+ */
+ get crs() { return this._crs; }
+
+ set crs(c) {
+ this._crs = c;
this._render();
}
@@ -367,8 +383,9 @@
testDigestStatus: {},
};
postBody.testDigestStatus[this._grouping] = digestStatus;
- if (this.issue) {
- postBody.issue = this.issue;
+ if (this.changeListID) {
+ postBody.changelist_id = this.changeListID;
+ postBody.crs = this.crs;
}
sendBeginTask(this);
diff --git a/golden/modules/digest-details-sk/digest-details-sk_test.js b/golden/modules/digest-details-sk/digest-details-sk_test.js
index a9e6b2c..5cb0e57 100644
--- a/golden/modules/digest-details-sk/digest-details-sk_test.js
+++ b/golden/modules/digest-details-sk/digest-details-sk_test.js
@@ -118,32 +118,38 @@
beforeEach(() => {
digestDetailsSk.details = typicalDetails;
digestDetailsSk.commits = twoHundredCommits;
- digestDetailsSk.issue = '12345';
+ digestDetailsSk.changeListID = '12345';
+ digestDetailsSk.crs = 'github';
});
it('includes changelist id on the appropriate links', () => {
- // (cluster doesn't have issue for now, since that was the way it was done before).
- // TODO(kjlubick) should cluster take changelist ID?
+ // (cluster doesn't have changelist id for now, since that was the way it was done before).
+ // TODO(kjlubick) the new cluster page takes changelist_id and crs
const imgComp = $$('.comparison image-compare-sk', digestDetailsSk);
expect(imgComp.left).to.deep.equal({
digest: '6246b773851984c726cb2e1cb13510c2',
title: '6246b7738519...',
- detail: '/detail?test=dots-legend-sk_too-many-digests&digest=6246b773851984c726cb2e1cb13510c2&issue=12345',
+ detail: '/detail?test=dots-legend-sk_too-many-digests'
+ + '&digest=6246b773851984c726cb2e1cb13510c2&changelist_id=12345&crs=github',
});
expect(imgComp.right).to.deep.equal({
digest: '99c58c7002073346ff55f446d47d6311',
title: 'Closest Positive',
- detail: '/detail?test=dots-legend-sk_too-many-digests&digest=99c58c7002073346ff55f446d47d6311&issue=12345',
+ detail: '/detail?test=dots-legend-sk_too-many-digests&'
+ + 'digest=99c58c7002073346ff55f446d47d6311&changelist_id=12345&crs=github',
});
expect($$('.metrics_and_triage a.diffpage_link', digestDetailsSk).href).to.contain(
- '/diff?test=dots-legend-sk_too-many-digests&left=6246b773851984c726cb2e1cb13510c2&right=99c58c7002073346ff55f446d47d6311&issue=12345',
+ '/diff?test=dots-legend-sk_too-many-digests&left=6246b773851984c726cb2e1cb13510c2'
+ + '&right=99c58c7002073346ff55f446d47d6311&changelist_id=12345&crs=github',
);
});
- it('passes issue to appropriate subelements', () => {
- expect($$('.trace_info dots-legend-sk', digestDetailsSk).issue).to.equal('12345');
+ it('passes changeListID and crs to appropriate subelements', () => {
+ const dots = $$('.trace_info dots-legend-sk', digestDetailsSk);
+ expect(dots.changeListID).to.equal('12345');
+ expect(dots.crs).to.equal('github');
});
describe('RPC requests', () => {
@@ -155,7 +161,7 @@
it('includes changelist id when triaging', async () => {
const endPromise = eventPromise('end-task');
fetchMock.post('/json/triage', (url, req) => {
- expect(req.body).to.equal('{"testDigestStatus":{"dots-legend-sk_too-many-digests":{"6246b773851984c726cb2e1cb13510c2":"negative"}},"issue":"12345"}');
+ expect(req.body).to.equal('{"testDigestStatus":{"dots-legend-sk_too-many-digests":{"6246b773851984c726cb2e1cb13510c2":"negative"}},"changelist_id":"12345","crs":"github"}');
return 200;
});
diff --git a/golden/modules/dots-legend-sk/dots-legend-sk-demo.js b/golden/modules/dots-legend-sk/dots-legend-sk-demo.js
index fb44f35..9f2df5b 100644
--- a/golden/modules/dots-legend-sk/dots-legend-sk-demo.js
+++ b/golden/modules/dots-legend-sk/dots-legend-sk-demo.js
@@ -18,11 +18,11 @@
{ digest: 'b00cb97f0d4dd7b22fb9af5378918d9f', status: 'untriaged' },
];
-function newDotsLegendSk(parentSelector, id, digests, issue, test) {
+function newDotsLegendSk(parentSelector, id, digests, clID, test) {
const dotsLegendSk = document.createElement('dots-legend-sk');
dotsLegendSk.id = id;
dotsLegendSk.digests = digests;
- dotsLegendSk.issue = issue;
+ dotsLegendSk.changeListID = clID;
dotsLegendSk.test = test;
dotsLegendSk.totalDigests = digests.length;
$$(parentSelector).appendChild(dotsLegendSk);
diff --git a/golden/modules/dots-legend-sk/dots-legend-sk.js b/golden/modules/dots-legend-sk/dots-legend-sk.js
index 0afb00a..1a80f46 100644
--- a/golden/modules/dots-legend-sk/dots-legend-sk.js
+++ b/golden/modules/dots-legend-sk/dots-legend-sk.js
@@ -13,6 +13,8 @@
DOT_FILL_COLORS,
MAX_UNIQUE_DIGESTS,
} from '../dots-sk/constants';
+import { detailHref, diffPageHref } from '../common';
+
import 'elements-sk/icon/cancel-icon-sk';
import 'elements-sk/icon/check-circle-icon-sk';
import 'elements-sk/icon/help-icon-sk';
@@ -85,7 +87,8 @@
constructor() {
super(template);
this._digests = [];
- this._issue = '';
+ this._changeListID = '';
+ this._crs = '';
this._test = '';
this._totalDigests = 0;
}
@@ -107,12 +110,22 @@
}
/**
- * @prop issue {string} An issue number/ID.
+ * @prop changeListID {string} The changelist id (or empty string if this is the master branch).
*/
- get issue() { return this._issue; }
+ get changeListID() { return this._changeListID; }
- set issue(issue) {
- this._issue = issue;
+ set changeListID(id) {
+ this._changeListID = id;
+ this._render();
+ }
+
+ /**
+ * @prop crs {string} The Code Review System (e.g. "gerrit") if changeListID is set.
+ */
+ get crs() { return this._crs; }
+
+ set crs(c) {
+ this._crs = c;
this._render();
}
@@ -139,17 +152,11 @@
}
_digestDetailHref(index) {
- return `${'/detail'
- + `?test=${encodeURIComponent(this._test)}`
- + `&digest=${this._digests[index].digest}`}${
- this._issue ? `&issue=${this._issue}` : ''}`;
+ return detailHref(this._test, this._digests[index].digest, this.changeListID, this.crs);
}
_digestDiffHref(index) {
- return `${'/diff'
- + `?test=${encodeURIComponent(this._test)}`
- + `&left=${this._digests[0].digest}`
- + `&right=${this._digests[index].digest}`}${
- this._issue ? `&issue=${this._issue}` : ''}`;
+ return diffPageHref(this._test, this._digests[0].digest, this._digests[index].digest,
+ this.changeListID, this.crs);
}
});
diff --git a/golden/modules/dots-legend-sk/dots-legend-sk_test.js b/golden/modules/dots-legend-sk/dots-legend-sk_test.js
index 5aed572..1a3b266 100644
--- a/golden/modules/dots-legend-sk/dots-legend-sk_test.js
+++ b/golden/modules/dots-legend-sk/dots-legend-sk_test.js
@@ -81,14 +81,16 @@
]);
});
- describe('with issue number', () => {
+ describe('with CL ID and crs', () => {
beforeEach(() => {
dotsLegendSk.test = 'My Test';
- dotsLegendSk.issue = '123456';
+ dotsLegendSk.changeListID = '123456';
+ dotsLegendSk.crs = 'gerrit';
});
it('renders digest links correctly', () => {
- const digestLinkFor = (d) => `/detail?test=My%20Test&digest=${d}&issue=123456`;
+ const digestLinkFor = (d) => `/detail?test=My%20Test&digest=${d}`
+ + '&changelist_id=123456&crs=gerrit';
expect(digestLinks(dotsLegendSk)).to.deep.equal([
digestLinkFor('00000000000000000000000000000000'),
digestLinkFor('11111111111111111111111111111111'),
@@ -100,7 +102,7 @@
it('renders diff links correctly', () => {
const diffLinkFor = (d) => '/diff?test=My%20Test&left=00000000000000000000000000000000'
- + `&right=${d}&issue=123456`;
+ + `&right=${d}&changelist_id=123456&crs=gerrit`;
expect(diffLinks(dotsLegendSk)).to.deep.equal([
diffLinkFor('11111111111111111111111111111111'),
diffLinkFor('22222222222222222222222222222222'),
diff --git a/golden/modules/image-compare-sk/image-compare-sk.js b/golden/modules/image-compare-sk/image-compare-sk.js
index 266e314..b625512 100644
--- a/golden/modules/image-compare-sk/image-compare-sk.js
+++ b/golden/modules/image-compare-sk/image-compare-sk.js
@@ -72,8 +72,8 @@
this._left = {
digest: '',
title: '',
- // We can't derive the detail url w/o also passing down issue (and maybe other things), so
- // for simplicity, we have the client compute what detail page they want to link to.
+ // We can't derive the detail url w/o also passing down changelistID, crs etc, so we have
+ // the caller compute those URLs and pass them into this element.
detail: '',
};
this._right = null;
diff --git a/golden/modules/triagelog-page-sk/triagelog-page-sk.js b/golden/modules/triagelog-page-sk/triagelog-page-sk.js
index 4931334..54e858e 100644
--- a/golden/modules/triagelog-page-sk/triagelog-page-sk.js
+++ b/golden/modules/triagelog-page-sk/triagelog-page-sk.js
@@ -68,8 +68,8 @@
const detailsEntryTemplate = (el, detailsEntry) => {
let detailHref = `/detail?test=${detailsEntry.test_name}&digest=${detailsEntry.digest}`;
- if (el._issue) {
- detailHref += `&issue=${el._issue}`;
+ if (el._changelistID) {
+ detailHref += `&changelist_id=${el._changelistID}&crs=${el._crs}`;
}
return html`
<tr class=details>
@@ -92,12 +92,18 @@
this._entries = []; // Log entries fetched from the server.
this._pageOffset = 0; // Reflected in the URL.
this._pageSize = 0; // Reflected in the URL.
- this._issue = 0; // Reflected in the URL.
+ this._changelistID = ''; // Reflected in the URL.
+ this._crs = ''; // Code Review System (e.g. 'gerrit', 'github')
this._totalEntries = 0; // Total number of entries in the server.
// stateReflector will trigger on DomReady.
this._stateChanged = stateReflector(
- /* getState */ () => this._getState(),
+ /* getState */ () => ({
+ offset: this._pageOffset,
+ page_size: this._pageSize,
+ changelist_id: this._changelistID,
+ crs: this._crs,
+ }),
/* setState */ (newState) => {
// The stateReflector's lingering popstate event handler will continue
// to call this function on e.g. browser back button clicks long after
@@ -108,21 +114,14 @@
this._pageOffset = newState.offset || 0;
this._pageSize = newState.page_size || 20;
- this._issue = newState.issue || 0;
+ this._changelistID = newState.changelist_id || '';
+ this._crs = newState.crs || '';
this._render();
this._fetchEntries();
},
);
}
- _getState() {
- return {
- offset: this._pageOffset,
- page_size: this._pageSize,
- issue: this._issue,
- };
- }
-
connectedCallback() {
super.connectedCallback();
this._render();
@@ -150,8 +149,8 @@
_fetchEntries(sendBusyDoneEvents = true) {
let url = `/json/triagelog?details=true&offset=${this._pageOffset}`
+ `&size=${this._pageSize}`;
- if (this._issue) {
- url += `&issue=${this._issue}`;
+ if (this._changelistID) {
+ url += `&changelist_id=${this._changelistID}&crs=${this._crs}`;
}
if (sendBusyDoneEvents) {
sendBeginTask(this);
diff --git a/golden/modules/triagelog-page-sk/triagelog-page-sk_test.js b/golden/modules/triagelog-page-sk/triagelog-page-sk_test.js
index 4311e9f..484d154 100644
--- a/golden/modules/triagelog-page-sk/triagelog-page-sk_test.js
+++ b/golden/modules/triagelog-page-sk/triagelog-page-sk_test.js
@@ -84,15 +84,15 @@
expectFirstPageOfResultsFirstEntryUndone(triagelogPageSk);
});
- it('handles the "issue" URL parameter', async () => {
+ it('handles the "changelist_id" and "crs" URL parameters', async () => {
fetchMock.get(
- '/json/triagelog?details=true&offset=0&size=20&issue=123456',
+ '/json/triagelog?details=true&offset=0&size=20&changelist_id=123456&crs=gerrit',
firstPage,
);
- setQueryString('?issue=123456');
+ setQueryString('?changelist_id=123456&crs=gerrit');
const triagelogPageSk = await loadTriagelogPageSk(); // Load first page.
- expectQueryStringToEqual('?issue=123456'); // No changes to the URL.
- expectFirstPageOfResults(triagelogPageSk, '123456' /*= changelistID */);
+ expectQueryStringToEqual('?changelist_id=123456&crs=gerrit'); // No changes to the URL.
+ expectFirstPageOfResults(triagelogPageSk, '123456' /*= changelistID */, 'gerrit' /*= crs */);
});
describe('URL parameters', () => {
@@ -207,7 +207,7 @@
expect($$('tbody', triagelogPageSk).children).to.be.empty;
}
-function expectFirstPageOfResults(triagelogPageSk, changelistID = '') {
+function expectFirstPageOfResults(triagelogPageSk, changelistID = '', crs = '') {
expect(nthEntryTimestamp(triagelogPageSk, 0)).to.equal(toLocalDateStr(1572000000000));
expect(nthEntryAuthor(triagelogPageSk, 0)).to.equal('alpha@google.com');
expect(nthEntryNumChanges(triagelogPageSk, 0)).to.equal(2);
@@ -220,9 +220,9 @@
'/detail?test=async_rescale_and_read_dog_up&digest=f16298eb14e19f9230fe81615200561f',
);
if (changelistID) {
- expect(digestHref).to.contain(`&issue=${changelistID}`);
+ expect(digestHref).to.contain(`&changelist_id=${changelistID}&crs=${crs}`);
} else {
- expect(digestHref).not.to.contain('&issue');
+ expect(digestHref).not.to.contain('&changelist_id');
}
expect(nthEntryTimestamp(triagelogPageSk, 1)).to.equal(toLocalDateStr(1571900000000));
@@ -237,9 +237,9 @@
'/detail?test=async_rescale_and_read_rose&digest=35c77280a7d5378033f9bf8f3c755e78',
);
if (changelistID) {
- expect(digestHref).to.contain(`&issue=${changelistID}`);
+ expect(digestHref).to.contain(`&changelist_id=${changelistID}&crs=${crs}`);
} else {
- expect(digestHref).not.to.contain('&issue');
+ expect(digestHref).not.to.contain('&changelist_id');
}
expect(nthEntryTimestamp(triagelogPageSk, 2)).to.equal(toLocalDateStr(1571800000000));
@@ -254,9 +254,9 @@
'/detail?test=draw_image_set&digest=b788aadee662c2b0390d698cbe68b808',
);
if (changelistID) {
- expect(digestHref).to.contain(`&issue=${changelistID}`);
+ expect(digestHref).to.contain(`&changelist_id=${changelistID}&crs=${crs}`);
} else {
- expect(digestHref).not.to.contain('&issue');
+ expect(digestHref).not.to.contain('&changelist_id');
}
expect(nthDetailsRowTestName(triagelogPageSk, 3)).to.equal('filterbitmap_text_7.00pt');
@@ -267,9 +267,9 @@
'/detail?test=filterbitmap_text_7.00pt&digest=454b4b547bc6ceb4cdeb3305553be98a',
);
if (changelistID) {
- expect(digestHref).to.contain(`&issue=${changelistID}`);
+ expect(digestHref).to.contain(`&changelist_id=${changelistID}&crs=${crs}`);
} else {
- expect(digestHref).not.to.contain('&issue');
+ expect(digestHref).not.to.contain('&changelist_id');
}
}
