Skip to content

Commit

Permalink
Fix deserialization error on Existing Connection Option click in webv…
Browse files Browse the repository at this point in the history
…iew (#4852)

* Fix desrialization error on Existing Connection  Option click in webview

* address feedback

* Added tests
  • Loading branch information
manodnyab authored Sep 5, 2024
1 parent b64a7c0 commit 375083e
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,12 @@ class QWebviewBrowser(val project: Project, private val parentDisposable: Dispos
}

is BrowserMessage.SendUiClickTelemetry -> {
UiTelemetry.click(project, message.signInOptionClicked)
val signInOption = message.signInOptionClicked
if (signInOption.isNullOrEmpty()) {
LOG.warn("Unknown sign in option")
} else {
UiTelemetry.click(project, signInOption)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ sealed interface BrowserMessage {

object Reauth : BrowserMessage

data class SendUiClickTelemetry(val signInOptionClicked: String) : BrowserMessage
data class SendUiClickTelemetry(val signInOptionClicked: String?) : BrowserMessage
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.assertj.core.api.Assertions.assertThatThrownBy
import org.assertj.core.api.ObjectAssert
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import org.junit.jupiter.api.extension.RegisterExtension
import org.mockito.kotlin.mock
import software.aws.toolkits.jetbrains.core.webview.BrowserMessage
Expand Down Expand Up @@ -148,6 +149,18 @@ class BrowserMessageTest {
}
"""
)

assertDeserializedInstanceOf<BrowserMessage.SendUiClickTelemetry>(
"""
{
"command": "sendUiClickTelemetry"
}
"""
).isEqualTo(
BrowserMessage.SendUiClickTelemetry(
signInOptionClicked = null
)
)
}

@Test
Expand Down Expand Up @@ -273,4 +286,29 @@ class BrowserMessageTest {
"""
)
}

@Test
fun `Nullable fields in sendUiClickTelemetry should not throw exception`() {
assertDoesNotThrow {
objectMapper.readValue<BrowserMessage>(
"""
{
"command": "sendUiClickTelemetry",
"signInOptionClicked": null
}
"""
)
}

assertDoesNotThrow {
objectMapper.readValue<BrowserMessage>(
"""
{
"command": "sendUiClickTelemetry"
}
"""
)
}
}
}
6 changes: 5 additions & 1 deletion plugins/core/webview/src/q-ui/components/login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,14 @@ const authUiClickOptionMap = {
[LoginIdentifier.ENTERPRISE_SSO]: 'auth_idcOption',
[LoginIdentifier.IAM_CREDENTIAL]: 'auth_credentialsOption',
[LoginIdentifier.EXISTING_LOGINS]: 'auth_existingAuthOption',
[LoginIdentifier.NONE]: "Unknown"
}
function getUiClickEvent(loginIdentifier: LoginIdentifier) {
return (authUiClickOptionMap as any)[loginIdentifier]
if(!Object.keys(authUiClickOptionMap).includes(loginIdentifier)) {
return "Unknown"
}
return (authUiClickOptionMap)[loginIdentifier]
}
export default defineComponent({
Expand Down
8 changes: 7 additions & 1 deletion plugins/core/webview/src/q-ui/components/qOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,13 @@ export default defineComponent({
this.selectedLoginOption = itemId
},
emitUiClickMetric(itemId: string) {
this.$emit('emitUiClickTelemetry', itemId)
const loginIdentifiers = Object.values(LoginIdentifier).map(value => value.toString());
if(loginIdentifiers.includes(itemId) ) {
this.$emit('emitUiClickTelemetry', itemId)
} else {
this.$emit('emitUiClickTelemetry', LoginIdentifier.EXISTING_LOGINS)
}
},
handleBackButtonClick() {
this.$emit('backToMenu')
Expand Down
7 changes: 6 additions & 1 deletion plugins/core/webview/src/q-ui/components/toolkitOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ export default defineComponent({
this.selectedLoginOption = itemId
},
emitUiClickMetric(itemId: string) {
this.$emit('emitUiClickTelemetry', itemId)
const loginIdentifiers = Object.values(LoginIdentifier).map(value => value.toString());
if(loginIdentifiers.includes(itemId) ) {
this.$emit('emitUiClickTelemetry', itemId)
} else {
this.$emit('emitUiClickTelemetry', LoginIdentifier.EXISTING_LOGINS)
}
},
handleBackButtonClick() {
this.$emit('backToMenu')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,12 @@ class ToolkitWebviewBrowser(val project: Project, private val parentDisposable:
}

is BrowserMessage.SendUiClickTelemetry -> {
UiTelemetry.click(project, message.signInOptionClicked)
val signInOption = message.signInOptionClicked
if (signInOption.isNullOrEmpty()) {
LOG.warn("Unknown sign in option")
} else {
UiTelemetry.click(project, signInOption)
}
}
}
}
Expand Down Expand Up @@ -336,6 +341,7 @@ class ToolkitWebviewBrowser(val project: Project, private val parentDisposable:
fun component(): JComponent? = jcefBrowser.component

companion object {
private val LOG = getLogger<ToolkitWebviewBrowser>()
private const val WEB_SCRIPT_URI = "http://webview/js/toolkitGetStart.js"
private const val DOMAIN = "webview"
}
Expand Down

0 comments on commit 375083e

Please sign in to comment.