fix: reorder input checks, remove modals, add jobId to table (#4184)

dhasani23 · David Hasani · web-flow · commit 8ecf0571e48c · 2023-12-18T14:34:05.000-08:00
* fix: reorder input checks, remove modals, add jobId to table

* fix: fix unit test

* fix: fix unit test

* fix: address comments

* fix: omit redundant metric emission

* nit: modify error message

---------

Co-authored-by: David Hasani &lt;davhasan@amazon.com&gt;
diff --git a/src/codewhisperer/commands/startTransformByQ.ts b/src/codewhisperer/commands/startTransformByQ.ts
@@ -256,7 +256,7 @@ export async function startTransformByQ() {
     } catch (error) {
         if (transformByQState.isCancelled()) {
             stopJob(transformByQState.getJobId())
-            vscode.window.showErrorMessage(CodeWhispererConstants.transformByQCancelledMessage, { modal: true })
+            vscode.window.showErrorMessage(CodeWhispererConstants.transformByQCancelledMessage)
         } else {
             transformByQState.setToFailed()
             let displayedErrorMessage = CodeWhispererConstants.transformByQFailedMessage
@@ -266,7 +266,7 @@ export async function startTransformByQ() {
             if (transformByQState.getJobFailureReason() !== '') {
                 displayedErrorMessage += `: ${transformByQState.getJobFailureReason()}`
             }
-            vscode.window.showErrorMessage(displayedErrorMessage, { modal: true })
+            vscode.window.showErrorMessage(displayedErrorMessage)
         }
         if (sessionPlanProgress['uploadCode'] !== StepProgress.Succeeded) {
             sessionPlanProgress['uploadCode'] = StepProgress.Failed
@@ -295,14 +295,11 @@ export async function startTransformByQ() {
             )
         }
         if (transformByQState.isSucceeded()) {
-            vscode.window.showInformationMessage(CodeWhispererConstants.transformByQCompletedMessage, { modal: true })
+            vscode.window.showInformationMessage(CodeWhispererConstants.transformByQCompletedMessage)
         } else if (transformByQState.isPartiallySucceeded()) {
-            vscode.window.showInformationMessage(CodeWhispererConstants.transformByQPartiallyCompletedMessage, {
-                modal: true,
-            })
+            vscode.window.showInformationMessage(CodeWhispererConstants.transformByQPartiallyCompletedMessage)
         }
     }
-    await sleep(2000) // needed as a buffer to allow TransformationHub to update before state is updated
     clearInterval(intervalId)
     transformByQState.setToNotStarted() // so that the "Transform by Q" button resets
     transformByQState.setPolledJobStatus('') // reset polled job status too
diff --git a/src/codewhisperer/models/constants.ts b/src/codewhisperer/models/constants.ts
@@ -323,7 +323,7 @@ export const dependencyDisclaimer =
 export const dependencyFolderName = 'transformation_dependencies_temp_'
 
 export const dependencyErrorMessage =
-    'Failed to execute Maven. It is possible that the upload does not include all dependencies.'
+    'Failed to execute Maven. It is possible that the upload does not include all dependencies. We will still attempt to complete the transformation.'
 
 export const planIntroductionMessage =
     'We reviewed your Java JAVA_VERSION_HERE application and generated a transformation plan. Any code changes made to your application will be done in the sandbox so as to not interfere with your working repository. Once the transformation job is done, we will share the new code which you can review before acccepting the code changes. In the meantime, you can work on your codebase and invoke Q Chat to answer questions about your codebase.'
diff --git a/src/codewhisperer/models/model.ts b/src/codewhisperer/models/model.ts
@@ -241,6 +241,12 @@ export enum JDKVersion {
     JDK17 = '17',
 }
 
+export enum BuildSystem {
+    Maven = 'Maven',
+    Gradle = 'Gradle',
+    Unknown = 'Unknown',
+}
+
 export enum DropdownStep {
     STEP_1 = 1,
     STEP_2 = 2,
diff --git a/src/codewhisperer/service/transformByQHandler.ts b/src/codewhisperer/service/transformByQHandler.ts
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { transformByQState, TransformByQStoppedError, ZipManifest } from '../models/model'
+import { BuildSystem, transformByQState, TransformByQStoppedError, ZipManifest } from '../models/model'
 import * as codeWhisperer from '../client/codewhisperer'
 import * as crypto from 'crypto'
 import { getLogger } from '../../shared/logger'
@@ -64,7 +64,7 @@ export function throwIfCancelled() {
 export async function getOpenProjects() {
     const folders = vscode.workspace.workspaceFolders
     if (folders === undefined) {
-        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
+        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage)
         throw new ToolkitError('No Java projects found since no projects are open', { code: 'NoOpenProjects' })
     }
     const openProjects: vscode.QuickPickItem[] = []
@@ -84,13 +84,24 @@ export async function getOpenProjects() {
  */
 export async function validateProjectSelection(project: vscode.QuickPickItem) {
     const projectPath = project.description
+    const buildSystem = await checkBuildSystem(projectPath!)
+    if (buildSystem !== BuildSystem.Maven) {
+        vscode.window.showErrorMessage(CodeWhispererConstants.noPomXmlFoundMessage)
+        telemetry.codeTransform_isDoubleClickedToTriggerInvalidProject.emit({
+            codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
+            codeTransformPreValidationError: 'NonMavenProject',
+            result: MetadataResult.Fail,
+            reason: buildSystem === BuildSystem.Gradle ? buildSystem : 'NoPomFileFound',
+        })
+        throw new ToolkitError('No valid Maven build file found', { code: 'CouldNotFindPomXml' })
+    }
     const compiledJavaFiles = await vscode.workspace.findFiles(
         new vscode.RelativePattern(projectPath!, '**/*.class'),
         '**/node_modules/**',
         1
     )
     if (compiledJavaFiles.length < 1) {
-        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
+        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage)
         telemetry.codeTransform_isDoubleClickedToTriggerInvalidProject.emit({
             codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
             codeTransformPreValidationError: 'NoJavaProject',
@@ -105,7 +116,7 @@ export async function validateProjectSelection(project: vscode.QuickPickItem) {
     const spawnResult = spawnSync(baseCommand, args, { shell: false, encoding: 'utf-8' })
 
     if (spawnResult.error || spawnResult.status !== 0) {
-        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
+        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage)
         telemetry.codeTransform_isDoubleClickedToTriggerInvalidProject.emit({
             codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
             codeTransformPreValidationError: 'NoJavaProject',
@@ -124,7 +135,7 @@ export async function validateProjectSelection(project: vscode.QuickPickItem) {
     } else if (javaVersion === CodeWhispererConstants.JDK11VersionNumber) {
         transformByQState.setSourceJDKVersionToJDK11()
     } else {
-        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
+        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage)
         telemetry.codeTransform_isDoubleClickedToTriggerInvalidProject.emit({
             codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
             codeTransformPreValidationError: 'UnsupportedJavaVersion',
@@ -133,30 +144,6 @@ export async function validateProjectSelection(project: vscode.QuickPickItem) {
         })
         throw new ToolkitError('Project selected is not Java 8 or Java 11', { code: 'UnsupportedJavaVersion' })
     }
-    const buildFile = await vscode.workspace.findFiles(
-        new vscode.RelativePattern(projectPath!, 'pom.xml'), // check for pom.xml in root directory only
-        '**/node_modules/**',
-        1
-    )
-    if (buildFile.length < 1) {
-        const buildType = await checkIfGradle(projectPath!)
-        vscode.window.showErrorMessage(CodeWhispererConstants.noPomXmlFoundMessage, { modal: true })
-        if (buildType === 'Gradle') {
-            telemetry.codeTransform_isDoubleClickedToTriggerInvalidProject.emit({
-                codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
-                codeTransformPreValidationError: 'NonMavenProject',
-                result: MetadataResult.Fail,
-                reason: buildType,
-            })
-        }
-        telemetry.codeTransform_isDoubleClickedToTriggerInvalidProject.emit({
-            codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
-            codeTransformPreValidationError: 'NoPom',
-            result: MetadataResult.Fail,
-            reason: 'CouldNotFindPomXml',
-        })
-        throw new ToolkitError('No valid Maven build file found', { code: 'CouldNotFindPomXml' })
-    }
 }
 
 export function getSha256(fileName: string) {
@@ -188,7 +175,6 @@ export async function uploadArtifactToS3(fileName: string, resp: CreateUploadUrl
     const sha256 = getSha256(fileName)
     const headersObj = getHeadersObj(sha256, resp.kmsKeyArn)
 
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
     try {
         const apiStartTime = Date.now()
@@ -255,7 +241,6 @@ export async function stopJob(jobId: string) {
 
 export async function uploadPayload(payloadFileName: string) {
     const sha256 = getSha256(payloadFileName)
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
     try {
         const apiStartTime = Date.now()
@@ -290,14 +275,25 @@ export async function uploadPayload(payloadFileName: string) {
     }
 }
 
-function getFilesRecursively(dir: string): string[] {
+/**
+ * Gets all files in dir. We use this method to get the source code, then we run a mvn command to
+ * copy over dependencies into their own folder, then we use this method again to get those
+ * dependencies. If isDependenciesFolder is true, then we are getting all the files
+ * of the dependencies which were copied over by the previously-run mvn command, in which case
+ * we DO want to include any dependencies that may happen to be named "target", hence the check
+ * in the first part of the IF statement. The point of excluding folders named target is that
+ * "target" is also the name of the folder where .class files, large JARs, etc. are stored after
+ * building, and we do not want these included in the ZIP so we exclude these when calling
+ * getFilesRecursively on the source code folder.
+ */
+function getFilesRecursively(dir: string, isDependenciesFolder: boolean): string[] {
     const entries = fs.readdirSync(dir, { withFileTypes: true })
     const files = entries.flatMap(entry => {
         const res = path.resolve(dir, entry.name)
-        // exclude 'target' directory from ZIP due to issues in backend
+        // exclude 'target' directory from ZIP (except if zipping dependencies) due to issues in backend
         if (entry.isDirectory()) {
-            if (entry.name !== 'target') {
-                return getFilesRecursively(res)
+            if (isDependenciesFolder || entry.name !== 'target') {
+                return getFilesRecursively(res, isDependenciesFolder)
             } else {
                 return []
             }
@@ -324,7 +320,7 @@ function getProjectDependencies(modulePath: string): string[] {
     const spawnResult = spawnSync(baseCommand, args, { cwd: modulePath, shell: false, encoding: 'utf-8' })
 
     if (spawnResult.error || spawnResult.status !== 0) {
-        vscode.window.showErrorMessage(CodeWhispererConstants.dependencyErrorMessage, { modal: true })
+        vscode.window.showErrorMessage(CodeWhispererConstants.dependencyErrorMessage)
         getLogger().error('CodeTransform: Error in running Maven command = ')
         // Maven command can still go through and still return an error. Won't be caught in spawnResult.error in this case
         if (spawnResult.error) {
@@ -339,13 +335,11 @@ function getProjectDependencies(modulePath: string): string[] {
 }
 
 export async function zipCode(modulePath: string) {
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
     const zipStartTime = Date.now()
     const sourceFolder = modulePath
-    const sourceFiles = getFilesRecursively(sourceFolder)
+    const sourceFiles = getFilesRecursively(sourceFolder, false)
 
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
 
     let dependencyFolderInfo: string[] = []
@@ -359,7 +353,6 @@ export async function zipCode(modulePath: string) {
     const dependencyFolderPath = !mavenFailed ? dependencyFolderInfo[0] : ''
     const dependencyFolderName = !mavenFailed ? dependencyFolderInfo[1] : ''
 
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
 
     const zip = new AdmZip()
@@ -371,12 +364,11 @@ export async function zipCode(modulePath: string) {
         zip.addLocalFile(file, path.dirname(paddedPath))
     }
 
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
 
     let dependencyFiles: string[] = []
     if (!mavenFailed && fs.existsSync(dependencyFolderPath)) {
-        dependencyFiles = getFilesRecursively(dependencyFolderPath)
+        dependencyFiles = getFilesRecursively(dependencyFolderPath, true)
     }
 
     if (!mavenFailed && dependencyFiles.length > 0) {
@@ -395,7 +387,6 @@ export async function zipCode(modulePath: string) {
     }
     zip.addFile('manifest.json', Buffer.from(JSON.stringify(zipManifest), 'utf-8'))
 
-    await sleep(2000) // pause to give time to recognize potential cancellation
     throwIfCancelled()
 
     const tempFilePath = path.join(os.tmpdir(), 'zipped-code.zip')
@@ -605,11 +596,14 @@ export async function pollTransformationJob(jobId: string, validStates: string[]
     return status
 }
 
-async function checkIfGradle(projectPath: string) {
+export async function checkBuildSystem(projectPath: string) {
+    const mavenBuildFilePath = path.join(projectPath, 'pom.xml')
+    if (fs.existsSync(mavenBuildFilePath)) {
+        return BuildSystem.Maven
+    }
     const gradleBuildFilePath = path.join(projectPath, 'build.gradle')
     if (fs.existsSync(gradleBuildFilePath)) {
-        return 'Gradle'
-    } else {
-        return 'Unknown'
+        return BuildSystem.Gradle
     }
+    return BuildSystem.Unknown
 }
diff --git a/src/codewhisperer/service/transformationHubViewProvider.ts b/src/codewhisperer/service/transformationHubViewProvider.ts
@@ -85,6 +85,7 @@ export class TransformationHubViewProvider implements vscode.WebviewViewProvider
                         <th>Module</th>
                         <th>Status</th>
                         <th>Duration</th>
+                        <th>Id</th>
                     </tr>
                 </thead>
                 <tbody>
@@ -94,6 +95,7 @@ export class TransformationHubViewProvider implements vscode.WebviewViewProvider
                         <td>${job.module}</td>
                         <td>${job.status}</td>
                         <td>${job.duration}</td>
+                        <td>${job.id}</td>
                     </tr>`
                     )}
                 </tbody>
diff --git a/src/codewhisperer/service/transformationResultsViewProvider.ts b/src/codewhisperer/service/transformationResultsViewProvider.ts
@@ -278,6 +278,7 @@ export class ProposedTransformationExplorer {
                 )
             } catch (e: any) {
                 // This allows the customer to retry the download
+                vscode.window.showErrorMessage('Transform by Q experienced an error when downloading the diff')
                 vscode.commands.executeCommand('setContext', 'gumby.reviewState', TransformByQReviewStatus.NotStarted)
                 const errorMessage = 'There was a problem fetching the transformed code.'
                 getLogger().error('CodeTransform: ExportResultArchive error = ', errorMessage)
@@ -313,8 +314,11 @@ export class ProposedTransformationExplorer {
                     path.join(pathContainingArchive, ExportResultArchiveStructure.PathToSummary)
                 )
             } catch (e: any) {
-                deserializeErrorMessage = e?.message || 'Error during deserialization of result archive'
+                deserializeErrorMessage =
+                    e?.message ||
+                    'Transform by Q experienced an error during the deserialization of the downloaded result archive'
                 getLogger().error('CodeTransform: ParseDiff error = ', deserializeErrorMessage)
+                vscode.window.showErrorMessage(deserializeErrorMessage)
             } finally {
                 telemetry.codeTransform_jobArtifactDownloadAndDeserializeTime.emit({
                     codeTransformSessionId: codeTransformTelemetryState.getSessionId(),
@@ -328,8 +332,7 @@ export class ProposedTransformationExplorer {
             }
 
             await vscode.window.showInformationMessage(
-                'Transformation job completed. You can view the transformation summary along with the proposed changes and accept or reject them in the Proposed Changes panel.',
-                { modal: true }
+                'Transformation job completed. You can view the transformation summary along with the proposed changes and accept or reject them in the Proposed Changes panel.'
             )
             await vscode.commands.executeCommand('aws.amazonq.transformationHub.summary.reveal')
             telemetry.codeTransform_vcsDiffViewerVisible.emit({
diff --git a/src/test/codewhisperer/commands/transformByQ.test.ts b/src/test/codewhisperer/commands/transformByQ.test.ts

Original file line number	Diff line number	Diff line change
`@@ -256,7 +256,7 @@ export async function startTransformByQ() {`
`256`	`256`	`} catch (error) {`
`257`	`257`	`if (transformByQState.isCancelled()) {`
`258`	`258`	`stopJob(transformByQState.getJobId())`
`259`		`- vscode.window.showErrorMessage(CodeWhispererConstants.transformByQCancelledMessage, { modal: true })`
	`259`	`+ vscode.window.showErrorMessage(CodeWhispererConstants.transformByQCancelledMessage)`
`260`	`260`	`} else {`
`261`	`261`	`transformByQState.setToFailed()`
`262`	`262`	`let displayedErrorMessage = CodeWhispererConstants.transformByQFailedMessage`
`@@ -266,7 +266,7 @@ export async function startTransformByQ() {`
`266`	`266`	`if (transformByQState.getJobFailureReason() !== '') {`
`267`	`267`	displayedErrorMessage += `: ${transformByQState.getJobFailureReason()}`
`268`	`268`	`}`
`269`		`- vscode.window.showErrorMessage(displayedErrorMessage, { modal: true })`
	`269`	`+ vscode.window.showErrorMessage(displayedErrorMessage)`
`270`	`270`	`}`
`271`	`271`	`if (sessionPlanProgress['uploadCode'] !== StepProgress.Succeeded) {`
`272`	`272`	`sessionPlanProgress['uploadCode'] = StepProgress.Failed`
`@@ -295,14 +295,11 @@ export async function startTransformByQ() {`
`295`	`295`	`)`
`296`	`296`	`}`
`297`	`297`	`if (transformByQState.isSucceeded()) {`
`298`		`- vscode.window.showInformationMessage(CodeWhispererConstants.transformByQCompletedMessage, { modal: true })`
	`298`	`+ vscode.window.showInformationMessage(CodeWhispererConstants.transformByQCompletedMessage)`
`299`	`299`	`} else if (transformByQState.isPartiallySucceeded()) {`
`300`		`- vscode.window.showInformationMessage(CodeWhispererConstants.transformByQPartiallyCompletedMessage, {`
`301`		`- modal: true,`
`302`		`- })`
	`300`	`+ vscode.window.showInformationMessage(CodeWhispererConstants.transformByQPartiallyCompletedMessage)`
`303`	`301`	`}`
`304`	`302`	`}`
`305`		`- await sleep(2000) // needed as a buffer to allow TransformationHub to update before state is updated`
`306`	`303`	`clearInterval(intervalId)`
`307`	`304`	`transformByQState.setToNotStarted() // so that the "Transform by Q" button resets`
`308`	`305`	`transformByQState.setPolledJobStatus('') // reset polled job status too`