Merge "Move ApplyPatchUtil.java to a different package."
diff --git a/java/com/google/gerrit/extensions/registration/DynamicSet.java b/java/com/google/gerrit/extensions/registration/DynamicSet.java
index 9925a66..0c2691a 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicSet.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicSet.java
@@ -122,7 +122,7 @@
*/
public static <T> LinkedBindingBuilder<T> bind(Binder binder, Class<T> type, Named name) {
binder.disableCircularProxies();
- return bind(binder, TypeLiteral.get(type));
+ return bind(binder, TypeLiteral.get(type), name);
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 4814733..29d2bf0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -127,6 +127,7 @@
'line-selected': CustomEvent<LineSelectedEventDetail>;
// Fired if being logged in is required.
'show-auth-required': CustomEvent<{}>;
+ 'reload-diff': CustomEvent<{path: string | undefined}>;
}
}
@@ -343,6 +344,11 @@
this.addEventListener('diff-context-expanded', event =>
this.handleDiffContextExpanded(event)
);
+ this.addEventListener('reload-diff', (e: CustomEvent) => {
+ if (e.detail.path === this.path) {
+ this.reload(false);
+ }
+ });
subscribe(
this,
() => this.getBrowserModel().diffViewMode$,
diff --git a/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts b/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
index ca7fe9c..36266a0 100644
--- a/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
+++ b/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
@@ -7,7 +7,7 @@
import '../../shared/gr-icon/gr-icon';
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
import '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
-import {css, html, LitElement} from 'lit';
+import {css, html, LitElement, PropertyValues} from 'lit';
import {customElement, state, query, property} from 'lit/decorators.js';
import {fire} from '../../../utils/event-util';
import {getDocUrl} from '../../../utils/url-util';
@@ -16,7 +16,7 @@
import {configModelToken} from '../../../models/config/config-model';
import {GrSuggestionDiffPreview} from '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
import {changeModelToken} from '../../../models/change/change-model';
-import {Comment, PatchSetNumber} from '../../../types/common';
+import {Comment, NumericChangeId, PatchSetNumber} from '../../../types/common';
import {OpenFixPreviewEventDetail} from '../../../types/events';
import {pluginLoaderToken} from '../gr-js-api-interface/gr-plugin-loader';
import {SuggestionsProvider} from '../../../api/suggestions';
@@ -25,6 +25,7 @@
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
import {getAppContext} from '../../../services/app-context';
import {Interaction} from '../../../constants/reporting';
+import {isFileUnchanged} from '../../../utils/diff-util';
export const COLLAPSE_SUGGESTION_STORAGE_KEY = 'collapseSuggestionStorageKey';
@@ -47,11 +48,15 @@
@state() latestPatchNum?: PatchSetNumber;
+ @state() changeNum?: NumericChangeId;
+
@state()
suggestionsProvider?: SuggestionsProvider;
@state() private isOwner = false;
+ @state() private enableApplyOnUnModifiedFile = false;
+
/**
* This is just a reflected property such that css rules can be based on it.
*/
@@ -68,6 +73,8 @@
private readonly reporting = getAppContext().reportingService;
+ private readonly restApiService = getAppContext().restApiService;
+
constructor() {
super();
subscribe(
@@ -85,6 +92,17 @@
() => this.getChangeModel().isOwner$,
x => (this.isOwner = x)
);
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ x => (this.changeNum = x)
+ );
+ }
+
+ override updated(changed: PropertyValues) {
+ if (changed.has('changeNum') || changed.has('latestPatchNum')) {
+ this.checkIfcanEnableApplyOnUnModifiedFile();
+ }
}
override connectedCallback() {
@@ -277,7 +295,9 @@
if (!this.comment?.fix_suggestions) return;
this.applyingFix = true;
try {
- await this.suggestionDiffPreview?.applyFixSuggestion();
+ await this.suggestionDiffPreview?.applyFixSuggestion(
+ this.enableApplyOnUnModifiedFile
+ );
} finally {
this.applyingFix = false;
}
@@ -285,6 +305,7 @@
private isApplyEditDisabled() {
if (this.comment?.patch_set === undefined) return true;
+ if (this.enableApplyOnUnModifiedFile) return false;
return this.comment.patch_set !== this.latestPatchNum;
}
@@ -294,6 +315,29 @@
? 'You cannot apply this fix because it is from a previous patchset'
: '';
}
+
+ private async checkIfcanEnableApplyOnUnModifiedFile() {
+ // if enabled we don't need to enable
+ if (!this.isApplyEditDisabled()) return;
+
+ const basePatchNum = this.comment?.patch_set;
+ const path = this.comment?.path;
+
+ if (!basePatchNum || !this.latestPatchNum || !path || !this.changeNum) {
+ return;
+ }
+
+ const diff = await this.restApiService.getDiff(
+ this.changeNum,
+ basePatchNum,
+ this.latestPatchNum,
+ path
+ );
+
+ if (diff && isFileUnchanged(diff)) {
+ this.enableApplyOnUnModifiedFile = true;
+ }
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index d183211..6e76aaa 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -305,11 +305,14 @@
private convertCodeToSuggestions() {
const marks = this.renderRoot.querySelectorAll('mark');
- for (const userSuggestionMark of marks) {
+ marks.forEach((userSuggestionMark, index) => {
const userSuggestion = document.createElement('gr-user-suggestion-fix');
// Temporary workaround for bug - tabs replacement
if (this.content.includes('\t')) {
- userSuggestion.textContent = getUserSuggestionFromString(this.content);
+ userSuggestion.textContent = getUserSuggestionFromString(
+ this.content,
+ index
+ );
} else {
userSuggestion.textContent = userSuggestionMark.textContent ?? '';
}
@@ -317,7 +320,7 @@
userSuggestion,
userSuggestionMark
);
- }
+ });
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
index 87c81c0..d059c7c 100644
--- a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
+++ b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
@@ -7,7 +7,13 @@
import {css, html, LitElement, nothing, PropertyValues} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
import {getAppContext} from '../../../services/app-context';
-import {Comment, EDIT, BasePatchSetNum, RepoName} from '../../../types/common';
+import {
+ Comment,
+ EDIT,
+ BasePatchSetNum,
+ PatchSetNumber,
+ RepoName,
+} from '../../../types/common';
import {anyLineTooLong} from '../../../utils/diff-util';
import {
DiffLayer,
@@ -79,6 +85,8 @@
@state()
diffPrefs?: DiffPreferencesInfo;
+ @state() latestPatchNum?: PatchSetNumber;
+
@state()
renderPrefs: RenderPreferences = {
disable_context_control_buttons: true,
@@ -120,6 +128,11 @@
);
subscribe(
this,
+ () => this.getChangeModel().latestPatchNum$,
+ x => (this.latestPatchNum = x)
+ );
+ subscribe(
+ this,
() => this.getUserModel().diffPreferences$,
diffPreferences => {
if (!diffPreferences) return;
@@ -300,9 +313,9 @@
* Similar code flow is in gr-apply-fix-dialog.handleApplyFix
* Used in gr-fix-suggestions
*/
- public applyFixSuggestion() {
+ public applyFixSuggestion(onLatestPatchset = false) {
if (this.suggestion || !this.fixSuggestionInfo) return;
- this.applyFix(this.fixSuggestionInfo);
+ return this.applyFix(this.fixSuggestionInfo, onLatestPatchset);
}
/**
@@ -324,7 +337,10 @@
this.applyFix(fixSuggestions[0]);
}
- private async applyFix(fixSuggestion: FixSuggestionInfo) {
+ private async applyFix(
+ fixSuggestion: FixSuggestionInfo,
+ onLatestPatchset = false
+ ) {
const changeNum = this.changeNum;
const basePatchNum = this.comment?.patch_set as BasePatchSetNum;
if (!changeNum || !basePatchNum || !fixSuggestion) return;
@@ -332,7 +348,7 @@
this.reporting.time(Timing.APPLY_FIX_LOAD);
const res = await this.restApiService.applyFixSuggestion(
changeNum,
- basePatchNum,
+ onLatestPatchset ? this.latestPatchNum ?? basePatchNum : basePatchNum,
fixSuggestion.replacements
);
this.reporting.timeEnd(Timing.APPLY_FIX_LOAD, {
@@ -353,6 +369,7 @@
forceReload: !this.hasEdit,
})
);
+ fire(this, 'reload-diff', {path: this.comment?.path});
fire(this, 'apply-user-suggestion', {});
}
}
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 85a7be4..5eb9cac 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -147,7 +147,9 @@
if (replacements.length === 0) return undefined;
return {
- description: fix.description || `Fix provided by ${checkName}`,
+ description: [fix.description, `Fix provided by ${checkName}`]
+ .filter(Boolean)
+ .join(' - '),
fix_id: PROVIDED_FIX_ID,
replacements,
};
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index c8e5173..1324061 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -556,12 +556,17 @@
return comment.message?.includes(USER_SUGGESTION_START_PATTERN) ?? false;
}
-export function getUserSuggestionFromString(content: string) {
- const start =
- content.indexOf(USER_SUGGESTION_START_PATTERN) +
- USER_SUGGESTION_START_PATTERN.length;
- const end = content.indexOf('\n```', start);
- return content.substring(start, end);
+export function getUserSuggestionFromString(
+ content: string,
+ suggestionIndex = 0
+) {
+ const suggestions = content.split(USER_SUGGESTION_START_PATTERN).slice(1);
+ if (suggestions.length === 0) return '';
+
+ const targetIndex = Math.min(suggestionIndex, suggestions.length - 1);
+ const targetSuggestion = suggestions[targetIndex];
+ const end = targetSuggestion.indexOf('\n```');
+ return end !== -1 ? targetSuggestion.substring(0, end) : targetSuggestion;
}
export function getUserSuggestion(comment: Comment) {
diff --git a/polygerrit-ui/app/utils/comment-util_test.ts b/polygerrit-ui/app/utils/comment-util_test.ts
index 16c8bfd..031a738 100644
--- a/polygerrit-ui/app/utils/comment-util_test.ts
+++ b/polygerrit-ui/app/utils/comment-util_test.ts
@@ -18,6 +18,7 @@
getMentionedThreads,
isNewThread,
createNew,
+ getUserSuggestionFromString,
} from './comment-util';
import {
createAccountWithEmail,
@@ -533,4 +534,69 @@
]);
});
});
+
+ suite('getUserSuggestionFromString', () => {
+ const createSuggestionContent = (suggestions: string[]) =>
+ suggestions
+ .map(s => `${USER_SUGGESTION_START_PATTERN}${s}\n\`\`\``)
+ .join('\n');
+
+ test('returns empty string for content without suggestions', () => {
+ const content = 'This is a comment without any suggestions.';
+ assert.equal(getUserSuggestionFromString(content), '');
+ });
+
+ test('returns first suggestion when no index is provided', () => {
+ const content = createSuggestionContent(['First suggestion']);
+ assert.equal(getUserSuggestionFromString(content), 'First suggestion');
+ });
+
+ test('returns correct suggestion for given index', () => {
+ const content = createSuggestionContent([
+ 'First suggestion',
+ 'Second suggestion',
+ 'Third suggestion',
+ ]);
+ assert.equal(
+ getUserSuggestionFromString(content, 1),
+ 'Second suggestion'
+ );
+ });
+
+ test('returns last suggestion when index is out of bounds', () => {
+ const content = createSuggestionContent([
+ 'First suggestion',
+ 'Second suggestion',
+ ]);
+ assert.equal(
+ getUserSuggestionFromString(content, 5),
+ 'Second suggestion'
+ );
+ });
+
+ test('handles suggestion without closing backticks', () => {
+ const content = `${USER_SUGGESTION_START_PATTERN}Unclosed suggestion`;
+ assert.equal(getUserSuggestionFromString(content), 'Unclosed suggestion');
+ });
+
+ test('handles multiple suggestions with varying content', () => {
+ const content = createSuggestionContent([
+ 'First\nMultiline\nSuggestion',
+ 'Second suggestion',
+ 'Third suggestion with `backticks`',
+ ]);
+ assert.equal(
+ getUserSuggestionFromString(content, 0),
+ 'First\nMultiline\nSuggestion'
+ );
+ assert.equal(
+ getUserSuggestionFromString(content, 1),
+ 'Second suggestion'
+ );
+ assert.equal(
+ getUserSuggestionFromString(content, 2),
+ 'Third suggestion with `backticks`'
+ );
+ });
+ });
});
