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 beta] Ability to set a query param to a string of "null" when default value is null #13816

Closed
wants to merge 1 commit into from

Conversation

webark
Copy link
Contributor

@webark webark commented Jul 13, 2016

If a default query parameter is null, we should leave it as null and not switch it to a string value of "null".

This came up when in a controller we had set a default value for a query parameter of search to be null. Then if we tried to do a search for "null" it would not perform the search since it was casting the null value to a string of "null" for the default, and failed the difference check between the new query param and the default.

Didn't see any pertinent tests in the local package tests, so I added a test to the general ember routing query param test.

@webark webark force-pushed the null-query-parameters branch 2 times, most recently from 169e2df to 7c3150b Compare July 14, 2016 14:20
@webark webark force-pushed the null-query-parameters branch from 7c3150b to c7bc19c Compare July 23, 2016 02:34
@webark webark changed the title Type of null for default query parameter. [BUGFIX beta] Type of null for default query parameter. Jul 23, 2016
@webark webark changed the title [BUGFIX beta] Type of null for default query parameter. [BUGFIX beta] Setting query param to a string of "null" when default value is null Jul 23, 2016
@webark webark changed the title [BUGFIX beta] Setting query param to a string of "null" when default value is null [BUGFIX beta] Able to set a query param to a string of "null" when default value is null Jul 23, 2016
@webark webark changed the title [BUGFIX beta] Able to set a query param to a string of "null" when default value is null [BUGFIX beta] Ability to set a query param to a string of "null" when default value is null Jul 23, 2016
@webark
Copy link
Contributor Author

webark commented Aug 26, 2016

Wondering if there is anything I could do to get this merged in?

lan0 added a commit to lan0/ember.js that referenced this pull request Aug 31, 2016
`"[1]"` gets stringified in a subsequent call to `serializeQueryParam`, resulting in the serialized param being `""[1]""` instead of `"[1]"`.

This fixes emberjs#13591 and is similar to emberjs#13816.
@homu
Copy link
Contributor

homu commented Oct 4, 2016

☔ The latest upstream changes (presumably #14415) made this pull request unmergeable. Please resolve the merge conflicts.

@heat
Copy link

heat commented Oct 6, 2016

Is it the desirable behavior ? I wondering, if you really want a null value you need to just remove the parameter, right ?

@webark
Copy link
Contributor Author

webark commented Oct 6, 2016

@heat we came across it when someone named something in the system as "null" and ran a search for "null". Ember ended up removing that query paramater due to the casting that it does.

@webark webark force-pushed the null-query-parameters branch from c7bc19c to 913fa1d Compare October 6, 2016 19:50
@webark webark force-pushed the null-query-parameters branch from 913fa1d to 94fb668 Compare October 6, 2016 19:56
@homu
Copy link
Contributor

homu commented Oct 12, 2016

☔ The latest upstream changes (presumably #14433) made this pull request unmergeable. Please resolve the merge conflicts.

@webark
Copy link
Contributor Author

webark commented Jan 26, 2017

I think due to nathan's future work with serializing parameters, and the shifting around of this issue, and due to the fact that it's sad dormant for 6 months, I'm going to go ahead and close it as "unmerged".

@webark webark closed this Jan 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants