Merge "Add hovercard-accounts to avatar stacks."
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 2e175e6..32c05b0 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1672,6 +1672,16 @@
+
Default is 5 minutes.
+[[change.diff3ConflictView]]change.diff3ConflictView::
++
+Use the diff3 formatter for merge commits with conflicts. With diff3 when
+the conflicts are shown in the "Auto Merge" view, the base section from the
+common parents will be shown as well.
+This setting takes effect when generating the automerge, which happens on upload.
+Changing the setting leaves existing changes unaffected.
++
+Default is false.
+
[[changeCleanup]]
=== Section changeCleanup
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 9e71df7..528be41 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -139,11 +139,16 @@
)]}'
{
"some-project": {
- "id": "some-project"
+ "id": "some-project",
+ _more_projects: true
}
}
----
+If the number of projects matching the query exceeds either the
+internal limit or a supplied `limit` query parameter, the last project
+object has a `_more_projects: true` JSON field set.
+
[[suggest-projects]]
Prefix(p)::
@@ -420,6 +425,10 @@
GET /projects/?query=<query>&limit=25 HTTP/1.0
----
+If the number of projects matching the query exceeds either the
+internal limit or a supplied `limit` query parameter, the last project
+object has a `_more_projects: true` JSON field set.
+
The `/projects/` URL also accepts a start integer in the `start`
parameter. The results will skip `start` projects from project list.
@@ -4370,28 +4379,31 @@
The `ProjectInfo` entity contains information about a project.
[options="header",cols="1,^2,4"]
-|===========================
-|Field Name ||Description
-|`id` ||The URL encoded project name.
-|`name` |
+|=============================
+|Field Name ||Description
+|`id` ||The URL encoded project name.
+|`name` |
not set if returned in a map where the project name is used as map key|
The name of the project.
-|`parent` |optional|
+|`parent` |optional|
The name of the parent project. +
`?-<n>` if the parent project is not visible (`<n>` is a number which
is increased for each non-visible project).
-|`description` |optional|The description of the project.
-|`state` |optional|`ACTIVE`, `READ_ONLY` or `HIDDEN`.
-|`branches` |optional|Map of branch names to HEAD revisions.
-|`labels` |optional|
+|`description` |optional|The description of the project.
+|`state` |optional|`ACTIVE`, `READ_ONLY` or `HIDDEN`.
+|`branches` |optional|Map of branch names to HEAD revisions.
+|`labels` |optional|
Map of label names to
link:#label-type-info[LabelTypeInfo] entries.
This field is filled for link:#create-project[Create Project] and
link:#get-project[Get Project] calls.
-|`web_links` |optional|
+|`web_links` |optional|
Links to the project in external sites as a list of
link:rest-api-changes.html#web-link-info[WebLinkInfo] entries.
-|===========================
+|`_more_projects`|optional, not set if `false`|
+Whether the query would deliver more results if not limited. +
+Only set on the last project that is returned.
+|=============================
[[project-input]]
=== ProjectInput
diff --git a/java/com/google/gerrit/extensions/common/ProjectInfo.java b/java/com/google/gerrit/extensions/common/ProjectInfo.java
index 46b2599..2b00710 100644
--- a/java/com/google/gerrit/extensions/common/ProjectInfo.java
+++ b/java/com/google/gerrit/extensions/common/ProjectInfo.java
@@ -27,4 +27,10 @@
public Map<String, String> branches;
public List<WebLinkInfo> webLinks;
public Map<String, LabelTypeInfo> labels;
+
+ /**
+ * Whether the query would deliver more results if not limited. Only set on the last project that
+ * is returned as a query result.
+ */
+ public Boolean _moreProjects;
}
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index a522a2f..73aec64 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -286,7 +286,6 @@
return commit;
}
- @SuppressWarnings("resource") // TemporaryBuffer requires calling close before reading.
public static ObjectId mergeWithConflicts(
RevWalk rw,
ObjectInserter ins,
@@ -297,6 +296,21 @@
RevCommit theirs,
Map<String, MergeResult<? extends Sequence>> mergeResults)
throws IOException {
+ return mergeWithConflicts(rw, ins, dc, oursName, ours, theirsName, theirs, mergeResults, false);
+ }
+
+ @SuppressWarnings("resource") // TemporaryBuffer requires calling close before reading.
+ public static ObjectId mergeWithConflicts(
+ RevWalk rw,
+ ObjectInserter ins,
+ DirCache dc,
+ String oursName,
+ RevCommit ours,
+ String theirsName,
+ RevCommit theirs,
+ Map<String, MergeResult<? extends Sequence>> mergeResults,
+ boolean diff3Format)
+ throws IOException {
rw.parseBody(ours);
rw.parseBody(theirs);
String oursMsg = ours.getShortMessage();
@@ -324,7 +338,11 @@
try {
// TODO(dborowitz): Respect inCoreLimit here.
buf = new TemporaryBuffer.LocalFile(null, 10 * 1024 * 1024);
- fmt.formatMerge(buf, p, "BASE", oursNameFormatted, theirsNameFormatted, UTF_8);
+ if (diff3Format) {
+ fmt.formatMergeDiff3(buf, p, "BASE", oursNameFormatted, theirsNameFormatted, UTF_8);
+ } else {
+ fmt.formatMerge(buf, p, "BASE", oursNameFormatted, theirsNameFormatted, UTF_8);
+ }
buf.close(); // Flush file and close for writes, but leave available for reading.
try (InputStream in = buf.openInputStream()) {
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index 0818f23..e27faf6 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -83,6 +83,10 @@
return cfg.getBoolean("change", null, "cacheAutomerge", true);
}
+ public static boolean diff3ConflictView(Config cfg) {
+ return cfg.getBoolean("change", null, "diff3ConflictView", false);
+ }
+
private enum OperationType {
CACHE_LOAD,
IN_MEMORY_WRITE,
@@ -93,6 +97,7 @@
private final Timer1<OperationType> latency;
private final Provider<PersonIdent> gerritIdentProvider;
private final boolean save;
+ private final boolean useDiff3;
private final ThreeWayMergeStrategy configuredMergeStrategy;
@Inject
@@ -117,6 +122,7 @@
.setUnit("milliseconds"),
operationTypeField);
this.save = cacheAutomerge(cfg);
+ this.useDiff3 = diff3ConflictView(cfg);
this.gerritIdentProvider = gerritIdentProvider;
this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
}
@@ -254,7 +260,8 @@
merge.getParent(0),
"BRANCH",
merge.getParent(1),
- m.getMergeResults());
+ m.getMergeResults(),
+ useDiff3);
}
logger.atFine().log("AutoMerge treeId=%s", treeId.name());
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java b/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java
index 055648d..88aec1a 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java
@@ -41,6 +41,8 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.index.project.ProjectField;
+import com.google.gerrit.index.project.ProjectIndexCollection;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.WebLinks;
@@ -184,6 +186,7 @@
private AccountGroup.UUID groupUuid;
private final Provider<QueryProjects> queryProjectsProvider;
private final boolean listProjectsFromIndex;
+ private final ProjectIndexCollection projectIndexes;
@Inject
protected ListProjectsImpl(
@@ -196,7 +199,8 @@
ProjectNode.Factory projectNodeFactory,
WebLinks webLinks,
Provider<QueryProjects> queryProjectsProvider,
- @GerritServerConfig Config config) {
+ @GerritServerConfig Config config,
+ ProjectIndexCollection projectIndexes) {
this.currentUser = currentUser;
this.projectCache = projectCache;
this.groupResolver = groupResolver;
@@ -207,6 +211,7 @@
this.webLinks = webLinks;
this.queryProjectsProvider = queryProjectsProvider;
this.listProjectsFromIndex = config.getBoolean("gerrit", "listProjectsFromIndex", false);
+ this.projectIndexes = projectIndexes;
}
public List<String> getShowBranch() {
@@ -251,11 +256,15 @@
return display(null);
}
- private Optional<String> expressAsProjectsQuery() {
+ private Optional<String> expressAsProjectsQuery() throws BadRequestException {
return listProjectsFromIndex
&& !all
&& state != HIDDEN
- && isNullOrEmpty(matchPrefix)
+ && (isNullOrEmpty(matchPrefix)
+ || projectIndexes
+ .getSearchIndex()
+ .getSchema()
+ .hasField(ProjectField.PREFIX_NAME_SPEC))
&& isNullOrEmpty(matchRegex)
&& type == FilterType.ALL
&& showBranch.isEmpty()
@@ -264,12 +273,25 @@
: Optional.empty();
}
- private String toQuery() {
+ private String toQuery() throws BadRequestException {
+ // QueryProjects supports specifying matchPrefix and matchSubstring at the same time, but to
+ // keep the behavior consistent regardless of whether 'gerrit.listProjectsFromIndex' is true or
+ // false, disallow specifying both at the same time here. This way
+ // 'gerrit.listProjectsFromIndex' can be troggled without breaking any caller.
+ if (matchPrefix != null) {
+ checkMatchOptions(matchSubstring == null);
+ } else if (matchSubstring != null) {
+ checkMatchOptions(matchPrefix == null);
+ }
+
List<String> queries = new ArrayList<>();
if (state != null) {
queries.add(String.format("(state:%s)", state.name()));
}
+ if (!isNullOrEmpty(matchPrefix)) {
+ queries.add(String.format("prefix:%s", matchPrefix));
+ }
if (!isNullOrEmpty(matchSubstring)) {
queries.add(String.format("substring:%s", matchSubstring));
}
@@ -353,6 +375,7 @@
Map<Project.NameKey, Boolean> accessibleParents = new HashMap<>();
PermissionBackend.WithUser perm = permissionBackend.user(currentUser);
final TreeMap<Project.NameKey, ProjectNode> treeMap = new TreeMap<>();
+ ProjectInfo lastInfo = null;
try {
Iterator<ProjectState> projectStatesIt = filter(perm).iterator();
while (projectStatesIt.hasNext()) {
@@ -384,10 +407,15 @@
continue;
}
if (limit > 0 && ++found > limit) {
+ if (lastInfo != null) {
+ lastInfo._moreProjects = true;
+ }
break;
}
ProjectInfo info = new ProjectInfo();
+ lastInfo = info;
+
info.name = projectName.get();
if (showTree && format.isJson()) {
addParentProjectInfo(hiddenNames, accessibleParents, perm, e, info);
diff --git a/java/com/google/gerrit/server/restapi/project/QueryProjects.java b/java/com/google/gerrit/server/restapi/project/QueryProjects.java
index ef64bad..dc7499d 100644
--- a/java/com/google/gerrit/server/restapi/project/QueryProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/QueryProjects.java
@@ -129,6 +129,9 @@
for (ProjectData pd : pds) {
projectInfos.add(json.format(pd.getProject()));
}
+ if (!projectInfos.isEmpty() && result.more()) {
+ projectInfos.get(projectInfos.size() - 1)._moreProjects = true;
+ }
return projectInfos;
} catch (QueryParseException e) {
throw new BadRequestException(e.getMessage(), e);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
index d7078d8..b1cc866 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -533,6 +533,23 @@
}
@Test
+ public void commitMessage_providedMessageWithCorrectChangeId() throws Exception {
+ initDestBranch();
+ String originalChangeId =
+ gApi.changes()
+ .create(new ChangeInput(project.get(), DESTINATION_BRANCH, "Default commit message"))
+ .info()
+ .changeId;
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.commitMessage = "custom commit message\n\nChange-Id: " + originalChangeId + "\n";
+
+ ChangeInfo result = gApi.changes().id(originalChangeId).applyPatch(in);
+
+ ChangeInfo info = get(result.changeId, CURRENT_REVISION, CURRENT_COMMIT);
+ assertThat(info.revisions.get(info.currentRevision).commit.message).isEqualTo(in.commitMessage);
+ }
+
+ @Test
public void commitMessage_providedMessageWithWrongChangeId() throws Exception {
initDestBranch();
String originalChangeId =
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
index 5a7d7b3..1df3f3e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
@@ -140,6 +140,33 @@
}
@Test
+ @GerritConfig(name = "gerrit.listProjectsFromIndex", value = "true")
+ public void listProjectsSetsMoreProjectsIfLimited_indexEnabled() throws Exception {
+ testListProjectsSetsMoreProjectsIfLimited();
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.listProjectsFromIndex", value = "false")
+ public void listProjectsSetsMoreProjectsIfLimited_indexDisabled() throws Exception {
+ testListProjectsSetsMoreProjectsIfLimited();
+ }
+
+ private void testListProjectsSetsMoreProjectsIfLimited() throws Exception {
+ for (int i = 0; i < 3; i++) {
+ projectOperations.newProject().name("prefix-" + i).create();
+ }
+
+ List<ProjectInfo> result = gApi.projects().list().withPrefix("prefix").get();
+ assertThat(Iterables.getLast(result)._moreProjects).isNull();
+
+ result = gApi.projects().list().withPrefix("prefix").withLimit(Integer.MAX_VALUE).get();
+ assertThat(Iterables.getLast(result)._moreProjects).isNull();
+
+ result = gApi.projects().list().withPrefix("prefix").withLimit(2).get();
+ assertThat(Iterables.getLast(result)._moreProjects).isTrue();
+ }
+
+ @Test
public void listProjectsToOutputStream() throws Exception {
int numInitialProjects = gApi.projects().list().get().size();
int numTestProjects = 5;
@@ -199,7 +226,18 @@
}
@Test
- public void listProjectsWithPrefix() throws Exception {
+ @GerritConfig(name = "gerrit.listProjectsFromIndex", value = "true")
+ public void listProjectsWithPrefix_indexEnabled() throws Exception {
+ testListProjectsWithPrefix();
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.listProjectsFromIndex", value = "false")
+ public void listProjectsWithPrefix_indexDisabled() throws Exception {
+ testListProjectsWithPrefix();
+ }
+
+ private void testListProjectsWithPrefix() throws Exception {
Project.NameKey someProject = projectOperations.newProject().name("listtest-p1").create();
Project.NameKey someOtherProject = projectOperations.newProject().name("listtest-p2").create();
projectOperations.newProject().name("other-prefix-project").create();
diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
index 3377b92..47d485d 100644
--- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
+++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
@@ -23,6 +23,7 @@
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
@@ -360,8 +361,10 @@
String query =
"name:" + project1.name + " OR name:" + project2.name + " OR name:" + project3.name;
List<ProjectInfo> result = assertQuery(query, project1, project2, project3);
+ assertThat(Iterables.getLast(result)._moreProjects).isNull();
- assertQuery(newQuery(query).withLimit(2), result.subList(0, 2));
+ result = assertQuery(newQuery(query).withLimit(2), result.subList(0, 2));
+ assertThat(Iterables.getLast(result)._moreProjects).isTrue();
}
@Test
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 02d5124..769b064 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -42,7 +42,7 @@
import {Timing} from '../../../constants/reporting';
import {changeModelToken} from '../../../models/change/change-model';
-interface FilePreview {
+export interface FilePreview {
filepath: string;
preview: DiffInfo;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-model/gr-comment-model.ts b/polygerrit-ui/app/elements/shared/gr-comment-model/gr-comment-model.ts
new file mode 100644
index 0000000..974329a
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-comment-model/gr-comment-model.ts
@@ -0,0 +1,31 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {Observable} from 'rxjs';
+import {filter} from 'rxjs/operators';
+import {define} from '../../../models/dependency';
+import {Model} from '../../../models/model';
+import {isDefined} from '../../../types/types';
+import {select} from '../../../utils/observable-util';
+import {Comment} from '../../../types/common';
+
+export interface CommentState {
+ comment: Comment;
+ commentedText?: string;
+}
+
+export const commentModelToken = define<CommentModel>('diff-model');
+
+export class CommentModel extends Model<CommentState | undefined> {
+ readonly comment$: Observable<Comment> = select(
+ this.state$.pipe(filter(isDefined)),
+ commentState => commentState.comment
+ );
+
+ readonly commentedText$: Observable<string | undefined> = select(
+ this.state$.pipe(filter(isDefined)),
+ commentState => commentState.commentedText
+ );
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 30576c8..24777fb 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -315,6 +315,8 @@
font-size: var(--font-size-normal);
font-weight: var(--font-weight-normal);
line-height: var(--line-height-normal);
+ }
+ gr-diff#diff {
/* Explicitly set the background color of the diff. We
* cannot use the diff content type ab because of the skip chunk preceding
* it, diff processor assumes the chunk of type skip/ab can be collapsed
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 69d7387..d27be9f 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -17,7 +17,7 @@
import {getAppContext} from '../../../services/app-context';
import {css, html, LitElement, nothing, PropertyValues} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
-import {resolve} from '../../../models/dependency';
+import {provide, resolve} from '../../../models/dependency';
import {GrTextarea} from '../gr-textarea/gr-textarea';
import {
AccountDetailInfo,
@@ -68,6 +68,10 @@
import {modalStyles} from '../../../styles/gr-modal-styles';
import {KnownExperimentId} from '../../../services/flags/flags';
import {pluginLoaderToken} from '../gr-js-api-interface/gr-plugin-loader';
+import {
+ CommentModel,
+ commentModelToken,
+} from '../gr-comment-model/gr-comment-model';
// visible for testing
export const AUTO_SAVE_DEBOUNCE_DELAY_MS = 2000;
@@ -226,6 +230,8 @@
private readonly shortcuts = new ShortcutController(this);
+ private commentModel = new CommentModel(undefined);
+
/**
* This is triggered when the user types into the editing textarea. We then
* debounce it and call autoSave().
@@ -246,6 +252,7 @@
constructor() {
super();
+ provide(this, commentModelToken, () => this.commentModel);
// Allow the shortcuts to bubble up so that GrReplyDialog can respond to
// them as well.
this.shortcuts.addLocal({key: Key.ESC}, () => this.handleEsc(), {
@@ -979,6 +986,22 @@
whenVisible(this, () => this.textarea?.putCursorAtEnd());
}
}
+ if (changed.has('changeNum') || changed.has('comment')) {
+ if (
+ !this.flagsService.isEnabled(
+ KnownExperimentId.DIFF_FOR_USER_SUGGESTED_EDIT
+ ) ||
+ !this.changeNum ||
+ !this.comment
+ )
+ return;
+ (async () => {
+ const commentedText = await this.getCommentedCode();
+ this.commentModel.updateState({
+ commentedText,
+ });
+ })();
+ }
}
override willUpdate(changed: PropertyValues) {
@@ -987,6 +1010,11 @@
if (isDraft(this.comment) && isError(this.comment)) {
this.edit();
}
+ if (this.comment) {
+ this.commentModel.updateState({
+ comment: this.comment,
+ });
+ }
}
if (changed.has('editing')) {
this.onEditingChanged();
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
index 3f97ded..b6af123 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
@@ -187,7 +187,7 @@
}
protected override willUpdate(changedProperties: PropertyValues): void {
- if (changedProperties.has('items') || changedProperties.has('value')) {
+ if (changedProperties.has('value')) {
this.handleValueChange();
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index becfbc2..88866d4 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -14,11 +14,15 @@
import {getAppContext} from '../../../services/app-context';
import './gr-formatted-text';
import {GrFormattedText} from './gr-formatted-text';
-import {createConfig} from '../../../test/test-data-generators';
+import {createComment, createConfig} from '../../../test/test-data-generators';
import {queryAndAssert, waitUntilObserved} from '../../../test/test-utils';
import {CommentLinks, EmailAddress} from '../../../api/rest-api';
import {testResolver} from '../../../test/common-test-setup';
import {GrAccountChip} from '../gr-account-chip/gr-account-chip';
+import {
+ CommentModel,
+ commentModelToken,
+} from '../gr-comment-model/gr-comment-model';
suite('gr-formatted-text tests', () => {
let element: GrFormattedText;
@@ -37,6 +41,7 @@
testResolver(changeModelToken),
getAppContext().restApiService
);
+ const commentModel = new CommentModel({comment: createComment()});
await setCommentLinks({
customLinkRewrite: {
match: '(LinkRewriteMe)',
@@ -54,9 +59,13 @@
element = (
await fixture(
wrapInProvider(
- html`<gr-formatted-text></gr-formatted-text>`,
- configModelToken,
- configModel
+ wrapInProvider(
+ html`<gr-formatted-text></gr-formatted-text>`,
+ configModelToken,
+ configModel
+ ),
+ commentModelToken,
+ commentModel
)
)
).querySelector('gr-formatted-text')!;
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
index 2b13b40..e484d6b 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
@@ -3,12 +3,34 @@
* Copyright 2023 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
+import '../../../embed/diff/gr-diff/gr-diff';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-icon/gr-icon';
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
-import {css, html, LitElement, nothing} from 'lit';
-import {customElement} from 'lit/decorators.js';
+import {css, html, LitElement, nothing, PropertyValues} from 'lit';
+import {customElement, state} from 'lit/decorators.js';
+import {getAppContext} from '../../../services/app-context';
+import {Comment} from '../../../types/common';
import {fire} from '../../../utils/event-util';
+import {anyLineTooLong} from '../../../utils/diff-util';
+import {
+ DiffLayer,
+ DiffPreferencesInfo,
+ DiffViewMode,
+ RenderPreferences,
+} from '../../../api/diff';
+import {when} from 'lit/directives/when.js';
+import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
+import {resolve} from '../../../models/dependency';
+import {highlightServiceToken} from '../../../services/highlight/highlight-service';
+import {NumericChangeId} from '../../../api/rest-api';
+import {changeModelToken} from '../../../models/change/change-model';
+import {subscribe} from '../../lit/subscription-controller';
+import {FilePreview} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
+import {userModelToken} from '../../../models/user/user-model';
+import {createUserFixSuggestion} from '../../../utils/comment-util';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {commentModelToken} from '../gr-comment-model/gr-comment-model';
declare global {
interface HTMLElementEventMap {
@@ -23,7 +45,78 @@
}
@customElement('gr-user-suggestion-fix')
-export class GrUserSuggetionFix extends LitElement {
+export class GrUserSuggestionsFix extends LitElement {
+ @state()
+ comment?: Comment;
+
+ @state()
+ commentedText?: string;
+
+ @state()
+ layers: DiffLayer[] = [];
+
+ @state()
+ previewLoaded = false;
+
+ @state()
+ changeNum?: NumericChangeId;
+
+ @state()
+ preview?: FilePreview;
+
+ @state()
+ diffPrefs?: DiffPreferencesInfo;
+
+ @state()
+ renderPrefs: RenderPreferences = {
+ disable_context_control_buttons: true,
+ show_file_comment_button: false,
+ hide_line_length_indicator: true,
+ };
+
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
+ private readonly restApiService = getAppContext().restApiService;
+
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ private readonly getCommentModel = resolve(this, commentModelToken);
+
+ private readonly flagsService = getAppContext().flagsService;
+
+ private readonly syntaxLayer = new GrSyntaxLayerWorker(
+ resolve(this, highlightServiceToken),
+ () => getAppContext().reportingService
+ );
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ changeNum => (this.changeNum = changeNum)
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().diffPreferences$,
+ diffPreferences => {
+ if (!diffPreferences) return;
+ this.diffPrefs = diffPreferences;
+ this.syntaxLayer.setEnabled(!!this.diffPrefs.syntax_highlighting);
+ }
+ );
+ subscribe(
+ this,
+ () => this.getCommentModel().comment$,
+ comment => (this.comment = comment)
+ );
+ subscribe(
+ this,
+ () => this.getCommentModel().commentedText$,
+ commentedText => (this.commentedText = commentedText)
+ );
+ }
+
static override get styles() {
return [
css`
@@ -63,6 +156,19 @@
];
}
+ override updated(changed: PropertyValues) {
+ if (changed.has('commentedText') || changed.has('comment')) {
+ if (
+ this.flagsService.isEnabled(
+ KnownExperimentId.DIFF_FOR_USER_SUGGESTED_EDIT
+ ) &&
+ !this.previewLoaded
+ ) {
+ this.fetchFixPreview();
+ }
+ }
+ }
+
override render() {
if (!this.textContent) return nothing;
const code = this.textContent;
@@ -95,17 +201,79 @@
</gr-button>
</div>
</div>
- <code>${code}</code>`;
+ ${when(
+ this.previewLoaded,
+ () => this.renderDiff(),
+ () => html`<code>${code}</code>`
+ )} `;
}
handleShowFix() {
if (!this.textContent) return;
fire(this, 'open-user-suggest-preview', {code: this.textContent});
}
+
+ private renderDiff() {
+ if (!this.preview) return;
+ const diff = this.preview.preview;
+ if (!anyLineTooLong(diff)) {
+ this.syntaxLayer.process(diff);
+ }
+ return html`<gr-diff
+ .prefs=${this.overridePartialDiffPrefs()}
+ .path=${this.preview.filepath}
+ .diff=${diff}
+ .layers=${this.layers}
+ .renderPrefs=${this.renderPrefs}
+ .viewMode=${DiffViewMode.UNIFIED}
+ ></gr-diff>`;
+ }
+
+ private async fetchFixPreview() {
+ if (
+ !this.changeNum ||
+ !this.comment?.patch_set ||
+ !this.textContent ||
+ !this.commentedText
+ )
+ return;
+
+ const fixSuggestions = createUserFixSuggestion(
+ this.comment,
+ this.commentedText,
+ this.textContent
+ );
+ const res = await this.restApiService.getFixPreview(
+ this.changeNum,
+ this.comment?.patch_set,
+ fixSuggestions[0].replacements
+ );
+ if (res) {
+ const currentPreviews = Object.keys(res).map(key => {
+ return {filepath: key, preview: res[key]};
+ });
+ if (currentPreviews.length > 0) {
+ this.preview = currentPreviews[0];
+ this.previewLoaded = true;
+ }
+ }
+
+ return res;
+ }
+
+ private overridePartialDiffPrefs() {
+ if (!this.diffPrefs) return undefined;
+ return {
+ ...this.diffPrefs,
+ context: 1,
+ line_length: Math.min(this.diffPrefs.line_length, 100),
+ line_wrapping: true,
+ };
+ }
}
declare global {
interface HTMLElementTagNameMap {
- 'gr-user-suggestion-fix': GrUserSuggetionFix;
+ 'gr-user-suggestion-fix': GrUserSuggestionsFix;
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
index 8b0a427..423fe62 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
@@ -6,18 +6,28 @@
import '../../../test/common-test-setup';
import './gr-user-suggestion-fix';
import {fixture, html, assert} from '@open-wc/testing';
-import {GrUserSuggetionFix} from './gr-user-suggestion-fix';
-import {getAppContext} from '../../../services/app-context';
+import {GrUserSuggestionsFix} from './gr-user-suggestion-fix';
+import {
+ CommentModel,
+ commentModelToken,
+} from '../gr-comment-model/gr-comment-model';
+import {wrapInProvider} from '../../../models/di-provider-element';
+import {createComment} from '../../../test/test-data-generators';
suite('gr-user-suggestion-fix tests', () => {
- let element: GrUserSuggetionFix;
+ let element: GrUserSuggestionsFix;
setup(async () => {
- const flagsService = getAppContext().flagsService;
- sinon.stub(flagsService, 'isEnabled').returns(true);
- element = await fixture<GrUserSuggetionFix>(html`
- <gr-user-suggestion-fix>Hello World</gr-user-suggestion-fix>
- `);
+ const commentModel = new CommentModel({comment: createComment()});
+ element = (
+ await fixture<GrUserSuggestionsFix>(
+ wrapInProvider(
+ html` <gr-user-suggestion-fix>Hello World</gr-user-suggestion-fix> `,
+ commentModelToken,
+ commentModel
+ )
+ )
+ ).querySelector<GrUserSuggestionsFix>('gr-user-suggestion-fix')!;
await element.updateComplete;
});
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 2104c4a..a8d4a3b 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -19,4 +19,5 @@
PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
ML_SUGGESTED_EDIT = 'UiFeature__ml_suggested_edit',
+ DIFF_FOR_USER_SUGGESTED_EDIT = 'UiFeature__diff_for_user_suggested_edit',
}
diff --git a/polygerrit-ui/app/utils/message-util.ts b/polygerrit-ui/app/utils/message-util.ts
index c70ccb1..fffe612 100644
--- a/polygerrit-ui/app/utils/message-util.ts
+++ b/polygerrit-ui/app/utils/message-util.ts
@@ -7,7 +7,8 @@
import {ChangeId, ChangeMessageInfo} from '../types/common';
function getRevertChangeIdFromMessage(msg: ChangeMessageInfo): ChangeId {
- const REVERT_REGEX = /^Created a revert of this change as (I[0-9a-f]{40})$/;
+ const REVERT_REGEX =
+ /^Created a revert of this change as .*?(I[0-9a-f]{40})$/;
const changeId = msg.message.match(REVERT_REGEX)?.[1];
if (!changeId) throw new Error('revert changeId not found');
return changeId as ChangeId;
diff --git a/polygerrit-ui/app/utils/message-util_test.ts b/polygerrit-ui/app/utils/message-util_test.ts
index ca82818..64d765a 100644
--- a/polygerrit-ui/app/utils/message-util_test.ts
+++ b/polygerrit-ui/app/utils/message-util_test.ts
@@ -29,4 +29,24 @@
'If02ca1cd494579d6bb92a157bf1819e3689cd6b1' as ChangeId,
]);
});
+
+ test('getRevertCreatedChangeIds with extra spam', () => {
+ const messages = [
+ {
+ ...createChangeMessage(),
+ message:
+ 'Created a revert of this change as IIf02ca1cd494579d6bb92a157bf1819e3689cd6b1',
+ tag: MessageTag.TAG_REVERT as ReviewInputTag,
+ },
+ {
+ ...createChangeMessage(),
+ message: 'Created a revert of this change as abc',
+ tag: undefined,
+ },
+ ];
+
+ assert.deepEqual(getRevertCreatedChangeIds(messages), [
+ 'If02ca1cd494579d6bb92a157bf1819e3689cd6b1' as ChangeId,
+ ]);
+ });
});
