Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #4294: After deleting a profile deleted successfully message should be displayed #4887

Closed
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
c8b91a4
added snackbar
Akshatkamboj14 Mar 2, 2023
8257029
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Mar 2, 2023
7e57949
merged into develop
Akshatkamboj14 Mar 2, 2023
38f0f6e
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Mar 10, 2023
748d8a1
changed the timing to short
Akshatkamboj14 Mar 11, 2023
f631b95
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Mar 11, 2023
6ea61fb
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Apr 4, 2023
a5a9a56
added comments
Akshatkamboj14 Apr 4, 2023
44e9e3b
added comments
Akshatkamboj14 Apr 4, 2023
80fbab2
added comments
Akshatkamboj14 Apr 4, 2023
84ca7c6
added comments
Akshatkamboj14 Apr 4, 2023
9c687fa
Merge branch 'develop' into After-deleting-a-profile-Deleted-successf…
adhiamboperes Apr 19, 2023
824fc2d
added snackbar
Akshatkamboj14 Apr 25, 2023
0eabafe
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Apr 25, 2023
85f8dbc
added snackbar
Akshatkamboj14 Apr 25, 2023
257e961
added snackbar
Akshatkamboj14 Apr 25, 2023
8960942
added snackbar
Akshatkamboj14 Apr 25, 2023
958b911
Merge branch 'After-deleting-a-profile-Deleted-successfully-message-s…
Akshatkamboj14 Apr 26, 2023
31faaef
added snackbar-request-provider-id
Akshatkamboj14 May 3, 2023
a3a3c19
rearranged-imports
Akshatkamboj14 May 3, 2023
46b1418
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 May 3, 2023
e5f39e9
rearranged-imports
Akshatkamboj14 May 3, 2023
39740b8
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 May 14, 2023
84da79d
added test for the snackbar controller
Akshatkamboj14 May 29, 2023
2db6631
corrected import statements
Akshatkamboj14 May 29, 2023
aa06e79
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jul 4, 2023
dbe3cc4
merged-into-develop
Akshatkamboj14 Jul 10, 2023
455edf4
added two tests
Akshatkamboj14 Jul 11, 2023
b7f1542
added the main functionality tests and some few changes
Akshatkamboj14 Jul 12, 2023
2f1c932
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jul 12, 2023
4d8d714
removed unused import
Akshatkamboj14 Jul 12, 2023
c92ddfd
added Kdoc for the sealed class
Akshatkamboj14 Jul 13, 2023
f2f51be
corrected tests and kdocs
Akshatkamboj14 Jul 14, 2023
e02a1e1
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jul 18, 2023
a40b2f8
corrected issues
Akshatkamboj14 Jul 18, 2023
1501043
added last
Akshatkamboj14 Jul 30, 2023
47baaae
added snackbarManagerTest
Akshatkamboj14 Aug 3, 2023
6f89a7d
added new line at the end
Akshatkamboj14 Aug 3, 2023
9aaff1e
merged into develop
Akshatkamboj14 Aug 3, 2023
f26d0e4
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Aug 3, 2023
319dd8a
test-commit
Akshatkamboj14 Aug 3, 2023
4176ab7
corrected k-doc
Akshatkamboj14 Aug 3, 2023
821c760
removed coroutine disp
Akshatkamboj14 Aug 3, 2023
a226cc1
restored
Akshatkamboj14 Aug 3, 2023
7abb519
corrected tests
Akshatkamboj14 Aug 8, 2023
5e2d339
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Aug 8, 2023
8252511
started changes
Akshatkamboj14 Oct 31, 2023
3cc76fb
merged-with-develop
Akshatkamboj14 Oct 31, 2023
34f45b1
few changes
Akshatkamboj14 Nov 10, 2023
64f48ba
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Nov 28, 2023
8430f81
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Nov 29, 2023
fc47617
done some changes acc to gist-1
Akshatkamboj14 Nov 29, 2023
3e7d3ce
done some changes acc to gist-1
Akshatkamboj14 Nov 29, 2023
599af85
done some changes acc to gist-1
Akshatkamboj14 Nov 29, 2023
f2f1f69
done some changes acc to gist-1
Akshatkamboj14 Nov 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import org.oppia.android.app.model.ProfileId
import org.oppia.android.databinding.ProfileEditFragmentBinding
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.profile.ProfileManagementController
import org.oppia.android.domain.snackbar.SnackbarController
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import javax.inject.Inject
Expand All @@ -29,7 +30,8 @@ class ProfileEditFragmentPresenter @Inject constructor(
private val activity: AppCompatActivity,
private val fragment: Fragment,
private val oppiaLogger: OppiaLogger,
private val profileManagementController: ProfileManagementController
private val profileManagementController: ProfileManagementController,
private val snackbarManager: SnackbarManager
) {

@Inject
Expand Down Expand Up @@ -151,6 +153,10 @@ class ProfileEditFragmentPresenter @Inject constructor(
fragment,
Observer {
if (it is AsyncResult.Success) {
snackbarManager.showSnackbar(
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
R.string.profile_edit_delete_success,
SnackbarController.SnackbarDuration.LONG
)
if (fragment.requireContext().resources.getBoolean(R.bool.isTablet)) {
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
val intent =
Intent(fragment.requireContext(), AdministratorControlsActivity::class.java)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ class ProfileListActivity :
RouteToProfileEditListener {
@Inject
lateinit var profileListActivityPresenter: ProfileListActivityPresenter
@Inject
lateinit var snackbarManager: SnackbarManager

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
(activityComponent as ActivityComponentImpl).inject(this)
profileListActivityPresenter.handleOnCreate()
snackbarManager.enableShowingSnackbars(this)
}

override fun onSupportNavigateUp(): Boolean {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package org.oppia.android.app.settings.profile

import android.view.View
import androidx.annotation.StringRes
import androidx.appcompat.app.AppCompatActivity
import com.google.android.material.snackbar.Snackbar
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.snackbar.SnackbarController
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import javax.inject.Inject

private const val TAG = "SnackbarManager"
private const val ERROR_MESSAGE = "can't be shown--no activity UI"

/** Manager for showing snackbars. */
class SnackbarManager @Inject constructor(private val snackbarController: SnackbarController) {
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved

@Inject
lateinit var oppiaLogger: OppiaLogger

/**
* Enqueues the snackbar that is be to shown in the FIFO buffer.
*
* @param messageStringId The message string of string resource that is to be displayed
* @param duration The duration for which snackbar is to be shown
*/
fun showSnackbar(@StringRes messageStringId: Int, duration: SnackbarController.SnackbarDuration) {
snackbarController.enqueueSnackbar(
SnackbarController.SnackbarRequest.ShowSnackbar(
messageStringId,
duration
)
)
}

/**
* Enables the activity to show the snackbar.
*
* @param activity An activity that will observe for snackbars and display them
*/
fun enableShowingSnackbars(activity: AppCompatActivity) {
snackbarController.getCurrentSnackbar().toLiveData().observe(activity) { result ->
when (result) {
is AsyncResult.Success -> when (val request = result.value) {
is SnackbarController.SnackbarRequest.ShowSnackbar -> showSnackbar(
activity.findViewById(
android.R.id.content
),
request
)
SnackbarController.SnackbarRequest.ShowNothing -> {
if (snackbarController.snackbarRequestQueue.isNotEmpty()) {
snackbarController.dismissCurrentSnackbar()
}
}
}
else -> {}
}
}
}

private fun showSnackbar(
activityView: View?,
showRequest: SnackbarController.SnackbarRequest.ShowSnackbar
) {

val duration = when (showRequest.duration) {
SnackbarController.SnackbarDuration.SHORT -> Snackbar.LENGTH_SHORT
SnackbarController.SnackbarDuration.LONG -> Snackbar.LENGTH_LONG
}

if (activityView == null) {
oppiaLogger.e(TAG, ERROR_MESSAGE)
} else {
Snackbar.make(activityView, showRequest.messageStringId, duration)
.addCallback(object : Snackbar.Callback() {
override fun onDismissed(transientBottomBar: Snackbar?, event: Int) {
super.onDismissed(transientBottomBar, event)
snackbarController.dismissCurrentSnackbar()
}
})
.show()
}
}
}
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -625,4 +625,5 @@
<string name="nps_score_question">On a scale from 0–10, how likely are you to recommend %s to a friend or colleague?</string>
<string name="previous_subtopic_talkback_text">The previous subtopic is %s</string>
<string name="next_subtopic_talkback_text">The next subtopic is %s</string>
<string name="profile_edit_delete_success">Profile has been successfully deleted.</string>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.common.truth.Truth.assertThat
import dagger.Component
import org.hamcrest.Matchers.allOf
import org.hamcrest.Matchers.not
import org.junit.After
import org.junit.Before
Expand Down Expand Up @@ -121,14 +122,21 @@ import javax.inject.Singleton
@Config(application = ProfileEditFragmentTest.TestApplication::class, qualifiers = "port-xxhdpi")
class ProfileEditFragmentTest {

@get:Rule val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()
@get:Rule val oppiaTestRule = OppiaTestRule()

@Inject lateinit var context: Context
@Inject lateinit var profileTestHelper: ProfileTestHelper
@Inject lateinit var profileManagementController: ProfileManagementController
@Inject lateinit var monitorFactory: DataProviderTestMonitor.Factory
@Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers
@get:Rule
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()
@get:Rule
val oppiaTestRule = OppiaTestRule()

@Inject
lateinit var context: Context
@Inject
lateinit var profileTestHelper: ProfileTestHelper
@Inject
lateinit var profileManagementController: ProfileManagementController
@Inject
lateinit var monitorFactory: DataProviderTestMonitor.Factory
@Inject
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers

@Before
fun setUp() {
Expand Down Expand Up @@ -158,6 +166,36 @@ class ProfileEditFragmentTest {
}
}

@Test
fun testProfileEdit_startWithUserProfile_clickProfileDeletionButton_deleteSnackbarIsVisible() {
launchFragmentTestActivity(internalProfileId = 1).use {
onView(withId(R.id.profile_delete_button)).perform(click())
testCoroutineDispatchers.runCurrent()
onView(withText(R.string.profile_edit_delete_dialog_positive))
.inRoot(isDialog())
.perform(click())
testCoroutineDispatchers.runCurrent()
onView(allOf(withText(R.string.profile_edit_delete_success)))
.check(matches(isDisplayed()))
}
Copy link
Collaborator

@adhiamboperes adhiamboperes Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could try using the visibility matcher instead of the isDisplayedMatcher()

Suggested change
onView(allOf(withText(R.string.profile_edit_delete_success)))
.check(matches(isDisplayed()))
}
onView(withText(R.string.profile_edit_delete_success))
.check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay done.

}

@Test
fun testProfileEdit_startWithUserProfile_AfterDeleteSnackbar_profileActivityIsShownAgain() {
launchFragmentTestActivity(internalProfileId = 1).use {
onView(withId(R.id.profile_delete_button)).perform(click())
testCoroutineDispatchers.runCurrent()
onView(withText(R.string.profile_edit_delete_dialog_positive))
.inRoot(isDialog())
.perform(click())
testCoroutineDispatchers.runCurrent()
onView(allOf(withText(R.string.profile_edit_delete_success)))
.check(matches(isDisplayed()))
testCoroutineDispatchers.runCurrent()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this assertion since it belongs to the pevious test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I am removing this assertion, test is failing on my local,
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore this comment, now it is passing on my local.

intended(hasComponent(ProfileListActivity::class.java.name))
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are failing on CI. Are they actually passing on your local?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @adhiamboperes it is passing on my local

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for this #4887 (comment), I have removed it from my PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can attach a screenshot here to show that the tests are passing on your end.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adhiamboperes I think the tests are failing because snackbar is hiding before this is asserted, wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not so sure. Could you please try out my suggestions above and see if it works?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

@Test
@Config(qualifiers = "land")
fun testProfileEdit_configChange_startWithUserProfile_clickDelete_checkOpensDeletionDialog() {
Expand Down
Loading