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

bugfix: + takes precedence over ternary operator #77

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

xiaoronglv
Copy link
Contributor

Problem

when the base URL contains parameters, the params argument will be ignored.

image

How to reproduce?

URL: https://api.checkr-staging.com/v1/nodes?include=packages
parameter: {page: 2}

Root cause

Ruby operator + takes precedence over ? :, when URI.parse returns true, only & will be returned.

 url += URI.parse(url).query ? '&' : '?' + Util.query_string(params)

Here is the execution plan

 url += if URI.parse(url).query 
  '&' 
 else
'?' + Util.query_string(params)

How to fix it?

solution 1: use the parenthesis to correct the precedence.

 url += (URI.parse(url).query ? '&' : '?') + Util.query_string(params)

solution2: change the code to two lines.

Changes

in this PR, we use solution2 to fix the bug.

@xiaoronglv xiaoronglv changed the title bugfix: + takes precedence over ternary bugfix: + takes precedence over ternary operator Aug 29, 2022
@akshay-8d66 akshay-8d66 merged commit 46e54a8 into checkr:master Aug 30, 2022
@akshay-8d66
Copy link
Collaborator

Thanks for this fix!

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

Successfully merging this pull request may close these issues.

None yet

2 participants