Remove the use of intent provider for request vpn permission

[Pururun]

Continuation from: #6592

Also removes intent holder (but still uses intents) for api overrides that comes for intents.

---

This change is 

[linear]

DROID-1110 Remove the use of IntentProvider for vpn permission requests

[Rawa]

Reviewed all commit messages.
Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @Pururun)

---

-- commits line 2 at r1:
I'm a bit on the fence about this, feels like we are going the wrong way about this solution.

If I understand correctly this is the case where a user click on a notification because they lack the permission of VPN? Wouldn't it be more natural to utilize deeplinks for this? Show a page saying helping the user to setup a VPN profile. Then I believe we should be able to avoid doing any dance with passing intents from the activity to the view.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt line 52 at r1 (raw file):

            mullvadAppViewModel.connect()
        }
    val activity = LocalContext.current.getActivity() as ComponentActivity

Feels a bit weird that we have to grab the activity, and cast it. Would be nice if we can avoid this.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt line 53 at r1 (raw file):

        }
    val activity = LocalContext.current.getActivity() as ComponentActivity
    LaunchedEffect(navHostController) {

What is the reasoning behind using navHostController as key for the launch effect?

[Pururun]

Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @Rawa)

---

-- commits line 2 at r1:

Previously, Rawa (David Göransson) wrote…

I'm a bit on the fence about this, feels like we are going the wrong way about this solution.

If I understand correctly this is the case where a user click on a notification because they lack the permission of VPN? Wouldn't it be more natural to utilize deeplinks for this? Show a page saying helping the user to setup a VPN profile. Then I believe we should be able to avoid doing any dance with passing intents from the activity to the view.

I avoided using a deep link because it was not clear where it would deep link to. I guess you could deep link to a dummy screen that then navigates to the correct screen or I guess a dedicated set up vpn permission screen.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt line 52 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Feels a bit weird that we have to grab the activity, and cast it. Would be nice if we can avoid this.

You think we should move the cast somewhere else?

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt line 53 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

What is the reasoning behind using navHostController as key for the launch effect?

Probably a remnant from a previous solution, will remove.

[Pururun]

Setting this as on hold since we have other vpn permission work ongoing.

[Pururun]

Reviewable status: 0 of 16 files reviewed, 3 unresolved discussions (waiting on @Rawa)

---

-- commits line 2 at r1:

Previously, Pururun (Jonatan Rhodin) wrote…

I avoided using a deep link because it was not clear where it would deep link to. I guess you could deep link to a dummy screen that then navigates to the correct screen or I guess a dedicated set up vpn permission screen.

Updated code based on our discussion.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt line 52 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

You think we should move the cast somewhere else?

No longer relevant.

[Rawa]

Reviewed 1 of 6 files at r1, 13 of 15 files at r2, all commit messages.
Reviewable status: 14 of 16 files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt line 134 at r2 (raw file):

    private fun setUpIntentListener() {
        lifecycleScope.launch {
            intents().collect {

Small remark regarding clarity.

I think we should do it in the following way:

onCreate() {
    ...
    lifecycleScope.launch{
        intents().collect(::handleIntent)
    }
    ...
}

private fun handleIntent(intent: Intent) {
    when(val action = intent.action) {
        KEY_REQUEST_VPN_PROFILE -> requestVpnProfile()
        KEY_SET_API_OVERRIDE -> apiEndpointOverrideHolder.set(intent.data.parse()) // or something like this
        else -> Log.w("Unhandled intent action: $action")
    }
}

---

android/lib/endpoint/src/debug/kotlin/net/mullvad/mullvadvpn/lib/endpoint/ApiEndpointFromIntentHolder.kt line 10 at r2 (raw file):

    fun handleIntent(intent: Intent) {
        apiEndpointOverride = intent.getApiEndpointConfigurationExtras()

Would be nice if this one isn't aware of intent.

[Pururun]

Reviewable status: 11 of 16 files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt line 134 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Small remark regarding clarity.

I think we should do it in the following way:

onCreate() {
    ...
    lifecycleScope.launch{
        intents().collect(::handleIntent)
    }
    ...
}

private fun handleIntent(intent: Intent) {
    when(val action = intent.action) {
        KEY_REQUEST_VPN_PROFILE -> requestVpnProfile()
        KEY_SET_API_OVERRIDE -> apiEndpointOverrideHolder.set(intent.data.parse()) // or something like this
        else -> Log.w("Unhandled intent action: $action")
    }
}

Done.

---

android/lib/endpoint/src/debug/kotlin/net/mullvad/mullvadvpn/lib/endpoint/ApiEndpointFromIntentHolder.kt line 10 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Would be nice if this one isn't aware of intent.

Done.

[Rawa]

Reviewed 1 of 15 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt line 134 at r3 (raw file):

    private fun handleIntent(intent: Intent) {
        if (intent.action == KEY_REQUEST_VPN_PROFILE) {

Don't forget when statement, with warning logging if unsupported action.

[Rawa]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt line 134 at r3 (raw file):

Previously, Rawa (David Göransson) wrote…

Don't forget when statement, with warning logging if unsupported action.

when(val action = intent.action) {
        KEY_REQUEST_VPN_PROFILE -> requestVpnProfile()
        KEY_SET_API_OVERRIDE -> apiEndpointOverrideHolder.set(intent.data.parse()) // or something like this
        else -> Log.w("Unhandled intent action: $action")
    }```

[Pururun]

Reviewable status: 13 of 16 files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt line 134 at r3 (raw file):

Previously, Rawa (David Göransson) wrote…

when(val action = intent.action) {
        KEY_REQUEST_VPN_PROFILE -> requestVpnProfile()
        KEY_SET_API_OVERRIDE -> apiEndpointOverrideHolder.set(intent.data.parse()) // or something like this
        else -> Log.w("Unhandled intent action: $action")
    }```

Ah sorry missed that part. Done

[Rawa]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved