Skip to content

Commit

Permalink
Fix part of #1633: Removed view dependency from ProfileEditViewModel (#…
Browse files Browse the repository at this point in the history
…1925)

* Removed view dependency from ProfileEditViewModel

- Used MutableLiveData to store the value of ProfileEditActivity Title
  and then expose it using a livedata variable in profleEditViewModel
  class
- Observe the livedata variable to set ProfileEditActivity Title in
  ProfileEditActivityPresenter class

* Resolved swith dependency from ProfileViewModel

- Added a livedata variable to expose the checked state of switch in
  ProfileViewModel through which ProfileEditActivityPresenter can
  observe and update the UI accordingly
- Removed switch variable initialization from viewmodel
- Added activity dependeny as it was before

* Added new lines as required

* Removed Switch & activity from ProfileViewModel

- Removed activity variable from ProfileViewModel, as we can simply use
  existing profile livedata variable to update the title of the
  ProfileEditActivity in the ProfileEditActivityPresenter
- Instead of using viewModelProvider in ProfileEditActivityPresenter to
  get viewmodel, now it is directly injected
- Since viewmodel can survive configuration changes, there is no need to
  create an extra variable to store profile name to use it in
  onRestoreInstanceState
- Added comments for public variable in ProfileEditViewModel

* Removed unused import

* Removed @ignore from activity title related testcases from ProfileEditActivityTest and modified description for the public variable in ProfielEditViewModel

* Resolved tests that are failing in roboelectric except that uses scrollTo() method

* FirebaseApp initializtion was giving an IO exception and since it is not used in activity, removed it from the test

* Removed unused imports

* added scrollview in port layout of profile_edit_activity to resolve roboelectric tests

* Removed unused imports

* Added feedback changes

- Removed unneccessary use of testCoroutineDispatchers.runCurrent()
- Renamed editViewModel to profileEditViewModel
- Few Indentation

* modified indentation

* Added PR requested changes

- Removed '// ktlint-disable max-line-length' by following suggested
  convention
- Renamed _isAllowedDownloadAccess to
  isAllowedDownloadAccessMutableLiveData

* Added feedback changes, renaming tests

* Used launch instead of ActivityScenario.launch

* Added named argument

* Removed 0dp margin and padding from scrollview and renamed id for linearlayout
  • Loading branch information
alokbharti authored Nov 24, 2020
1 parent ac29306 commit ff74da4
Show file tree
Hide file tree
Showing 5 changed files with 289 additions and 300 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ class ProfileEditActivity : InjectableAppCompatActivity() {
profileEditActivityPresenter.handleOnCreate()
}

override fun onRestoreInstanceState(savedInstanceState: Bundle?) {
super.onRestoreInstanceState(savedInstanceState)
profileEditActivityPresenter.handleOnRestoreSavedInstanceState()
}

