Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
|
You can test the changes on this Pull Request by downloading the APK here. |
zwarm
left a comment
There was a problem hiding this comment.
@malinajirka - Looks good 🎈 - all works as expected, external browser launched correctly. I left one question/comment on hardcoding, but it won't hold up the merge. Just a thought for the future.
| data class ShowUpdatedFixState(val threatId: Long) : ThreatDetailsNavigationEvents() | ||
|
|
||
| object ShowGetFreeEstimate : ThreatDetailsNavigationEvents() { | ||
| const val url = "https://codeable.io/partners/jetpack-scan/" |
There was a problem hiding this comment.
❓ - Do we have an alternative for hardcoding or is this sufficient for the MVP? Post MVP 🤔 perhaps an endpoint similar to jetpack capabilities, but for upsells?
There was a problem hiding this comment.
I asked the same question yesterday on Slack - tbh I'm not sure. I don't like hardcoding urls - especially 3rd party. The 3rd party can change the url and we'll never be able to remove it from this version of the app.
I was thinking if we could for example point to our url and redirect from there. Downsides of this approach is that some browsers might complain and suppress the redirect to a different domain.
There was a problem hiding this comment.
I'll merge this PR and update the code if we make a different decision on Slack.
Parent issue #13326
This PR implements "get free estimate" action which is shown on ThreatDetail screen for non-fixable threats.
To test:
Prerequisites:
PR submission checklist:
RELEASE-NOTES.txtif necessary.