Skip to content

Commit a03167c

Browse files
siakmun-awsrli
authored and
karanA-aws
committed
Fix rejected files display inconsistency between VS Code and JetBrains (aws#5169)
* Fix rejected files display inconsistency between VS Code and JetBrains * Refactor * Refactor * Refactor * Clean * Clean * Changed from `when` to whenever * Fixing detekt * Fixing detekt * Fixing detekt --------- Co-authored-by: Richard Li <[email protected]>
1 parent ec529a9 commit a03167c

File tree

4 files changed

+66
-33
lines changed

4 files changed

+66
-33
lines changed

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/controller/FeatureDevController.kt‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,6 @@ class FeatureDevController(
311311
filePaths = filePaths.filter { it.zipFilePath == fileToUpdate },
312312
deletedFiles = deletedFiles.filter { it.zipFilePath == fileToUpdate },
313313
references = references, // Add all references (not attributed per-file)
314-
messenger
315314
)
316315

317316
AmazonqTelemetry.isAcceptedCodeChanges(
@@ -326,8 +325,6 @@ class FeatureDevController(
326325
deletedFiles.find { it.zipFilePath == fileToUpdate }?.let { it.rejected = !it.rejected }
327326
}
328327

329-
// FIXME: This is a kludge that is hiding the fact that insertChanges is updating the file tree above this point to
330-
// an incorrect state. Update the state of the tree view:
331328
messenger.updateFileComponent(message.tabId, filePaths, deletedFiles, messageId)
332329

333330
// Then, if the accepted file is not a deletion, open a diff to show the changes are applied:
@@ -434,14 +431,14 @@ class FeatureDevController(
434431
credentialStartUrl = getStartUrl(project = context.project)
435432
)
436433

437-
// Caution: insertChanges has multiple responsibilities.
438-
// The filter here results in rejected files being hidden from the tree after continuing, by design.
439-
// However, it is critical that we don't hide already-accepted files. Inside insertChanges, it
440-
// filters to only update the subset of passed files that aren't accepted or rejected already.
441434
session.insertChanges(
442-
filePaths = filePaths.filter { !it.rejected },
443-
deletedFiles = deletedFiles.filter { !it.rejected },
444-
references = references,
435+
filePaths = filePaths,
436+
deletedFiles = deletedFiles,
437+
references = references
438+
)
439+
session.updateFilesPaths(
440+
filePaths = filePaths,
441+
deletedFiles = deletedFiles,
445442
messenger
446443
)
447444

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/session/Session.kt‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,29 @@ class Session(val tabID: String, val project: Project) {
108108
this._codeResultMessageId = messageId
109109
}
110110

111+
suspend fun updateFilesPaths(
112+
filePaths: List<NewFileZipInfo>,
113+
deletedFiles: List<DeletedFileInfo>,
114+
messenger: MessagePublisher,
115+
disableFileActions: Boolean = false,
116+
) {
117+
val codeResultMessageId = this._codeResultMessageId
118+
if (codeResultMessageId != null) {
119+
messenger.updateFileComponent(this.tabID, filePaths, deletedFiles, codeResultMessageId, disableFileActions)
120+
}
121+
}
122+
111123
/**
112124
* Triggered by the Insert code follow-up button to apply code changes.
113125
*/
114126
suspend fun insertChanges(
115127
filePaths: List<NewFileZipInfo>,
116128
deletedFiles: List<DeletedFileInfo>,
117129
references: List<CodeReferenceGenerated>,
118-
messenger: MessagePublisher,
119130
) {
120-
val selectedSourceFolder = context.selectedSourceFolder.toNioPath()
121131
val newFilePaths = filePaths.filter { !it.rejected && !it.changeApplied }
122132
val newDeletedFiles = deletedFiles.filter { !it.rejected && !it.changeApplied }
133+
val selectedSourceFolder = context.selectedSourceFolder.toNioPath()
123134

124135
runCatching {
125136
var insertedLines = 0
@@ -156,23 +167,39 @@ class Session(val tabID: String, val project: Project) {
156167
}
157168
}.onFailure { /* Noop on diff telemetry failure */ }
158169

159-
newFilePaths.forEach {
160-
resolveAndCreateOrUpdateFile(selectedSourceFolder, it.zipFilePath, it.fileContent)
161-
it.changeApplied = true
162-
}
170+
insertNewFiles(newFilePaths)
163171

164-
newDeletedFiles.forEach {
165-
resolveAndDeleteFile(selectedSourceFolder, it.zipFilePath)
166-
it.changeApplied = true
167-
}
172+
applyDeleteFiles(newDeletedFiles)
168173

169174
ReferenceLogController.addReferenceLog(references, project)
170175

171176
// Taken from https://intellij-support.jetbrains.com/hc/en-us/community/posts/206118439-Refresh-after-external-changes-to-project-structure-and-sources
172177
VfsUtil.markDirtyAndRefresh(true, true, true, context.selectedSourceFolder)
173-
val codeResultMessageId = this._codeResultMessageId
174-
if (codeResultMessageId != null) {
175-
messenger.updateFileComponent(this.tabID, filePaths, deletedFiles, codeResultMessageId)
178+
}
179+
180+
// Suppressing because insertNewFiles needs to be a suspend function in order to be tested
181+
@Suppress("RedundantSuspendModifier")
182+
suspend fun insertNewFiles(
183+
filePaths: List<NewFileZipInfo>,
184+
) {
185+
val selectedSourceFolder = context.selectedSourceFolder.toNioPath()
186+
187+
filePaths.forEach {
188+
resolveAndCreateOrUpdateFile(selectedSourceFolder, it.zipFilePath, it.fileContent)
189+
it.changeApplied = true
190+
}
191+
}
192+
193+
// Suppressing because applyDeleteFiles needs to be a suspend function in order to be tested
194+
@Suppress("RedundantSuspendModifier")
195+
suspend fun applyDeleteFiles(
196+
deletedFiles: List<DeletedFileInfo>,
197+
) {
198+
val selectedSourceFolder = context.selectedSourceFolder.toNioPath()
199+
200+
deletedFiles.forEach {
201+
resolveAndDeleteFile(selectedSourceFolder, it.zipFilePath)
202+
it.changeApplied = true
176203
}
177204
}
178205

@@ -185,10 +212,7 @@ class Session(val tabID: String, val project: Project) {
185212
return
186213
}
187214

188-
val codeResultMessageId = this._codeResultMessageId
189-
if (codeResultMessageId != null) {
190-
messenger.updateFileComponent(this.tabID, filePaths, deletedFiles, codeResultMessageId, disableFileActions = true)
191-
}
215+
updateFilesPaths(filePaths, deletedFiles, messenger, disableFileActions = true)
192216
this._codeResultMessageId = null
193217
}
194218

‎plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/controller/FeatureDevControllerTest.kt‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,22 +241,34 @@ class FeatureDevControllerTest : FeatureDevTestBase() {
241241
),
242242
)
243243

244-
doReturn(Unit).`when`(spySession).insertChanges(any(), any(), any(), any())
244+
doReturn(Unit).whenever(spySession).insertChanges(any(), any(), any())
245+
doReturn(Unit).whenever(spySession).insertNewFiles(any())
246+
doReturn(Unit).whenever(spySession).applyDeleteFiles(any())
245247

246248
spySession.preloader(messenger)
247249
controller.processFollowupClickedMessage(message)
248250

249251
mockitoVerify(
250252
spySession,
251253
times(1),
252-
).insertChanges(listOf(newFileContents[0]), listOf(deletedFiles[0]), testReferences, messenger) // insert changes for only non rejected files
254+
).insertChanges(newFileContents, deletedFiles, testReferences) // updates for all files
253255
coVerifyOrder {
254256
AmazonqTelemetry.isAcceptedCodeChanges(
255257
amazonqNumberOfFilesAccepted = 2.0, // it should be 2 files per test setup
256258
amazonqConversationId = spySession.conversationId,
257259
enabled = true,
258260
createTime = any(),
259261
)
262+
263+
// insert changes for only non rejected files
264+
spySession.insertNewFiles(listOf(newFileContents[0]))
265+
spySession.applyDeleteFiles(listOf(deletedFiles[0]))
266+
267+
spySession.updateFilesPaths(
268+
filePaths = newFileContents,
269+
deletedFiles = deletedFiles,
270+
messenger
271+
)
260272
messenger.sendAnswer(
261273
tabId = testTabId,
262274
message = message("amazonqFeatureDev.code_generation.updated_code"),
@@ -469,7 +481,7 @@ class FeatureDevControllerTest : FeatureDevTestBase() {
469481
),
470482
)
471483
doReturn(testConversationId).`when`(spySession).conversationId
472-
doReturn(Unit).`when`(spySession).insertChanges(any(), any(), any(), any())
484+
doReturn(Unit).`when`(spySession).insertChanges(any(), any(), any())
473485

474486
mockkObject(AmazonqTelemetry)
475487
every {
@@ -492,7 +504,7 @@ class FeatureDevControllerTest : FeatureDevTestBase() {
492504
mockitoVerify(
493505
spySession,
494506
times(1),
495-
).insertChanges(listOf(newFileContents[0]), listOf(), testReferences, messenger)
507+
).insertChanges(listOf(newFileContents[0]), listOf(), testReferences)
496508

497509
// Does not continue automatically, because files are remaining:
498510
mockitoVerify(
@@ -526,7 +538,7 @@ class FeatureDevControllerTest : FeatureDevTestBase() {
526538
),
527539
)
528540
doReturn(testConversationId).`when`(spySession).conversationId
529-
doReturn(Unit).`when`(spySession).insertChanges(any(), any(), any(), any())
541+
doReturn(Unit).`when`(spySession).insertChanges(any(), any(), any())
530542

531543
mockkObject(AmazonqTelemetry)
532544
every {

‎plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/session/SessionTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class SessionTest : FeatureDevTestBase() {
8383
whenever(session.context.selectedSourceFolder.toNioPath()).thenReturn(Path(""))
8484

8585
runBlocking {
86-
session.insertChanges(mockNewFile, mockDeletedFile, emptyList(), messenger)
86+
session.insertChanges(mockNewFile, mockDeletedFile, emptyList())
8787
}
8888

8989
verify(exactly = 1) { resolveAndDeleteFile(any(), "deletedTest.ts") }

0 commit comments

Comments
 (0)