This project is archived and is in readonly mode.

#3122 ✓stale
Lennart Koopmann

Support of OR as condition connector in merge_conditions

Reported by Lennart Koopmann | August 31st, 2009 @ 06:15 PM

This patch allows you to choose between AND or OR as condition connector in merge_conditions. No need to change your code if you don't want to use OR as connector with the new patched method.

Comments and changes to this ticket

  • Kane

    Kane September 1st, 2009 @ 12:19 AM

    interessing.. but i dont like the syntax :connector => "OR" cant it be a symbol?
    and just defaulting to and if its nor and or or feels strange, the developer should be noticed about his error otherwise he wonders why he didnt get back what he expected.

  • Lennart Koopmann

    Lennart Koopmann September 1st, 2009 @ 01:18 PM

    I made the :connector parameter optional because currently AND is the standard connector. I don't know what the usual way for Rails is but my experience says that it's better to keep the default behaviour and add the new function as an not required option.

    I'll create a new patch that makes the :connector a symbol.

  • Kane

    Kane September 1st, 2009 @ 02:56 PM

        # Only allow AND or OR as connector. Default to AND.
        connector = "AND" unless connector == "AND" or connector == "OR"
    

    i meant this line of code, if i put some other thing into it, say:

    Product.merge_conditions :conditions => conditions, :connector => :xor

    i end up with the connector "and", but i specified "xor".

    you are absolutely right it doesnt makes any sense to require the connector option and it should indeed default to 'and', but if i explicitly specify it, it should not simply be set to 'and' cause its not 'and' or 'or'.

    There is more then 'and' and 'or' and i would suggest just setting it to 'and' if no connector is set else just use the passed operator.

  • Lennart Koopmann

    Lennart Koopmann September 1st, 2009 @ 03:31 PM

    Ah, okay. That's true. I'll also change that in the next patch. :)

  • Lennart Koopmann
  • Lennart Koopmann

    Lennart Koopmann September 1st, 2009 @ 04:15 PM

    Previous patch deleted. Here is the new one (Changed email address)

  • Josh Sharpe

    Josh Sharpe September 1st, 2009 @ 08:12 PM

    -1 applies cleanly to master
    breaks existing tests
    no new tests

  • Lennart Koopmann

    Lennart Koopmann September 2nd, 2009 @ 09:46 AM

    All the tests run fine for me. Which fail on your system?

  • CancelProfileIsBroken

    CancelProfileIsBroken September 25th, 2009 @ 12:37 PM

    • Tag changed from patch to bugmash, patch
  • Rizwan Reza

    Rizwan Reza January 21st, 2010 @ 07:09 AM

    • Tag changed from bugmash, patch to activerecord, feature, patch
    • State changed from “new” to “open”

    The patch applies with some whitespace errors. Please provide tests also. Thanks!

  • Kane

    Kane June 15th, 2010 @ 06:26 PM

    this is no longer relevant since arel or is it?
    I think it can be closed.

  • Ryan Bigg

    Ryan Bigg October 11th, 2010 @ 10:56 AM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer October 12th, 2010 @ 06:58 AM

    • Title changed from “[PATCH] Support of OR as condition connector in merge_conditions” to “Support of OR as condition connector in merge_conditions”
    • Tag set to activerecord, patch

    Using the "patch" tag instead of prefixing the ticket title with "[PATCH]" to make sure patched tickets end up in the open patches bin. :)

  • Santiago Pastorino

    Santiago Pastorino February 9th, 2011 @ 12:31 AM

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 9th, 2011 @ 12:31 AM

    • State changed from “open” to “stale”
  • bingbing

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Pages