Refactor gr-suggestion-diff-preview
Simplify gr-suggestion-diff-preview, reduce code duplication.
Simply by calculating fixSuggestionInfo in gr-user-suggestion-fix.
Google-Bug-Id: b/360288262
Release-Notes: skip
Change-Id: Ic3e1fed2e0548797bb769f7345eaaeacc506efa2
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 7a3b7ec..ea1c20d 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1177,6 +1177,7 @@
if (this.generatedFixSuggestion) {
return html`<gr-suggestion-diff-preview
id="suggestionDiffPreview"
+ .uuid=${this.generatedSuggestionId}
.fixSuggestionInfo=${this.generatedFixSuggestion}
></gr-suggestion-diff-preview>`;
} else {
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 d03bd54..8ff0292 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
@@ -280,7 +280,7 @@
if (!this.comment?.fix_suggestions) return;
this.applyingFix = true;
try {
- await this.suggestionDiffPreview?.applyFixSuggestion();
+ await this.suggestionDiffPreview?.applyFix();
} finally {
this.applyingFix = false;
}
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 6fb7d533..5dbf9b7 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
@@ -30,7 +30,6 @@
import {subscribe} from '../../lit/subscription-controller';
import {DiffPreview} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
import {userModelToken} from '../../../models/user/user-model';
-import {createUserFixSuggestion} from '../../../utils/comment-util';
import {commentModelToken} from '../gr-comment-model/gr-comment-model';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {fire} from '../../../utils/event-util';
@@ -48,15 +47,18 @@
*/
@customElement('gr-suggestion-diff-preview')
export class GrSuggestionDiffPreview extends LitElement {
+ // Optional. Used as backup when preview is not loaded.
@property({type: String})
- suggestion?: string;
+ codeText?: string;
+ // Required.
@property({type: Object})
fixSuggestionInfo?: FixSuggestionInfo;
@property({type: Boolean, attribute: 'previewed', reflect: true})
previewed = false;
+ // Optional. Used in logging.
@property({type: String})
uuid?: string;
@@ -64,13 +66,16 @@
comment?: Comment;
@state()
- commentedText?: string;
-
- @state()
layers: DiffLayer[] = [];
+ /**
+ * The fix suggestion info that the preview is loaded for.
+ *
+ * This is used to determine if the preview has been loaded for the same
+ * fix suggestion info currently in gr-comment.
+ */
@state()
- previewLoadedFor?: string | FixSuggestionInfo;
+ public previewLoadedFor?: string | FixSuggestionInfo;
@state() repo?: RepoName;
@@ -147,11 +152,6 @@
);
subscribe(
this,
- () => this.getCommentModel().commentedText$,
- commentedText => (this.commentedText = commentedText)
- );
- subscribe(
- this,
() => this.getChangeModel().repo$,
x => (this.repo = x)
);
@@ -192,27 +192,24 @@
}
override updated(changed: PropertyValues) {
- if (changed.has('commentedText') || changed.has('comment')) {
- if (this.previewLoadedFor !== this.suggestion) {
- this.fetchFixPreview();
- }
- }
-
- if (changed.has('changeNum') || changed.has('comment')) {
+ if (
+ changed.has('fixSuggestionInfo') ||
+ changed.has('changeNum') ||
+ changed.has('comment')
+ ) {
if (this.previewLoadedFor !== this.fixSuggestionInfo) {
- this.fetchfixSuggestionInfoPreview();
+ this.fetchFixPreview();
}
}
}
override render() {
- if (!this.suggestion && !this.fixSuggestionInfo) return nothing;
- const code = this.suggestion;
+ if (!this.fixSuggestionInfo) return nothing;
return html`
${when(
this.previewLoadedFor,
() => this.renderDiff(),
- () => html`<code>${code}</code>`
+ () => html`<code>${this.codeText}</code>`
)}
`;
}
@@ -236,58 +233,15 @@
}
private async fetchFixPreview() {
- if (
- !this.changeNum ||
- !this.comment?.patch_set ||
- !this.suggestion ||
- !this.commentedText
- )
- return;
- const fixSuggestions = createUserFixSuggestion(
- this.comment,
- this.commentedText,
- this.suggestion
- );
- this.reporting.time(Timing.PREVIEW_FIX_LOAD);
- const res = await this.restApiService.getFixPreview(
- this.changeNum,
- this.comment?.patch_set,
- fixSuggestions[0].replacements
- );
- if (!res) return;
- const currentPreviews = Object.keys(res).map(key => {
- return {filepath: key, preview: res[key]};
- });
- this.reporting.timeEnd(Timing.PREVIEW_FIX_LOAD, {
- uuid: this.uuid,
- commentId: this.comment?.id ?? '',
- });
- if (currentPreviews.length > 0) {
- this.preview = currentPreviews[0];
- this.previewLoadedFor = this.suggestion;
- this.previewed = true;
- }
-
- return res;
- }
-
- private async fetchfixSuggestionInfoPreview() {
- if (
- this.suggestion ||
- !this.changeNum ||
- !this.comment?.patch_set ||
- !this.fixSuggestionInfo
- )
+ if (!this.changeNum || !this.comment?.patch_set || !this.fixSuggestionInfo)
return;
- this.previewed = false;
this.reporting.time(Timing.PREVIEW_FIX_LOAD);
const res = await this.restApiService.getFixPreview(
this.changeNum,
this.comment?.patch_set,
this.fixSuggestionInfo.replacements
);
-
if (!res) return;
const currentPreviews = Object.keys(res).map(key => {
return {filepath: key, preview: res[key]};
@@ -298,26 +252,12 @@
});
if (currentPreviews.length > 0) {
this.preview = currentPreviews[0];
- this.previewed = true;
this.previewLoadedFor = this.fixSuggestionInfo;
+ this.previewed = true;
}
return res;
}
-
- /**
- * Applies a fix (fix_suggestion in comment) previewed in
- * `suggestion-diff-preview`, navigating to the new change URL with the EDIT
- * patchset.
- *
- * Similar code flow is in gr-apply-fix-dialog.handleApplyFix
- * Used in gr-fix-suggestions
- */
- public applyFixSuggestion() {
- if (this.suggestion || !this.fixSuggestionInfo) return;
- return this.applyFix(this.fixSuggestionInfo);
- }
-
/**
* Applies a fix (codeblock in comment message) previewed in
* `suggestion-diff-preview`, navigating to the new change URL with the EDIT
@@ -326,20 +266,11 @@
* Similar code flow is in gr-apply-fix-dialog.handleApplyFix
* Used in gr-user-suggestion-fix
*/
- public applyUserSuggestedFix() {
- if (!this.comment || !this.suggestion || !this.commentedText) return;
- const fixSuggestions = createUserFixSuggestion(
- this.comment,
- this.commentedText,
- this.suggestion
- );
- this.applyFix(fixSuggestions[0]);
- }
-
- private async applyFix(fixSuggestion: FixSuggestionInfo) {
+ public async applyFix() {
const changeNum = this.changeNum;
const basePatchNum = this.comment?.patch_set as BasePatchSetNum;
+ const fixSuggestion = this.fixSuggestionInfo;
if (!changeNum || !basePatchNum || !fixSuggestion) return;
this.reporting.time(Timing.APPLY_FIX_LOAD);
diff --git a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview_test.ts b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview_test.ts
index 2630aad..39d937d 100644
--- a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview_test.ts
@@ -11,7 +11,10 @@
commentModelToken,
} from '../gr-comment-model/gr-comment-model';
import {wrapInProvider} from '../../../models/di-provider-element';
-import {createComment} from '../../../test/test-data-generators';
+import {
+ createComment,
+ createFixSuggestionInfo,
+} from '../../../test/test-data-generators';
import {getAppContext} from '../../../services/app-context';
import {GrSuggestionDiffPreview} from './gr-suggestion-diff-preview';
import {stubFlags} from '../../../test/test-utils';
@@ -29,7 +32,8 @@
wrapInProvider(
html`
<gr-suggestion-diff-preview
- .suggestion=${'Hello World'}
+ .codeText=${'Hello World'}
+ .fixSuggestionInfo=${createFixSuggestionInfo()}
></gr-suggestion-diff-preview>
`,
commentModelToken,
@@ -48,7 +52,7 @@
test('render diff', async () => {
stubFlags('isEnabled').returns(true);
- element.suggestion =
+ element.codeText =
' private handleClick(e: MouseEvent) {\ne.stopPropagation();\ne.preventDefault();';
element.previewLoadedFor =
' private handleClick(e: MouseEvent) {\ne.stopPropagation();\ne.preventDefault();';
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 14e7134..fef2252 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
@@ -19,6 +19,7 @@
import {Comment, PatchSetNumber} from '../../../types/common';
import {commentModelToken} from '../gr-comment-model/gr-comment-model';
import {waitUntil} from '../../../utils/async-util';
+import {createUserFixSuggestion} from '../../../utils/comment-util';
declare global {
interface HTMLElementEventMap {
@@ -47,6 +48,8 @@
@state() private previewLoaded = false;
+ @state() commentedText?: string;
+
private readonly getConfigModel = resolve(this, configModelToken);
private readonly getChangeModel = resolve(this, changeModelToken);
@@ -70,6 +73,11 @@
() => this.getCommentModel().comment$,
comment => (this.comment = comment)
);
+ subscribe(
+ this,
+ () => this.getCommentModel().commentedText$,
+ commentedText => (this.commentedText = commentedText)
+ );
}
static override get styles() {
@@ -95,8 +103,14 @@
}
override render() {
- if (!this.textContent) return nothing;
+ if (!this.textContent || !this.comment || !this.commentedText)
+ return nothing;
const code = this.textContent;
+ const fixSuggestions = createUserFixSuggestion(
+ this.comment,
+ this.commentedText,
+ code
+ );
return html`<div class="header">
<div class="title">
<span>Suggested edit</span>
@@ -139,7 +153,8 @@
</div>
</div>
<gr-suggestion-diff-preview
- .suggestion=${this.textContent}
+ .fixSuggestions=${fixSuggestions[0]}
+ .codeText=${code}
></gr-suggestion-diff-preview>`;
}
@@ -151,7 +166,7 @@
async handleApplyFix() {
if (!this.textContent) return;
this.applyingFix = true;
- await this.suggestionDiffPreview?.applyUserSuggestedFix();
+ await this.suggestionDiffPreview?.applyFix();
this.applyingFix = false;
}
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 8baee43..56d826e 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
@@ -22,6 +22,7 @@
const commentModel = new CommentModel(getAppContext().restApiService);
commentModel.updateState({
comment: createComment(),
+ commentedText: 'Hello World',
});
element = (
await fixture<GrUserSuggestionsFix>(
