refactor: update ModalFragment to use newInstance factory (#84)

Author: grablack

jacoco.gradle

@@ -70,7 +70,9 @@ def createVariantCoverage(variant) {
 		description = "Generate Jacoco coverage reports for the ${variantName.capitalize()} build."
 
 		reports {
-			html.enabled = true
+			html.required = true
+			xml.required = true
+			csv.required = false
 		}
 
 		def javaClasses = fileTree(dir: variant.javaCompileProvider.get().destinationDir, excludes: project.excludes)

library/src/androidTest/java/com/paypal/messages/ModalExternalLinkTest.kt

@@ -37,7 +37,7 @@ class ModalExternalLinkTest {
 				val webView = WebView(recordingContext)
 
 				// Initialize the modal WebView configuration
-				val fragment = ModalFragment(clientId = "test-client-id")
+				val fragment = ModalFragment.newInstance("test-client-id")
 				fragment.setupWebView(webView)
 
 				// Load a minimal page that triggers a target=_blank navigation

library/src/androidTest/java/com/paypal/messages/ModalFragmentTest.kt

@@ -1,13 +1,357 @@
-package com.paypal.messages.totest
+package com.paypal.messages
 
+import android.os.Bundle
+import androidx.fragment.app.testing.launchFragmentInContainer
+import androidx.test.core.app.ActivityScenario
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import com.paypal.messages.config.modal.ModalCloseButton
+import com.paypal.messages.config.modal.ModalConfig
+import com.paypal.messages.config.modal.ModalEvents
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 
 @RunWith(AndroidJUnit4::class)
 class ModalFragmentTest {
+
+	@Test
+	fun newInstance_setsClientIdInArguments() {
+		val testClientId = "test-client-id-123"
+		val fragment = ModalFragment.newInstance(testClientId)
+
+		assertNotNull("Fragment should not be null", fragment)
+		assertNotNull("Arguments should not be null", fragment.arguments)
+		assertEquals(
+			"ClientId should be stored in arguments",
+			testClientId,
+			fragment.arguments?.getString("clientId"),
+		)
+	}
+
+	@Test
+	fun fragmentRestoration_preservesClientId() {
+		val testClientId = "test-client-id-restoration"
+		
+		// Create fragment using factory method
+		val originalFragment = ModalFragment.newInstance(testClientId)
+		
+		// Get the arguments that would be saved (Android automatically saves/restores arguments)
+		val savedArguments = originalFragment.arguments
+		
+		// Create a new fragment instance (simulating Android's recreation when "Don't keep activities" is enabled)
+		// This uses the no-arg constructor, which is required for proper state restoration
+		val recreatedFragment = ModalFragment()
+		
+		// Restore the arguments (Android does this automatically during recreation)
+		recreatedFragment.arguments = savedArguments
+		
+		// Verify clientId can be accessed (this would fail with the old constructor approach)
+		// We can't directly access private clientId, but we can verify arguments are set
+		assertNotNull("Recreated fragment arguments should not be null", recreatedFragment.arguments)
+		assertEquals(
+			"ClientId should be preserved in recreated fragment",
+			testClientId,
+			recreatedFragment.arguments?.getString("clientId"),
+		)
+	}
+
+	@Test
+	fun fragmentRecreation_withActivityScenario() {
+		val testClientId = "test-client-id-scenario"
+		
+		// Launch fragment in a container using FragmentScenario
+		val scenario = launchFragmentInContainer<ModalFragment>(
+			Bundle().apply {
+				putString("clientId", testClientId)
+			},
+		)
+		
+		// Verify fragment is created
+		scenario.onFragment { fragment ->
+			assertNotNull("Fragment should be created", fragment)
+			assertNotNull("Fragment arguments should be set", fragment.arguments)
+			assertEquals(
+				"ClientId should be accessible",
+				testClientId,
+				fragment.arguments?.getString("clientId"),
+			)
+		}
+		
+		// Simulate activity recreation (like "Don't keep activities")
+		scenario.recreate()
+		
+		// Verify fragment is recreated with same arguments
+		scenario.onFragment { recreatedFragment ->
+			assertNotNull("Recreated fragment should not be null", recreatedFragment)
+			assertNotNull("Recreated fragment arguments should not be null", recreatedFragment.arguments)
+			assertEquals(
+				"ClientId should be preserved after recreation",
+				testClientId,
+				recreatedFragment.arguments?.getString("clientId"),
+			)
+		}
+	}
+
+	@Test
+	fun fragmentWithoutArguments_throwsException() {
+		// Create fragment without using newInstance (simulating old way or error case)
+		val fragment = ModalFragment()
+		fragment.arguments = null // No arguments set
+		
+		// Accessing clientId property should throw IllegalStateException
+		// We test this indirectly by verifying the fragment can't work without arguments
+		// Since clientId is private, we test via reflection or by ensuring arguments are required
+		assertNotNull("Fragment should be created", fragment)
+		// The actual exception would be thrown when clientId is accessed internally
+		// This test documents the expected behavior
+	}
+
+	@Test
+	fun newInstance_differentClientIds() {
+		val clientId1 = "client-id-1"
+		val clientId2 = "client-id-2"
+		
+		val fragment1 = ModalFragment.newInstance(clientId1)
+		val fragment2 = ModalFragment.newInstance(clientId2)
+		
+		assertEquals(
+			"First fragment should have first clientId",
+			clientId1,
+			fragment1.arguments?.getString("clientId"),
+		)
+		assertEquals(
+			"Second fragment should have second clientId",
+			clientId2,
+			fragment2.arguments?.getString("clientId"),
+		)
+	}
+
+	@Test
+	fun fragmentStateRestoration_simulatesDontKeepActivities() {
+		val testClientId = "test-client-dont-keep-activities"
+		
+		// Step 1: Create fragment using factory method (normal flow)
+		val fragment = ModalFragment.newInstance(testClientId)
+		
+		// Step 2: Simulate what happens when "Don't keep activities" is enabled:
+		// - Activity is destroyed
+		// - Fragment arguments are automatically saved by Android
+		// - Fragment instance is destroyed
+		val savedArguments = Bundle().apply {
+			putAll(fragment.arguments ?: Bundle())
+		}
+		
+		// Step 3: Simulate Android recreating the fragment (uses no-arg constructor)
+		// This is the key fix: the fragment must have a no-arg constructor
+		val recreatedFragment = ModalFragment()
+		
+		// Step 4: Android restores the arguments Bundle automatically
+		recreatedFragment.arguments = savedArguments
+		
+		// Step 5: Verify the fragment can function properly
+		// The key test: can we access clientId without constructor?
+		assertEquals(
+			"Recreated fragment should have same clientId",
+			testClientId,
+			recreatedFragment.arguments?.getString("clientId"),
+		)
+		
+		// This test verifies the fix: fragment can be recreated using no-arg constructor
+		// and clientId is accessible via arguments Bundle
+	}
+
+	/**
+	 * Test that dismissing a fragment removes it from the fragment manager
+	 * and marks it as not added
+	 */
 	@Test
-	fun testSomething() {
-		//
+	fun dismissFragment_removesFromFragmentManager() {
+		val testClientId = "test-client-dismiss"
+		val context = InstrumentationRegistry.getInstrumentation().targetContext
+		val activityScenario = ActivityScenario.launch(TestActivity::class.java)
+
+		activityScenario.onActivity { activity ->
+			val fragment = ModalFragment.newInstance(testClientId)
+			
+			// Initialize fragment with minimal config
+			fragment.init(
+				ModalConfig(
+					amount = 100.0,
+					buyerCountry = "US",
+					offer = null,
+					ignoreCache = false,
+					devTouchpoint = false,
+					stageTag = null,
+					events = ModalEvents(),
+					modalCloseButton = ModalCloseButton(),
+				),
+			)
+
+			// Show the fragment
+			fragment.show(activity.supportFragmentManager, "test-modal")
+			
+			// Wait for fragment to be added
+			activity.supportFragmentManager.executePendingTransactions()
+			
+			// Verify fragment is added
+			assertTrue(
+				"Fragment should be added to fragment manager",
+				fragment.isAdded,
+			)
+			
+			// Dismiss the fragment
+			fragment.dismiss()
+			activity.supportFragmentManager.executePendingTransactions()
+			
+			// Verify fragment is no longer added
+			assertFalse(
+				"Fragment should not be added after dismissal",
+				fragment.isAdded,
+			)
+			
+			// Verify fragment is not in fragment manager
+			val foundFragment = activity.supportFragmentManager.findFragmentByTag("test-modal")
+			assertTrue(
+				"Fragment should be removed from fragment manager after dismissal",
+				foundFragment == null || !foundFragment.isAdded,
+			)
+		}
+	}
+
+	/**
+	 * Test that a dismissed fragment is not restored after activity recreation
+	 * This simulates the scenario: open modal -> dismiss -> press home -> reopen app
+	 */
+	@Test
+	fun dismissedFragment_notRestoredAfterActivityRecreation() {
+		val testClientId = "test-client-dismiss-restore"
+		val context = InstrumentationRegistry.getInstrumentation().targetContext
+		val activityScenario = ActivityScenario.launch(TestActivity::class.java)
+
+		activityScenario.onActivity { activity ->
+			val fragment = ModalFragment.newInstance(testClientId)
+			
+			// Initialize fragment
+			fragment.init(
+				ModalConfig(
+					amount = 100.0,
+					buyerCountry = "US",
+					offer = null,
+					ignoreCache = false,
+					devTouchpoint = false,
+					stageTag = null,
+					events = ModalEvents(),
+					modalCloseButton = ModalCloseButton(),
+				),
+			)
+
+			// Show the fragment
+			fragment.show(activity.supportFragmentManager, "test-modal-restore")
+			activity.supportFragmentManager.executePendingTransactions()
+			
+			// Verify fragment is shown
+			assertTrue("Fragment should be added", fragment.isAdded)
+			
+			// Dismiss the fragment (simulating user clicking close button)
+			fragment.dismiss()
+			activity.supportFragmentManager.executePendingTransactions()
+			
+			// Verify fragment is dismissed
+			assertFalse("Fragment should be dismissed", fragment.isAdded)
+		}
+
+		// Simulate activity recreation (like when app goes to background and comes back)
+		activityScenario.recreate()
+
+		activityScenario.onActivity { recreatedActivity ->
+			// After recreation, check that the dismissed fragment is not restored
+			val restoredFragment = recreatedActivity.supportFragmentManager.findFragmentByTag("test-modal-restore")
+			
+			// The fragment should not exist or should not be added
+			assertTrue(
+				"Dismissed fragment should not be restored after activity recreation",
+				restoredFragment == null || !restoredFragment.isAdded,
+			)
+		}
+	}
+
+	/**
+	 * Test that onCreateView handles null closeButtonData gracefully
+	 * (This tests the fix for the NullPointerException during fragment recreation)
+	 */
+	@Test
+	fun onCreateView_withNullCloseButtonData_usesDefaults() {
+		val testClientId = "test-client-null-button"
+		val scenario = launchFragmentInContainer<ModalFragment>(
+			Bundle().apply {
+				putString("clientId", testClientId)
+			},
+		)
+
+		// onCreateView should not crash even if closeButtonData is null
+		// This happens during fragment recreation before init() is called
+		scenario.onFragment { fragment ->
+			// Fragment should be created successfully
+			assertNotNull("Fragment should be created", fragment)
+			assertNotNull("Fragment view should be created", fragment.view)
+			
+			// The view should have the close button with default values
+			val closeButton = fragment.view?.findViewById<android.widget.ImageButton>(
+				com.paypal.messages.R.id.ModalCloseButton,
+			)
+			assertNotNull("Close button should exist", closeButton)
+		}
+	}
+
+	/**
+	 * Test that onDismiss callback is invoked when fragment is dismissed
+	 */
+	@Test
+	fun dismissFragment_invokesOnDismissCallback() {
+		val testClientId = "test-client-on-dismiss"
+		val context = InstrumentationRegistry.getInstrumentation().targetContext
+		val activityScenario = ActivityScenario.launch(TestActivity::class.java)
+		var onDismissCalled = false
+
+		activityScenario.onActivity { activity ->
+			val fragment = ModalFragment.newInstance(testClientId)
+			
+			// Initialize fragment with onClose callback
+			fragment.init(
+				ModalConfig(
+					amount = 100.0,
+					buyerCountry = "US",
+					offer = null,
+					ignoreCache = false,
+					devTouchpoint = false,
+					stageTag = null,
+					events = ModalEvents(
+						onClose = { onDismissCalled = true },
+					),
+					modalCloseButton = ModalCloseButton(),
+				),
+			)
+
+			// Show the fragment
+			fragment.show(activity.supportFragmentManager, "test-modal-dismiss")
+			activity.supportFragmentManager.executePendingTransactions()
+			
+			// Dismiss the fragment
+			fragment.dismiss()
+			activity.supportFragmentManager.executePendingTransactions()
+		}
+
+		// Give time for the dismiss callback to be invoked
+		Thread.sleep(100)
+
+		// Verify onDismiss callback was called
+		assertTrue(
+			"onDismiss callback should be invoked when fragment is dismissed",
+			onDismissCalled,
+		)
 	}
 }

library/src/androidTest/java/com/paypal/messages/PayPalMessageViewTest.kt

@@ -113,7 +113,7 @@ class PayPalMessageViewTest {
 // 	fun dismissAfterFragmentDetached_shouldThrow() {
 // 		val scenario: ActivityScenario<TestActivity> = ActivityScenario.launch(TestActivity::class.java)
 // 		scenario.onActivity { activity: TestActivity ->
-// 			val fragment = ModalFragment("test_client_id")
+// 			val fragment = ModalFragment.newInstance("test_client_id")
 // 			fragment.show(activity.supportFragmentManager, "test")
 //
 // 			// Remove the fragment to simulate detachment