Fix REST search when using zero ('0') as the query parameter

[kinow]

Reasons for creating this PR

Prevent a string value being treated as a boolean, causing a valid search query to fail.

Link to relevant issue(s), if any

• Closes Vocabulary search using "0" as the query argument throws an error #1260

Description of the changes in this PR

1. added a unit test with some cases where it should pass, and where it should fail. It's using parameters with phpunit, so this can be extended later if needed to cover more cases.
2. fixed the code by using isset() and also trimming the value, and checking if the strlen returned is zero.
3. happy to revert, but the invalid return's have always bothered me in the controller methods 😥 so I also fixed that by using return, and annotated the params & method changed with types (I think our minimum version of PHP accepts it? if not, and CI fails, I'll revert this part)

Known problems or uncertainties in this PR

Checklist

• phpUnit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if not, explain why below)
• The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

[kinow]

SoanrCloud analysis is failing due to duplicated code, but as far as I can tell, none is in this PR:

[osma]

added a unit test with some cases where it should pass, and where it should fail. It's using parameters with phpunit, so this can be extended later if needed to cover more cases.

Great work!

fixed the code by using isset() and also trimming the value, and checking if the strlen returned is zero.

Sounds like a belt-and-suspenders approach but this seems to work fine!

happy to revert, but the invalid return's have always bothered me in the controller methods disappointed_relieved so I also fixed that by using return, and annotated the params & method changed with types (I think our minimum version of PHP accepts it? if not, and CI fails, I'll revert this part)

Yes, these are excellent improvements! Adding type annotations is good whenever you are editing Skosmos code and these are not problematic - the minimum PHP version is currently 7.2 and it already has good support for type annotations (7.0 was a bit lacking IIRC).

I'm not sure why the GitHub Actions CI jobs don't seem to have run for this PR. I can try to execute them some other way.

As pointed out in this comment, a very similar problem affects also searches done via the web UI. This PR takes care of the REST side, but not the web UI searches. So I think we need another PR to fix that, but this is already a great improvement!

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
3.1% Duplication

[codecov]

Codecov Report

Merging #1261 (aff32dd) into master (b7fd768) will decrease coverage by 0.00%.
The diff coverage is 55.55%.

@@             Coverage Diff              @@
##             master    #1261      +/-   ##
============================================
- Coverage     69.33%   69.33%   -0.01%     
- Complexity     1647     1648       +1     
============================================
  Files            32       32              
  Lines          4044     4047       +3     
============================================
+ Hits           2804     2806       +2     
- Misses         1240     1241       +1     

Impacted Files	Coverage Δ
controller/RestController.php	22.75% <55.55%> (+0.23%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7fd768...aff32dd. Read the comment docs.

[osma]

I still don't understand why CI actions aren't running. I reviewed the settings for this repository and there should be an approval process for first-time contributors, but I'm not seeing the approval button...

I added a very minor typo fix comment to @kinow's PR branch, but that didn't trigger it either. Then I pushed the same branch into NatLibFi/Skosmos:kinow-fix-rest-search-zero . That triggered a CI run which finished successfully.

All seems to be good, I'm merging this PR. Thanks again @kinow!