override fun onSupportNavigateUp(): Boolean {
val isMultipane = intent.extras!!.getBoolean(KEY_IS_MULTIPANE, false)
if (isMultipane) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package org.oppia.android.app.settings.profile

import android.content.Intent
import android.widget.Switch
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.app.AppCompatActivity
import androidx.databinding.DataBindingUtil
import androidx.lifecycle.Observer
import org.oppia.android.R
import org.oppia.android.app.activity.ActivityScope
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.viewmodel.ViewModelProvider
import org.oppia.android.databinding.ProfileEditActivityBinding
import org.oppia.android.domain.profile.ProfileManagementController
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
Expand All @@ -21,12 +19,11 @@ import javax.inject.Inject
class ProfileEditActivityPresenter @Inject constructor(
private val activity: AppCompatActivity,
private val logger: ConsoleLogger,
private val profileManagementController: ProfileManagementController,
private val viewModelProvider: ViewModelProvider<ProfileEditViewModel>
private val profileManagementController: ProfileManagementController
) {
private val editViewModel: ProfileEditViewModel by lazy {
getProfileEditViewModel()
}

@Inject
lateinit var profileEditViewModel: ProfileEditViewModel

fun handleOnCreate() {
activity.supportActionBar?.setDisplayHomeAsUpEnabled(true)
Expand All @@ -37,12 +34,10 @@ class ProfileEditActivityPresenter @Inject constructor(
R.layout.profile_edit_activity
)
val profileId = activity.intent.getIntExtra(KEY_PROFILE_EDIT_PROFILE_ID, 0)
editViewModel.setProfileId(
profileId,
activity.findViewById<Switch>(R.id.profile_edit_allow_download_switch)
)
profileEditViewModel.setProfileId(profileId)

binding.apply {
viewModel = editViewModel
viewModel = profileEditViewModel
lifecycleOwner = activity
}

Expand All @@ -55,7 +50,7 @@ class ProfileEditActivityPresenter @Inject constructor(
ProfileResetPinActivity.createProfileResetPinActivity(
activity,
profileId,
editViewModel.isAdmin
profileEditViewModel.isAdmin
)
)
}
Expand All @@ -64,6 +59,20 @@ class ProfileEditActivityPresenter @Inject constructor(
showDeletionDialog(profileId)
}

profileEditViewModel.profile.observe(
activity,
Observer {
activity.title = it.name
}
)

profileEditViewModel.isAllowedDownloadAccess.observe(
activity,
Observer {
binding.profileEditAllowDownloadSwitch.isChecked = it
}
)

binding.profileEditAllowDownloadSwitch.setOnCheckedChangeListener { compoundButton, checked ->
if (compoundButton.isPressed) {
profileManagementController.updateAllowDownloadAccess(
Expand All @@ -85,10 +94,6 @@ class ProfileEditActivityPresenter @Inject constructor(
}
}

fun handleOnRestoreSavedInstanceState() {
activity.title = editViewModel.profileName
}

private fun showDeletionDialog(profileId: Int) {
AlertDialog.Builder(activity, R.style.AlertDialogTheme)
.setTitle(R.string.profile_edit_delete_dialog_title)
Expand All @@ -111,8 +116,4 @@ class ProfileEditActivityPresenter @Inject constructor(
)
}.create().show()
}

private fun getProfileEditViewModel(): ProfileEditViewModel {
return viewModelProvider.getForActivity(activity, ProfileEditViewModel::class.java)
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package org.oppia.android.app.settings.profile

import android.widget.Switch
import androidx.appcompat.app.AppCompatActivity
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.Transformations
import org.oppia.android.app.activity.ActivityScope
import org.oppia.android.app.model.Profile
Expand All @@ -14,18 +13,18 @@ import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import org.oppia.android.util.logging.ConsoleLogger
import javax.inject.Inject

// TODO(#1633): Fix ViewModel to not depend on View
/** The ViewModel for [ProfileEditActivity]. */
@ActivityScope
class ProfileEditViewModel @Inject constructor(
private val activity: AppCompatActivity,
private val logger: ConsoleLogger,
private val profileManagementController: ProfileManagementController
) : ObservableViewModel() {
private lateinit var profileId: ProfileId
private lateinit var switch: Switch

lateinit var profileName: String
private val isAllowedDownloadAccessMutableLiveData = MutableLiveData<Boolean>()

/** Specifies whether download access has been enabled by the user. */
val isAllowedDownloadAccess: LiveData<Boolean> = isAllowedDownloadAccessMutableLiveData

val profile: LiveData<Profile> by lazy {
Transformations.map(
Expand All @@ -36,9 +35,8 @@ class ProfileEditViewModel @Inject constructor(

var isAdmin = false

fun setProfileId(id: Int, switch: Switch) {
fun setProfileId(id: Int) {
profileId = ProfileId.newBuilder().setInternalId(id).build()
this.switch = switch
}

private fun processGetProfileResult(profileResult: AsyncResult<Profile>): Profile {
Expand All @@ -50,9 +48,7 @@ class ProfileEditViewModel @Inject constructor(
)
}
val profile = profileResult.getOrDefault(Profile.getDefaultInstance())
switch.isChecked = profile.allowDownloadAccess
activity.title = profile.name
profileName = profile.name
isAllowedDownloadAccessMutableLiveData.value = profile.allowDownloadAccess
isAdmin = profile.isAdmin
return profile
}
Expand Down
Loading

0 comments on commit ff74da4

Please sign in to comment.