Do not treat 0 as boolean value in the REST controller #1276
Conversation
|
This is now overlapping with #1267 by @miguelvaara - there are changes to the same lines in ConceptSearchParameters.getSearchTerm. These need to be consolidated somehow. Maybe simply by merging one first, then fixing the conflicts in the other. |
|
Rebased, GH Actions working 🧰 thanks @osma ! |
Codecov Report
@@ Coverage Diff @@
## master #1276 +/- ##
============================================
+ Coverage 69.33% 69.36% +0.02%
- Complexity 1648 1649 +1
============================================
Files 32 32
Lines 4047 4047
============================================
+ Hits 2806 2807 +1
+ Misses 1241 1240 -1
Continue to review full report at Codecov.
|
|
The code looks good - I think the easiest way forward is to merge this first and then sort out the conflicts in PR #1267, as that one is still a bit WIP (at least the PR description is outdated). The only thing I wonder about is the title of this PR (and the commit message) which says "Do not treat 0 as truthy value". Isn't the point actually the opposite - "Do not treat 0 as a non-truthy (i.e. false) value"? |
kinow
changed the title
|
You're right! s/truthy/boolean , WDYT? |
|
Sounds good! |
| @@ -86,6 +86,26 @@ public function testGetSearchTerm() { | |||
| $this->assertEquals('test', $params->getSearchTerm()); | |||
| } | |||
|
|
|||
| /** | |||
| * For https://github.com/NatLibFi/Skosmos/issues/1275, to verify | |||
| * that querying for `0` (zero) does not evaluate it as a truthy | |||
There was a problem hiding this comment.
Oh, missed that one. Just a sec.
There was a problem hiding this comment.
Done. That should be the last one ™
|
SonarCloud Quality Gate failed.
|
|
Will do one final test round and then merge! |
|
Works like a charm! Thanks a lot @kinow ! |
Reasons for creating this PR
Prevent
0being treated as a truthy value in the REST controller. Complements #1261Link to relevant issue(s), if any
Description of the changes in this PR
Same fix as #1261
Known problems or uncertainties in this PR
None.
Checklist