Awight has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/341848 )
Change subject: [WIP] Put the ctor sequence back as it was, fix fatals. ...................................................................... [WIP] Put the ctor sequence back as it was, fix fatals. Change-Id: I820e7c847619928a70e63ddb1102ee663a8120ce --- M gateway_common/gateway.adapter.php M globalcollect_gateway/orphan.adapter.php M tests/phpunit/DonationDataTest.php M tests/phpunit/GatewayValidationTest.php 4 files changed, 45 insertions(+), 61 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface refs/changes/48/341848/1 diff --git a/gateway_common/gateway.adapter.php b/gateway_common/gateway.adapter.php index 65619bc..fc2dc29 100644 --- a/gateway_common/gateway.adapter.php +++ b/gateway_common/gateway.adapter.php @@ -245,6 +245,18 @@ $this->defineDataTransformers(); + $this->session_resetOnSwitch(); // Need to do this before creating DonationData + + // FIXME: this should not have side effects like setting order_id_meta['final'] + // TODO: On second thought, neither set data nor validate in this constructor. + $this->dataObj = new DonationData( $this, $options['external_data'] ); + + $this->unstaged_data = $this->dataObj->getData(); + $this->staged_data = $this->unstaged_data; + + // checking to see if we have an edit token in the request... + $this->posted = ( $this->dataObj->wasPosted() && ( !is_null( WmfFramework::getRequestValue( 'wmf_token', null ) ) ) ); + $this->findAccount(); $this->defineAccountInfo(); $this->defineTransactions(); @@ -254,23 +266,9 @@ $this->setGatewayDefaults( $options ); - $this->session_resetOnSwitch(); // Need to do this before creating DonationData - - // FIXME: this should not have side effects like setting order_id_meta['final'] - // TODO: On second thought, neither set data nor validate in this constructor. - $this->dataObj = new DonationData( $this, $options['external_data'] ); - // FIXME: Same as above, don't validate or stage in the constructor. - // Sets $this->errors if validation fails. $this->validate(); - $this->unstaged_data = $this->dataObj->getData(); - $this->staged_data = $this->unstaged_data; - - // checking to see if we have an edit token in the request... - $this->posted = ( $this->dataObj->wasPosted() && ( !is_null( WmfFramework::getRequestValue( 'wmf_token', null ) ) ) ); - - // FIXME: Don't stage if invalid. $this->stageData(); BannerHistoryLogIdProcessor::onGatewayReady( $this ); @@ -1232,7 +1230,7 @@ */ public function getPaymentMethod() { // FIXME: this should return the final calculated method - return $this->dataObj->getVal( 'payment_method' ); + return $this->getData_Unstaged_Escaped( 'payment_method' ); } /** @@ -1247,7 +1245,7 @@ } public function getPaymentSubmethod() { - return $this->dataObj->getVal( 'payment_submethod' ); + return $this->getData_Unstaged_Escaped( 'payment_submethod' ); } public function getPaymentSubmethods() { @@ -2379,7 +2377,7 @@ // Add any country-specific required fields if ( isset( $this->config['country_fields'] ) ) { - $country = $this->dataObj->getVal( 'country' ); + $country = $this->getData_Unstaged_Escaped( 'country' ); if ( $country && isset( $this->config['country_fields'][$country] ) ) { $validation = $this->config['country_fields'][$country]; } @@ -2417,7 +2415,7 @@ //however, that's not happening in this class in the code I'm replacing, so... //TODO: Something clever in the DataValidator with data groups like these. ); - $country = $this->dataObj->getVal( 'country' ); + $country = $this->getData_Unstaged_Escaped( 'country' ); if ( $country && Subdivisions::getByCountry( $country ) ) { $check_not_empty[] = 'state'; } diff --git a/globalcollect_gateway/orphan.adapter.php b/globalcollect_gateway/orphan.adapter.php index c1edf13..aaf48c2 100644 --- a/globalcollect_gateway/orphan.adapter.php +++ b/globalcollect_gateway/orphan.adapter.php @@ -63,7 +63,7 @@ $this->dataObj = new DonationData( $this, $data ); - $this->unstaged_data = $this->dataObj->getDataEscaped(); + $this->unstaged_data = $this->dataObj->getData(); $this->hard_data = array_merge( $this->hard_data, $this->getContributionTracking() ); $this->reAddHardData(); diff --git a/tests/phpunit/DonationDataTest.php b/tests/phpunit/DonationDataTest.php index 7b14dd1..96b7cfc 100644 --- a/tests/phpunit/DonationDataTest.php +++ b/tests/phpunit/DonationDataTest.php @@ -79,7 +79,7 @@ /** * @covers DonationData::__construct - * @covers DonationData::getDataEscaped + * @covers DonationData::getData * @covers DonationData::populateData */ public function testConstruct(){ @@ -87,7 +87,7 @@ $request = $context->getRequest(); $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ) ); //as if we were posted. - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $expected = array( 'posted' => '', 'amount' => '0.00', 'appeal' => 'JimmyQuote', @@ -153,7 +153,7 @@ $adapter = $this->getFreshGatewayObject( self::$initial_vars, array( 'batch_mode' => true ) ); $ddObj = new DonationData( $adapter, $expected ); //external data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertNotNull( $returned['contribution_tracking_id'], 'There is no contribution tracking ID' ); @@ -205,7 +205,7 @@ RequestContext::getMain()->setRequest( new FauxRequest( $expected, false ) ); $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ) ); //Get all data from request - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertNotNull( $returned['contribution_tracking_id'], 'There is no contribution tracking ID' ); $this->assertNotEquals( $returned['contribution_tracking_id'], '', 'There is not a valid contribution tracking ID' ); @@ -255,7 +255,7 @@ unset($expected['test_string']); $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $expected ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); //unset these, because they're always new $unsettable = array( 'order_id', @@ -290,7 +290,7 @@ $data['amount'] = 'this is not a number'; $data['amountGiven'] = 42.50; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals( 42.50, $returned['amount'], "Amount was not properly reset" ); $this->assertArrayNotHasKey( 'amountGiven', $returned, "amountGiven should have been removed from the data" ); } @@ -303,7 +303,7 @@ $data['amount'] = 88.15; $data['amountGiven'] = 42.50; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals( 88.15, $returned['amount'], "Amount was not properly reset" ); $this->assertArrayNotHasKey( 'amountGiven', $returned, "amountGiven should have been removed from the data" ); } @@ -316,7 +316,7 @@ $data['amount'] = -1; $data['amountOther'] = 3.25; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals(3.25, $returned['amount'], "Amount was not properly reset"); $this->assertArrayNotHasKey( 'amountOther', $returned, "amountOther should have been removed from the data"); } @@ -330,7 +330,7 @@ $data['amountGiven'] = 'wombat'; $data['amountOther'] = 'macedonia'; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals( 'invalid', $returned['amount'], "Amount was not properly reset"); $this->assertArrayNotHasKey( 'amountOther', $returned, "amountOther should have been removed from the data"); $this->assertArrayNotHasKey( 'amountGiven', $returned, "amountGiven should have been removed from the data"); @@ -346,7 +346,7 @@ // country - in this test data, US. $data['currency_code'] = 'splunge'; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals( 'USD', $returned['currency_code'], 'Currency code was not properly reset'); } @@ -361,7 +361,7 @@ $data['uselang'] = 'no'; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals( 'no', $returned['language'], "Language 'no' was normalized out of existance. Sad." ); $this->assertArrayNotHasKey( 'uselang', $returned, "'uselang' should have been removed from the data" ); } @@ -377,7 +377,7 @@ $data['language'] = 'no'; $ddObj = new DonationData( $this->getFreshGatewayObject( self::$initial_vars ), $data ); //change to test mode with explicit test data - $returned = $ddObj->getDataEscaped(); + $returned = $ddObj->getData(); $this->assertEquals( 'no', $returned['language'], "Language 'no' was normalized out of existance. Sad." ); $this->assertArrayNotHasKey( 'uselang', $returned, "'uselang' should have been removed from the data" ); } diff --git a/tests/phpunit/GatewayValidationTest.php b/tests/phpunit/GatewayValidationTest.php index e009026..e9fb7e3 100644 --- a/tests/phpunit/GatewayValidationTest.php +++ b/tests/phpunit/GatewayValidationTest.php @@ -41,8 +41,6 @@ TestingGenericAdapter::$acceptedCurrencies[] = 'USD'; $this->page = new TestingGatewayPage(); - $this->adapter = new TestingGenericAdapter(); - $this->page->adapter = $this->adapter; } public function tearDown() { @@ -51,27 +49,30 @@ parent::tearDown(); } + public function setUpAdapter( $data = array() ) { + $this->adapter = new TestingGenericAdapter( array( + 'external_data' => $data, + ) ); + $this->page->adapter = $this->adapter; + } + public function testPassesValidation() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '2.00', 'country' => 'US', 'currency' => 'USD', 'email' => 'f...@localhost.net', ) ); - $this->page->validateForm(); - $this->assertTrue( $this->adapter->validatedOK() ); } public function testLowAmountError() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '1.99', 'country' => 'US', 'currency' => 'USD', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -80,13 +81,11 @@ } public function testHighAmountError() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '100.99', 'country' => 'US', 'currency' => 'USD', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -95,13 +94,11 @@ } public function testCurrencyCodeError() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '2.99', 'country' => 'BR', 'currency' => 'BRL', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -116,13 +113,11 @@ 'wgDonationInterfaceForbiddenCountries' => array( 'XX' ) ) ); - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '2.99', 'country' => 'XX', 'currency' => 'USD', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -131,13 +126,11 @@ } public function testEmailError() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '2.99', 'currency' => 'USD', 'email' => 'foo', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -146,13 +139,11 @@ } public function testSpuriousCcError() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '2.99', 'currency' => 'USD', 'fname' => '4111111111111111', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -161,11 +152,9 @@ } public function testMissingFieldError() { - $this->adapter->addRequestData( array( + $this->setUpAdapter( array( 'amount' => '2.99', ) ); - - $this->page->validateForm(); $this->assertFalse( $this->adapter->validatedOK() ); @@ -184,10 +173,7 @@ // GatewayAdapter::addRequestData(). TestingGenericAdapter::$fakeIdentifier = $badGateway; - $this->adapter = new TestingGenericAdapter(); - $this->page->adapter = $this->adapter; - - $this->page->validateForm(); + $this->setUpAdapter(); $this->assertFalse( $this->adapter->validatedOK() ); -- To view, visit https://gerrit.wikimedia.org/r/341848 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I820e7c847619928a70e63ddb1102ee663a8120ce Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/DonationInterface Gerrit-Branch: master Gerrit-Owner: Awight <awi...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits