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

Fix devision by zero PHP notice #27

Merged
merged 2 commits into from Apr 1, 2019
Merged

Fix devision by zero PHP notice #27

merged 2 commits into from Apr 1, 2019

Conversation

JeroenDeDauw
Copy link
Contributor

@@ -28,6 +28,10 @@ public function validate( ValidateFeeRequest $request ): ValidateFeeResult {
$this->paymentIntervalInMonths = $request->getPaymentIntervalInMonths();
$this->applicantType = $request->getApplicantType();

if ( $this->paymentIntervalInMonths === 0 ) {
Copy link
Member

@gbirke gbirke Mar 29, 2019

Choose a reason for hiding this comment

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

Maybe make this < 1 instead, as negative intervals make no sense here, but are the default value on the client side (although wmde/fundraising-application#1452 makes sure we don't send it any more).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't forget the test case if you do :)

The client side validation prevents negative intervals from occuring now,
but they should be caught here as well, just in case.

https://phabricator.wikimedia.org/T215163
@moiikana moiikana merged commit e4b1697 into master Apr 1, 2019
@moiikana moiikana deleted the division-by-zero branch April 1, 2019 11:34
@JeroenDeDauw
Copy link
Contributor Author

This is definitely fine, though I'd have extracted the request creation into some newValidRequestWithInterval( $interval ) private function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants