Complex classes like AddDonationRouteTest often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use AddDonationRouteTest, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 27 | class AddDonationRouteTest extends WebRouteTestCase { |
||
| 28 | |||
| 29 | const SOME_TOKEN = 'SomeToken'; |
||
| 30 | |||
| 31 | public function testGivenValidRequest_donationGetsPersisted(): void { |
||
| 32 | $this->createEnvironment( [], function ( Client $client, FunFunFactory $factory ): void { |
||
| 33 | |||
| 34 | $client->setServerParameter( 'HTTP_REFERER', 'https://en.wikipedia.org/wiki/Karla_Kennichnich' ); |
||
| 35 | $client->followRedirects( false ); |
||
| 36 | |||
| 37 | $client->request( |
||
| 38 | 'POST', |
||
| 39 | '/donation/add', |
||
| 40 | $this->newValidFormInput() |
||
| 41 | ); |
||
| 42 | |||
| 43 | $this->assertIsExpectedDonation( $this->getDonationFromDatabase( $factory ) ); |
||
| 44 | } ); |
||
| 45 | } |
||
| 46 | |||
| 47 | public function testWhenDonationGetsPersisted_timestampIsStoredInCookie(): void { |
||
| 48 | $client = $this->createClient(); |
||
| 49 | $client->followRedirects( true ); |
||
| 50 | $client->request( |
||
| 51 | 'POST', |
||
| 52 | '/donation/add', |
||
| 53 | $this->newValidFormInput() |
||
| 54 | ); |
||
| 55 | |||
| 56 | $cookie = $client->getCookieJar()->get( 'donation_timestamp' ); |
||
| 57 | $this->assertNotNull( $cookie ); |
||
| 58 | $donationTimestamp = new \DateTime( $cookie->getValue() ); |
||
| 59 | $this->assertEquals( time(), $donationTimestamp->getTimestamp(), 'Timestamp should be not more than 5 seconds old', 5.0 ); |
||
| 60 | } |
||
| 61 | |||
| 62 | public function testWhenMultipleDonationFormSubmissions_requestGetsRejected(): void { |
||
| 63 | $client = $this->createClient(); |
||
| 64 | $client->getCookieJar()->set( new Cookie( 'donation_timestamp', $this->getPastTimestamp() ) ); |
||
| 65 | |||
| 66 | $client->request( |
||
| 67 | 'POST', |
||
| 68 | '/donation/add', |
||
| 69 | $this->newValidFormInput() |
||
| 70 | ); |
||
| 71 | |||
| 72 | $this->assertContains( 'donation_rejected_limit', $client->getResponse()->getContent() ); |
||
| 73 | } |
||
| 74 | |||
| 75 | public function testWhenMultipleDonationsInAccordanceToTimeLimit_requestIsNotRejected(): void { |
||
| 76 | $client = $this->createClient(); |
||
| 77 | $client->getCookieJar()->set( |
||
| 78 | new Cookie( |
||
| 79 | 'donation_timestamp', |
||
| 80 | $this->getPastTimestamp( 'PT35M' ) |
||
| 81 | ) |
||
| 82 | ); |
||
| 83 | |||
| 84 | $client->request( |
||
| 85 | 'POST', |
||
| 86 | '/donation/add', |
||
| 87 | $this->newValidFormInput() |
||
| 88 | ); |
||
| 89 | |||
| 90 | $this->assertNotContains( 'donation_rejected_limit', $client->getResponse()->getContent() ); |
||
| 91 | } |
||
| 92 | |||
| 93 | private function getPastTimestamp( string $interval = 'PT10S' ): string { |
||
| 94 | return ( new \DateTime() )->sub( new \DateInterval( $interval ) )->format( 'Y-m-d H:i:s' ); |
||
| 95 | } |
||
| 96 | |||
| 97 | private function newValidFormInput(): array { |
||
| 98 | return [ |
||
| 99 | 'betrag' => '5,51', |
||
| 100 | 'zahlweise' => 'BEZ', |
||
| 101 | 'periode' => 0, |
||
| 102 | 'iban' => 'DE12500105170648489890', |
||
| 103 | 'bic' => 'INGDDEFFXXX', |
||
| 104 | 'konto' => '0648489890', |
||
| 105 | 'blz' => '50010517', |
||
| 106 | 'bankname' => 'ING-DiBa', |
||
| 107 | 'addressType' => 'person', |
||
| 108 | 'salutation' => 'Frau', |
||
| 109 | 'title' => 'Prof. Dr.', |
||
| 110 | 'company' => '', |
||
| 111 | 'firstName' => 'Karla', |
||
| 112 | 'lastName' => 'Kennichnich', |
||
| 113 | 'street' => 'Lehmgasse 12', |
||
| 114 | 'postcode' => '12345', |
||
| 115 | 'city' => 'Einort', |
||
| 116 | 'country' => 'DE', |
||
| 117 | 'email' => '[email protected]', |
||
| 118 | 'info' => '1', |
||
| 119 | 'piwik_campaign' => 'test', |
||
| 120 | 'piwik_kwd' => 'gelb', |
||
| 121 | 'impCount' => '3', |
||
| 122 | 'bImpCount' => '1', |
||
| 123 | 'layout' => 'Default', |
||
| 124 | 'color' => 'blue', |
||
| 125 | 'skin' => 'default', |
||
| 126 | ]; |
||
| 127 | } |
||
| 128 | |||
| 129 | private function assertIsExpectedDonation( Donation $donation ): void { |
||
| 130 | $data = $donation->getDecodedData(); |
||
| 131 | $this->assertSame( '5.51', $donation->getAmount() ); |
||
| 132 | $this->assertSame( 'BEZ', $donation->getPaymentType() ); |
||
| 133 | $this->assertSame( 0, $donation->getPaymentIntervalInMonths() ); |
||
| 134 | $this->assertSame( 'DE12500105170648489890', $data['iban'] ); |
||
| 135 | $this->assertSame( 'INGDDEFFXXX', $data['bic'] ); |
||
| 136 | $this->assertSame( '0648489890', $data['konto'] ); |
||
| 137 | $this->assertSame( '50010517', $data['blz'] ); |
||
| 138 | $this->assertSame( 'ING-DiBa', $data['bankname'] ); |
||
| 139 | $this->assertSame( 'person', $data['adresstyp'] ); |
||
| 140 | $this->assertSame( 'Frau', $data['anrede'] ); |
||
| 141 | $this->assertSame( 'Prof. Dr.', $data['titel'] ); |
||
| 142 | $this->assertSame( '', $data['firma'] ); |
||
| 143 | $this->assertSame( 'Karla', $data['vorname'] ); |
||
| 144 | $this->assertSame( 'Kennichnich', $data['nachname'] ); |
||
| 145 | $this->assertSame( 'Prof. Dr. Karla Kennichnich', $donation->getDonorFullName() ); |
||
| 146 | $this->assertSame( 'Lehmgasse 12', $data['strasse'] ); |
||
| 147 | $this->assertSame( '12345', $data['plz'] ); |
||
| 148 | $this->assertSame( 'Einort', $data['ort'] ); |
||
| 149 | $this->assertSame( 'Einort', $donation->getDonorCity() ); |
||
| 150 | $this->assertSame( 'DE', $data['country'] ); |
||
| 151 | $this->assertSame( '[email protected]', $data['email'] ); |
||
| 152 | $this->assertSame( '[email protected]', $donation->getDonorEmail() ); |
||
| 153 | $this->assertSame( 'test/gelb', $data['tracking'] ); |
||
| 154 | $this->assertSame( 3, $data['impCount'] ); |
||
| 155 | $this->assertSame( 1, $data['bImpCount'] ); |
||
| 156 | $this->assertSame( '', $data['layout'] ); |
||
| 157 | $this->assertSame( '', $data['color'] ); |
||
| 158 | $this->assertSame( '', $data['skin'] ); |
||
| 159 | $this->assertSame( 'en.wikipedia.org', $data['source'] ); |
||
| 160 | $this->assertSame( 'N', $donation->getStatus() ); |
||
| 161 | $this->assertTrue( $donation->getDonorOptsIntoNewsletter() ); |
||
| 162 | } |
||
| 163 | |||
| 164 | public function testGivenValidRequest_confirmationPageContainsEnteredData(): void { |
||
| 165 | $client = $this->createClient(); |
||
| 166 | $client->request( |
||
| 167 | 'POST', |
||
| 168 | '/donation/add', |
||
| 169 | $this->newValidFormInput() |
||
| 170 | ); |
||
| 171 | $client->followRedirect(); |
||
| 172 | |||
| 173 | $response = $client->getResponse()->getContent(); |
||
| 174 | |||
| 175 | $this->assertContains( '5,51 €', $response ); |
||
| 176 | $this->assertContains( 'donation.interval: 0', $response ); |
||
| 177 | $this->assertContains( 'DE12500105170648489890', $response ); |
||
| 178 | $this->assertContains( 'INGDDEFFXXX', $response ); |
||
| 179 | $this->assertContains( 'ING-DiBa', $response ); |
||
| 180 | $this->assertContains( 'Prof. Dr. Karla Kennichnich', $response ); |
||
| 181 | $this->assertContains( 'Lehmgasse 12', $response ); |
||
| 182 | $this->assertContains( '<span id="confirm-postcode">12345</span> <span id="confirm-city">Einort</span>', $response ); |
||
| 183 | $this->assertContains( '[email protected]', $response ); |
||
| 184 | $this->assertContains( 'send-info', $response ); |
||
| 185 | } |
||
| 186 | |||
| 187 | public function testGivenValidBankTransferRequest_donationGetsPersisted(): void { |
||
| 188 | /** |
||
| 189 | * @var FunFunFactory |
||
| 190 | */ |
||
| 191 | $factory = null; |
||
|
|
|||
| 192 | |||
| 193 | $this->createEnvironment( [], function ( Client $client, FunFunFactory $factory ): void { |
||
| 194 | |||
| 195 | $client->setServerParameter( 'HTTP_REFERER', 'https://en.wikipedia.org/wiki/Karla_Kennichnich' ); |
||
| 196 | $client->followRedirects( false ); |
||
| 197 | |||
| 198 | $client->request( |
||
| 199 | 'POST', |
||
| 200 | '/donation/add', |
||
| 201 | $this->newValidBankTransferInput() |
||
| 202 | ); |
||
| 203 | |||
| 204 | $donation = $this->getDonationFromDatabase( $factory ); |
||
| 205 | |||
| 206 | $data = $donation->getDecodedData(); |
||
| 207 | $this->assertSame( '12.34', $donation->getAmount() ); |
||
| 208 | $this->assertSame( 'UEB', $donation->getPaymentType() ); |
||
| 209 | $this->assertSame( 0, $donation->getPaymentIntervalInMonths() ); |
||
| 210 | $this->assertSame( 'person', $data['adresstyp'] ); |
||
| 211 | $this->assertSame( 'Frau', $data['anrede'] ); |
||
| 212 | $this->assertSame( 'Prof. Dr.', $data['titel'] ); |
||
| 213 | $this->assertSame( '', $data['firma'] ); |
||
| 214 | $this->assertSame( 'Karla', $data['vorname'] ); |
||
| 215 | $this->assertSame( 'Kennichnich', $data['nachname'] ); |
||
| 216 | $this->assertSame( 'Prof. Dr. Karla Kennichnich', $donation->getDonorFullName() ); |
||
| 217 | $this->assertSame( 'Lehmgasse 12', $data['strasse'] ); |
||
| 218 | $this->assertSame( '12345', $data['plz'] ); |
||
| 219 | $this->assertSame( 'Einort', $data['ort'] ); |
||
| 220 | $this->assertSame( 'Einort', $donation->getDonorCity() ); |
||
| 221 | $this->assertSame( 'DE', $data['country'] ); |
||
| 222 | $this->assertSame( '[email protected]', $data['email'] ); |
||
| 223 | $this->assertSame( '[email protected]', $donation->getDonorEmail() ); |
||
| 224 | $this->assertSame( 'test/gelb', $data['tracking'] ); |
||
| 225 | $this->assertSame( 3, $data['impCount'] ); |
||
| 226 | $this->assertSame( 1, $data['bImpCount'] ); |
||
| 227 | $this->assertSame( '', $data['layout'] ); |
||
| 228 | $this->assertSame( '', $data['color'] ); |
||
| 229 | $this->assertSame( '', $data['skin'] ); |
||
| 230 | $this->assertSame( 'en.wikipedia.org', $data['source'] ); |
||
| 231 | $this->assertSame( true, $donation->getDonorOptsIntoNewsletter() ); |
||
| 232 | |||
| 233 | $this->assertSame( 'Z', $donation->getStatus() ); |
||
| 234 | $this->assertRegExp( '/W-Q-[A-Z]{6}-[A-Z]/', $donation->getBankTransferCode() ); |
||
| 235 | } ); |
||
| 236 | } |
||
| 237 | |||
| 238 | private function newValidBankTransferInput(): array { |
||
| 239 | return [ |
||
| 240 | 'betrag' => '12,34', |
||
| 241 | 'zahlweise' => 'UEB', |
||
| 242 | 'periode' => 0, |
||
| 243 | 'addressType' => 'person', |
||
| 244 | 'salutation' => 'Frau', |
||
| 245 | 'title' => 'Prof. Dr.', |
||
| 246 | 'company' => '', |
||
| 247 | 'firstName' => 'Karla', |
||
| 248 | 'lastName' => 'Kennichnich', |
||
| 249 | 'street' => 'Lehmgasse 12', |
||
| 250 | 'postcode' => '12345', |
||
| 251 | 'city' => 'Einort', |
||
| 252 | 'country' => 'DE', |
||
| 253 | 'email' => '[email protected]', |
||
| 254 | 'info' => '1', |
||
| 255 | 'piwik_campaign' => 'test', |
||
| 256 | 'piwik_kwd' => 'gelb', |
||
| 257 | 'impCount' => '3', |
||
| 258 | 'bImpCount' => '1', |
||
| 259 | 'layout' => 'Default', |
||
| 260 | 'color' => 'blue', |
||
| 261 | 'skin' => 'default', |
||
| 262 | ]; |
||
| 263 | } |
||
| 264 | |||
| 265 | public function testGivenComplementableBankData_donationStillGetsPersisted(): void { |
||
| 266 | $this->createEnvironment( [], function ( Client $client, FunFunFactory $factory ): void { |
||
| 267 | |||
| 268 | $client->setServerParameter( 'HTTP_REFERER', 'https://en.wikipedia.org/wiki/Karla_Kennichnich' ); |
||
| 269 | $client->followRedirects( false ); |
||
| 270 | |||
| 271 | $client->request( |
||
| 272 | 'POST', |
||
| 273 | '/donation/add', |
||
| 274 | $this->newComplementableFormInput() |
||
| 275 | ); |
||
| 276 | |||
| 277 | $donation = $this->getDonationFromDatabase( $factory ); |
||
| 278 | |||
| 279 | $data = $donation->getDecodedData(); |
||
| 280 | $this->assertSame( 'DE12500105170648489890', $data['iban'] ); |
||
| 281 | $this->assertSame( 'INGDDEFFXXX', $data['bic'] ); |
||
| 282 | $this->assertSame( '0648489890', $data['konto'] ); |
||
| 283 | $this->assertSame( '50010517', $data['blz'] ); |
||
| 284 | $this->assertSame( 'ING-DiBa', $data['bankname'] ); |
||
| 285 | } ); |
||
| 286 | } |
||
| 287 | |||
| 288 | private function newComplementableFormInput(): array { |
||
| 289 | return [ |
||
| 290 | 'betrag' => '5,51', |
||
| 291 | 'zahlweise' => 'BEZ', |
||
| 292 | 'periode' => 0, |
||
| 293 | 'iban' => 'DE12500105170648489890', |
||
| 294 | 'addressType' => 'person', |
||
| 295 | 'salutation' => 'Frau', |
||
| 296 | 'title' => 'Prof. Dr.', |
||
| 297 | 'firstName' => 'Karla', |
||
| 298 | 'lastName' => 'Kennichnich', |
||
| 299 | 'street' => 'Lehmgasse 12', |
||
| 300 | 'postcode' => '12345', |
||
| 301 | 'city' => 'Einort', |
||
| 302 | 'country' => 'DE', |
||
| 303 | 'email' => '[email protected]', |
||
| 304 | ]; |
||
| 305 | } |
||
| 306 | |||
| 307 | private function getDonationFromDatabase( FunFunFactory $factory ): Donation { |
||
| 308 | $donationRepo = $factory->getEntityManager()->getRepository( Donation::class ); |
||
| 309 | $donation = $donationRepo->find( 1 ); |
||
| 310 | $this->assertInstanceOf( Donation::class, $donation ); |
||
| 311 | return $donation; |
||
| 312 | } |
||
| 313 | |||
| 314 | public function testGivenValidPayPalData_redirectsToPayPal(): void { |
||
| 315 | $client = $this->createClient(); |
||
| 316 | $client->followRedirects( false ); |
||
| 317 | |||
| 318 | $client->request( |
||
| 319 | 'POST', |
||
| 320 | '/donation/add', |
||
| 321 | $this->newValidPayPalInput() |
||
| 322 | ); |
||
| 323 | |||
| 324 | $response = $client->getResponse(); |
||
| 325 | $this->assertSame( Response::HTTP_FOUND, $response->getStatusCode() ); |
||
| 326 | $this->assertContains( 'sandbox.paypal.com', $response->getContent() ); |
||
| 327 | } |
||
| 328 | |||
| 329 | public function testWhenRedirectingToPayPal_translatedItemNameIsPassed(): void { |
||
| 330 | $client = $this->createClient(); |
||
| 331 | $client->followRedirects( false ); |
||
| 332 | |||
| 333 | $client->request( |
||
| 334 | 'POST', |
||
| 335 | '/donation/add', |
||
| 336 | $this->newValidPayPalInput() |
||
| 337 | ); |
||
| 338 | |||
| 339 | $response = $client->getResponse(); |
||
| 340 | $this->assertSame( Response::HTTP_FOUND, $response->getStatusCode() ); |
||
| 341 | $this->assertContains( 'item_name=item_name_donation', $response->getContent() ); |
||
| 342 | } |
||
| 343 | |||
| 344 | private function newValidPayPalInput(): array { |
||
| 352 | |||
| 353 | public function testGivenValidCreditCardData_redirectsToPaymentProvider(): void { |
||
| 354 | $client = $this->createClient(); |
||
| 355 | $client->request( |
||
| 356 | 'POST', |
||
| 357 | '/donation/add', |
||
| 358 | $this->newValidCreditCardInput() |
||
| 359 | ); |
||
| 360 | |||
| 361 | $response = $client->getResponse(); |
||
| 362 | $this->assertSame( Response::HTTP_FOUND, $response->getStatusCode() ); |
||
| 363 | $this->assertTrue( $response->isRedirect() ); |
||
| 364 | $this->assertContains( 'amount=1234', $response->headers->get( 'Location' ) ); |
||
| 365 | $this->assertContains( 'thatother.paymentprovider.com', $response->headers->get( 'Location' ) ); |
||
| 366 | } |
||
| 367 | |||
| 368 | public function testValidSofortInput_savesDonationAndRedirectsTo3rdPartyPage(): void { |
||
| 395 | |||
| 396 | private function newValidCreditCardInput(): array { |
||
| 404 | |||
| 405 | private function newValidSofortInput(): array { |
||
| 413 | |||
| 414 | public function testGivenInvalidRequest_formIsReloadedAndPrefilled(): void { |
||
| 442 | |||
| 443 | public function testGivenInvalidRequest_formStillContainsBannerTrackingData(): void { |
||
| 459 | |||
| 460 | public function testGivenNegativeDonationAmount_formIsReloadedAndPrefilledWithZero(): void { |
||
| 476 | |||
| 477 | private function newInvalidFormInput(): array { |
||
| 508 | |||
| 509 | public function testGivenInvalidAnonymousRequest_formIsReloadedAndPrefilled(): void { |
||
| 524 | |||
| 525 | private function newAnonymousFormInput(): array { |
||
| 533 | |||
| 534 | public function testGivenValidRequest_tokensAreReturned(): void { |
||
| 552 | |||
| 553 | public function testGivenValidRequest_clientIsRedirected(): void { |
||
| 567 | |||
| 568 | public function testWhenTrackingCookieExists_valueIsPersisted(): void { |
||
| 584 | |||
| 585 | public function testWhenTrackableInputDataIsSubmitted_theyAreStoredInSession(): void { |
||
| 604 | |||
| 605 | public function testWhenTolstojNovelIsPassed_isIsNotStoredInSession(): void { |
||
| 630 | |||
| 631 | public function testWhenParameterIsOmitted_itIsNotStoredInSession(): void { |
||
| 649 | |||
| 650 | public function testWhenInitiallyIntendedPaymentOptionsDifferFromActual_itIsReflectedInPiwikTrackingEvents(): void { |
||
| 679 | |||
| 680 | public function testWhenMobileTrackingIsRequested_piwikTrackerIsCalledForPaypalPayment(): void { |
||
| 702 | |||
| 703 | private function newValidMobilePayPalInput(): array { |
||
| 714 | |||
| 715 | public function testWhenMobileTrackingIsRequested_piwikTrackerIsNotCalledForNonPaypalPayment(): void { |
||
| 737 | |||
| 738 | public function testGivenCommasInStreetInput_donationGetsPersisted(): void { |
||
| 756 | |||
| 757 | public function testGivenSufficientForeignBankData_donationGetsPersisted(): void { |
||
| 781 | |||
| 782 | public function testCookieFlagsSecureAndHttpOnlyAreSet(): void { |
||
| 801 | |||
| 802 | public function testAllPaymentTypesAreOffered(): void { |
||
| 816 | |||
| 817 | public function testSofortPaymentTypeCanByDisabledViaQuery(): void { |
||
| 828 | |||
| 829 | } |
||
| 830 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVarassignment in line 1 and the$higherassignment in line 2 are dead. The first because$myVaris never used and the second because$higheris always overwritten for every possible time line.