Skip to content
This repository has been archived by the owner on Mar 29, 2019. It is now read-only.

[#698] Fix issue queries on Ruby 1.9 #123

Merged
merged 1 commit into from
Nov 25, 2011

Conversation

mbreit
Copy link
Contributor

@mbreit mbreit commented Nov 16, 2011

This should fix the issue query problem described in #698. Query#values_for(field) returns an array which is passed to the text_field_tag helper method. This works on Ruby 1.8, because ["test"].to_s returns test, but on Ruby 1.9 it returns ["test"].

Using the first element of this array as parameter for text_field_tag seems better than relying on Array#to_s.

I did a quick test on Ruby 1.8.7 as well, it seems to work but it should be tested by someone else as well.

@thegcat
Copy link
Member

thegcat commented Nov 16, 2011

Mmh, I'm not a huge fan on #try, any way to make it equally painless without try?

@thegcat
Copy link
Member

thegcat commented Nov 16, 2011

(and thanks for the submission :-) )

@mbreit
Copy link
Contributor Author

mbreit commented Nov 16, 2011

Query#values_for might return nil, so we have to check for that:

  def values_for(field)
    has_filter?(field) ? filters[field][:values] : nil
  end

We could write query.values_for(field).nil? ? nil : query.values_for(field) which (imho) reduces the readability and is exactly what try does. Another alternative would be to introduce a new variable, which is even worse from a readability point of view. It might make sense to change Query#values_for to return an empty array instead of nil, but I would try to avoid changing the API if there is no good test coverage.

But this is only my opinion and there are probably some other options which I don't see.

If you have another good idea or just another opinion, I will change it.

@thegcat
Copy link
Member

thegcat commented Nov 25, 2011

The output of Query#values_for isn't really satisfactory and I'd have liked it better if the method presented correctly-formated values, but that's for another refactoring I guess. Thanks for the contribution!

thegcat added a commit that referenced this pull request Nov 25, 2011
[#698] Fix single-value query atoms in issue queries on Ruby 1.9
@thegcat thegcat merged commit a80f822 into chiliproject:master Nov 25, 2011
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
mattconnolly pushed a commit to mattconnolly/chiliproject that referenced this pull request Apr 17, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants