[Validator] New `DivisibleBy` constraint for testing divisibility #28069

colinodell · 2018-07-26T19:38:16Z

This introduces a new ~~MultipleOf~~ DivisibleBy constraint which checks whether one number is a multiple of (aka "divisible by") some other number. Useful for enforcing specific increments on a number.

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	symfony/symfony-docs#10121

See symfony/symfony-docs#10121 for examples of this constraint in action.

colinodell · 2018-07-26T19:57:40Z

The AppVeyor failure seems to be unrelated to this change.

weaverryan

Nice job Colin! I just tested this in a real project - no missing pieces - it works great :).

colinodell · 2018-08-09T19:28:25Z

Thanks Ryan! I'm currently using this in an application with a "time spent" field which only accepts 0.25 hour increments. I feel like having a modulus == 0 constraint like this would round out Symfony's other comparison constrains nicely.

nicolas-grekas · 2018-08-10T06:46:01Z

What about renaming to DivisibleBy? I first read MultipleOf as something like SeveralOf, which made no sense.

xabbuh · 2018-08-10T07:45:59Z

I agree with Nicolas. And since the test in Twig is named the same that would feel more consistent.

colinodell · 2018-08-10T12:28:56Z

Thanks for the feedback @nicolas-grekas! I have renamed the constraint and updated the docs PR accordingly.

xabbuh · 2018-08-10T13:25:18Z

src/Symfony/Component/Validator/Constraints/DivisibleBy.php

+        self::NOT_DIVISIBLE_BY => 'NOT_MULTIPLE_OF',
+    );
+
+    public $message = 'This value should be divisible by {{ compared_value }}.';


Would be nice to also add translations for this string.

The message is also not all that clear from a user's perspective. To a developer "8 is divisible by 2" makes sense, but a mathematician would say "so is 7.6543", and a common user would just be like "erm keep the math out of my way".

I'd stick with the more generic This value must be a multiple of {{ compared_value }}. for the end-user feedback.

I have updated the message as requested and force-pushed the changes.

I have also added translation strings in English and Spanish.

curry684 · 2018-08-10T13:37:24Z

src/Symfony/Component/Validator/Constraints/DivisibleBy.php

+    const NOT_DIVISIBLE_BY = '6d99d6c3-1464-4ccf-bdc7-14d083cf455c';
+
+    protected static $errorNames = array(
+        self::NOT_DIVISIBLE_BY => 'NOT_MULTIPLE_OF',


Forgot to update the constant value.

Fixed and force-pushed. Thanks for catching this!

curry684 · 2018-08-13T12:50:21Z

For Dutch (nl):

            <trans-unit id="84">
                <source>This value should be a multiple of {{ compared_value }}.</source>
                <target>Deze waarde moet een veelvoud zijn van {{ compared_value }}.</target>
            </trans-unit>

dmaicher · 2018-08-13T13:07:55Z

For German (de):

Dieser Wert sollte ein Vielfaches von {{ compared_value }} sein.

xabbuh · 2018-08-13T13:16:32Z

@dmaicher Though we need to capitalise the V (see https://www.duden.de/rechtschreibung/Vielfaches):

Dieser Wert sollte ein Vielfaches von {{ compared_value }} sein.

colinodell · 2018-08-13T13:31:25Z

I have added the two translations (but now fabbot is failing due to an existing translation)

curry684 · 2018-08-13T13:37:10Z

The correction is wrong. @fabpot you should probably exempt .xlf files from typo checks in fabbot if it only searches for English typos 😉

nicolas-grekas · 2018-08-14T15:51:43Z

src/Symfony/Component/Validator/Constraints/DivisibleByValidator.php

+     */
+    protected function compareValues($value1, $value2)
+    {
+        return 0 == fmod($value1, $value2);


let's use strict comparison here

nicolas-grekas

For french:
Cette valeur doit être un multiple de {{ compared_value }}.

colinodell · 2018-08-14T19:19:00Z

Thanks for the feedback @nicolas-grekas! I have made the comparison strict and added the French translation.

…ivisibility (colinodell) This PR was squashed before being merged into the 4.2-dev branch (closes #28069). Discussion ---------- [Validator] New `DivisibleBy` constraint for testing divisibility This introduces a new ~`MultipleOf`~ `DivisibleBy` constraint which checks whether one number is a multiple of (aka "divisible by") some other number. Useful for enforcing specific increments on a number. | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | symfony/symfony-docs#10121 See symfony/symfony-docs#10121 for examples of this constraint in action. Commits ------- efcfb8b [Validator] New `DivisibleBy` constraint for testing divisibility

…ell) This PR was merged into the master branch. Discussion ---------- [Validator] Document the DivisibleBy constraint Documentation for the new ~`MultipleOf`~ `DivisibleBy` constraint proposed in symfony/symfony#28069 Commits ------- 3270cca Document the DivisibleBy constraint

carsonbot added Status: Needs Review Feature labels Jul 26, 2018

colinodell mentioned this pull request Jul 26, 2018

[Validator] Document the DivisibleBy constraint symfony/symfony-docs#10121

Merged

colinodell force-pushed the feature/multiple-of-validator branch from b96bb3a to f8cf896 Compare July 26, 2018 19:39

colinodell changed the title ~~Implement validation constraint for testing whether divisibility~~ [Validator] New MultipleOf constraint for testing divisibility Jul 26, 2018

colinodell force-pushed the feature/multiple-of-validator branch from f8cf896 to 36e369e Compare July 26, 2018 19:41

javiereguiluz added the Validator label Jul 27, 2018

javiereguiluz added this to the next milestone Jul 27, 2018

weaverryan approved these changes Aug 9, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Aug 9, 2018

colinodell changed the title ~~[Validator] New MultipleOf constraint for testing divisibility~~ [Validator] New DivisibleBy constraint for testing divisibility Aug 10, 2018

xabbuh approved these changes Aug 10, 2018

View reviewed changes

curry684 suggested changes Aug 10, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Aug 10, 2018

colinodell force-pushed the feature/multiple-of-validator branch 4 times, most recently from bb9feb7 to 6eff196 Compare August 10, 2018 17:03

xabbuh added Status: Needs Review and removed Status: Needs Work labels Aug 13, 2018

nicolas-grekas reviewed Aug 14, 2018

View reviewed changes

carsonbot removed the Status: Needs Review label Aug 14, 2018

nicolas-grekas approved these changes Aug 14, 2018

View reviewed changes

carsonbot added the Status: Reviewed label Aug 14, 2018

[Validator] New DivisibleBy constraint for testing divisibility

efcfb8b

nicolas-grekas force-pushed the feature/multiple-of-validator branch from 3993781 to efcfb8b Compare August 15, 2018 15:44

nicolas-grekas merged commit efcfb8b into symfony:master Aug 15, 2018

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

This was referenced Nov 3, 2018

Release v4.2.0-BETA1 #29072

Closed

Release v4.2.0-BETA1 #29073

Merged

curry684 mentioned this pull request Feb 12, 2019

[Validator] Add the missing translations for the Dutch ("nl") locale #30177

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Validator] New `DivisibleBy` constraint for testing divisibility #28069

[Validator] New `DivisibleBy` constraint for testing divisibility #28069

colinodell commented Jul 26, 2018 •

edited

Loading

colinodell commented Jul 26, 2018

weaverryan left a comment

colinodell commented Aug 9, 2018

nicolas-grekas commented Aug 10, 2018 •

edited

Loading

xabbuh commented Aug 10, 2018

colinodell commented Aug 10, 2018

xabbuh Aug 10, 2018

curry684 Aug 10, 2018 •

edited

Loading

colinodell Aug 10, 2018

colinodell Aug 10, 2018

curry684 Aug 10, 2018

colinodell Aug 10, 2018

curry684 commented Aug 13, 2018

dmaicher commented Aug 13, 2018 •

edited

Loading

xabbuh commented Aug 13, 2018

colinodell commented Aug 13, 2018

curry684 commented Aug 13, 2018

nicolas-grekas Aug 14, 2018

nicolas-grekas left a comment

colinodell commented Aug 14, 2018

[Validator] New DivisibleBy constraint for testing divisibility #28069

[Validator] New DivisibleBy constraint for testing divisibility #28069

Conversation

colinodell commented Jul 26, 2018 • edited Loading

colinodell commented Jul 26, 2018

weaverryan left a comment

Choose a reason for hiding this comment

colinodell commented Aug 9, 2018

nicolas-grekas commented Aug 10, 2018 • edited Loading

xabbuh commented Aug 10, 2018

colinodell commented Aug 10, 2018

xabbuh Aug 10, 2018

Choose a reason for hiding this comment

curry684 Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

colinodell Aug 10, 2018

Choose a reason for hiding this comment

colinodell Aug 10, 2018

Choose a reason for hiding this comment

curry684 Aug 10, 2018

Choose a reason for hiding this comment

colinodell Aug 10, 2018

Choose a reason for hiding this comment

curry684 commented Aug 13, 2018

dmaicher commented Aug 13, 2018 • edited Loading

xabbuh commented Aug 13, 2018

colinodell commented Aug 13, 2018

curry684 commented Aug 13, 2018

nicolas-grekas Aug 14, 2018

Choose a reason for hiding this comment

nicolas-grekas left a comment

Choose a reason for hiding this comment

colinodell commented Aug 14, 2018

[Validator] New `DivisibleBy` constraint for testing divisibility #28069

[Validator] New `DivisibleBy` constraint for testing divisibility #28069

colinodell commented Jul 26, 2018 •

edited

Loading

nicolas-grekas commented Aug 10, 2018 •

edited

Loading

curry684 Aug 10, 2018 •

edited

Loading

dmaicher commented Aug 13, 2018 •

edited

Loading