PDPDEVTOOL-6272: Create UI to provide Feedback Loop for DevAssist #971

TheJorgeBorras · 2025-11-05T13:44:18Z

On Code Review:

New Webview Panel to Provide Feedback for the DevAssist
HTML pages and rendering for different scenarios
Basic HTML and Javascript logic (missing CSS).
Proper Client-side field validation
Proper error handling and error messages.

Missing from the code review

Review CSS files and Styling on HTML files.
Maintain State and info of Feedback Form when reloading page/swappiing/throwing error.

…op-UI

…op-UI # Conflicts: # packages/vscode-extension/messages.json # packages/vscode-extension/src/service/TranslationKeys.ts

carolnavarropereira · 2025-11-11T09:25:40Z

packages/vscode-extension/resources/media/FeedbackForm.html

+		<form class="content" id="feedbackForm">
+			<div class="row">
+				<label for="feedback">Your Feedback</label>
+				<textarea id="feedback" name="feedback"


Maybe you should add the maxlength property

I like this proposal.
If we showed the remaining characters available count on the textarea (such that we display 0 when maxLength is archieved) then maybe this would be a better experience (HTML wouldn't let you type further).

But this isn't the case and I think the current Alert is more User Friendly.
Let's take this to Kostya and see what he thinks.

went with maxLength anyways.

carolnavarropereira · 2025-11-11T14:33:34Z

packages/vscode-extension/src/service/TranslationKeys.ts

 		}
 	},
+	FEEDBACK_FORM: {
+		VALIDATION_ERROR: 'DEVASSIST_SERVICE_FEEDBACK_FORM_VALIDATION_ERROR',


maybe you can create a group for generic errors

I renamed the message to make it clearer. Let me know what you think

packages/vscode-extension/resources/media/FeedbackForm.html

carolnavarropereira · 2025-11-11T16:20:58Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+			// Send request to NetSuite Backend through Proxy
+			try {
+				const currentProxySettings = getDevAssistCurrentSettings();
+				const response = await fetch(`http://127.0.0.1:${currentProxySettings.localPort}/api/internal/devassist/feedback`, {


The path can be a constant value into the class file

Reused the ApplicatinConstants instead of writing URL directly.

This change is better for maintainability, but is less clear IMO. Let me know if it's okay

carolnavarropereira · 2025-11-11T16:21:50Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+					vsLogger.error("Feedback Form External Failure: " + response.status + ' ' + response.statusText);
+					vsLogger.error('');
+
+					if (response.status === 403) {


It can be a constant value, i.e. HTTP_FORBIDDEN = 403

Added HTTP_STATUS to Node Application constants and reused it here as well as in the SuiteCLoudAuthProxyService.js

carolnavarropereira · 2025-11-11T16:26:30Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+					}
+				}
+			} catch (e) {
+				vsLogger.printTimestamp();


I am not very fond of large lines of code inside an exception catch. Maybe (optional) can you put it inside a method so does not look like a flow control instead of an exception

Nesting within Nesting within nesting... If possible I'd like to refuse this change

carolnavarropereira · 2025-11-11T16:27:42Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+
+const handleWebviewMessage = async (webviewMessage : any, feedbackFormCSSFilePath : string) : Promise<void> => {
+	switch (webviewMessage.type) {
+		case WEBVIEW_EVENTS.SUBMIT_FEEDBACK:


I see too much code into sentences swich case that should be brief. Maybe you can wrap the lines of code for each case into functions.

I agree with Carol. I would extract logic for each case into a method to improve code readability.

The deed is done

Good for me. If David agrees we can close this comment

packages/vscode-extension/messages.json

packages/vscode-extension/src/service/TranslationKeys.ts

davidecorreu · 2025-11-12T17:26:10Z

packages/vscode-extension/src/startup/DevAssistConfiguration.ts

+	const infoMessage: string = translationService.getMessage(DEVASSIST_SERVICE.IS_RUNNING.NOTIFICATION, proxyUrl);
+	const buttonsAndActions: { buttonMessage: string, buttonAction: () => void }[] = [
+		{
+			buttonMessage: translationService.getMessage(BUTTONS.SEE_DETAILS),
+			buttonAction: vsNotificationService.showOutput
+		},
+		{
+			buttonMessage: translationService.getMessage(DEVASSIST_SERVICE.IS_RUNNING.NOTIFICATION_BUTTON),
+			buttonAction: () => {
+				vscode.commands.executeCommand('suitecloud.opendevassistfeedbackform')
+			}
+		},
+	];
+	vsNotificationService.showCommandInfoWithSpecificButtonsAndActions(infoMessage, buttonsAndActions);


Can we extract this logic to a method to improve code readability?
I have some suggestions:

showDevAssistIsRunningNotification

showDevAssistStartNotification

showStartServiceNotification

showStartDevAssistNotification

showDevAssistStartedNotification

I'm kind of asking to replicate what it is done on showDevAssistStartUpNotification.

davidecorreu · 2025-11-13T09:22:21Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+]
+
+let feedbackFormPanel: vscode.WebviewPanel | undefined;
+let vscodeExtensionMediaPath : string;


I would like to have a service that can get the html content instead of constructing the path each time. It will make the implementation cleaner.

Implemented a MediaFileService (naming could be improved).

It mostly does what we discussed, the only limitation being that CSS file paths must be relative to the VSCode.WebView. Therefore, the cssUri must be calculated on the WebViewController and passed to the MediaFileService.

packages/vscode-extension/src/suitecloud.ts

davidecorreu · 2025-11-13T09:29:35Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+
+	// Handle messages/events sent from the webview
+	feedbackFormPanel.webview.onDidReceiveMessage(
+		(webviewMessage) => handleWebviewMessage(webviewMessage, feedbackFormCSSFilePath),


I think I would prefer webviewEvent instead of webviewMessage

davidecorreu · 2025-11-13T09:31:22Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+	return true;
+}
+
+const handleWebviewMessage = async (webviewMessage : any, feedbackFormCSSFilePath : string) : Promise<void> => {


Lets try to shape the webviewMessage/webviewEvent with a proper type.

…op-UI

packages/vscode-extension/src/service/TranslationKeys.ts

carolnavarropereira · 2025-11-14T10:35:17Z

packages/vscode-extension/src/webviews/FeedbackFormWebview.ts

+
+const handleWebviewMessage = async (webviewMessage : any, feedbackFormCSSFilePath : string) : Promise<void> => {
+	switch (webviewMessage.type) {
+		case WEBVIEW_EVENTS.SUBMIT_FEEDBACK:


Good for me. If David agrees we can close this comment

TheJorgeBorras added 9 commits October 30, 2025 17:49

Base Skeleton

3b2797c

New HTML

6d6bf0d

More changes

09c7cb8

Initial Alert Implementation

49b1ae7

Messages

d841c26

Moved HTML

ccab222

More stuff

51efa53

Merge branch 'dev' into feature/PDPDEVTOOL-6272-DevAssist-Feedback-Lo…

7ba8347

…op-UI

Finalizing the Functional Side of things

4111bbb

TheJorgeBorras requested review from carolnavarropereira and davidecorreu November 5, 2025 13:44

TheJorgeBorras self-assigned this Nov 5, 2025

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 5, 2025

TheJorgeBorras added 14 commits November 5, 2025 15:19

Minor comment for code reviewers

4a38f98

ops

de75eba

Validation of Fields

886a7f7

minor stuff

996979a

Readability

f0d1d59

Messages Reviewed

9e8c073

removing bad code

2ad6675

reverting message

fddbcac

more stuff

88a5f08

minor error

2194f2f

Minor comments

3975808

Showing Error Reauth

1f3ce30

Unused imports removed

4e5c91e

Improving Alert messages

f9bf601

TheJorgeBorras marked this pull request as ready for review November 10, 2025 09:23

Merge branch 'dev' into feature/PDPDEVTOOL-6272-DevAssist-Feedback-Lo…

1d329ed

…op-UI # Conflicts: # packages/vscode-extension/messages.json # packages/vscode-extension/src/service/TranslationKeys.ts

carolnavarropereira reviewed Nov 11, 2025

View reviewed changes

Removing enablement OpenFeedbackForm command

1f3acb1

carolnavarropereira reviewed Nov 11, 2025

View reviewed changes

packages/vscode-extension/resources/media/FeedbackForm.html Outdated Show resolved Hide resolved

carolnavarropereira reviewed Nov 11, 2025

View reviewed changes

packages/vscode-extension/messages.json Outdated Show resolved Hide resolved

TheJorgeBorras added 4 commits November 12, 2025 14:25

Requested Code Changes

131a83e

Requested Code Changes 2

0523bfc

Messages Reviewed pt3

141908d

curly stuff

5e36900

davidecorreu reviewed Nov 12, 2025

View reviewed changes

packages/vscode-extension/src/service/TranslationKeys.ts Show resolved Hide resolved

davidecorreu reviewed Nov 12, 2025

View reviewed changes

davidecorreu reviewed Nov 13, 2025

View reviewed changes

packages/vscode-extension/src/suitecloud.ts Show resolved Hide resolved

davidecorreu reviewed Nov 13, 2025

View reviewed changes

TheJorgeBorras added 3 commits November 13, 2025 18:01

David Changes

2119a46

More improvements

569e8e9

More improvements 2

1ea9342

TheJorgeBorras requested review from carolnavarropereira and davidecorreu November 14, 2025 09:08

TheJorgeBorras added 3 commits November 14, 2025 11:00

maxlength

6189c59

Merge branch 'dev' into feature/PDPDEVTOOL-6272-DevAssist-Feedback-Lo…

48b9e9a

…op-UI

Updating TranslationKeys Buttons

14420f5

carolnavarropereira approved these changes Nov 14, 2025

View reviewed changes

TheJorgeBorras added 2 commits November 14, 2025 13:34

Code Clarity

0d4fd54

Naming

cda0d06

PDPDEVTOOL-6272: Create UI to provide Feedback Loop for DevAssist #971

Are you sure you want to change the base?

PDPDEVTOOL-6272: Create UI to provide Feedback Loop for DevAssist #971

Conversation

TheJorgeBorras commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheJorgeBorras Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TheJorgeBorras commented Nov 5, 2025 •

edited

Loading

TheJorgeBorras Nov 14, 2025 •

edited

Loading