Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

use correct value when using virtualFields in conditions and IN () #897

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
4 participants
Member

ceeram commented Oct 11, 2012

No description provided.

@dereuromark dereuromark and 1 other commented on an outdated diff Oct 11, 2012

lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php
@@ -1103,4 +1103,40 @@ public function testBuildJoinStatement($join, $expected) {
$this->assertEquals($expected, $result);
}
+/**
+ * Test conditionKeysToString() with virtual field
+ *
+ * @return void
+ */
+ public function testconditionKeysToStringVirtualField() {
@dereuromark

dereuromark Oct 11, 2012

Member

testC (capital C?)

@ceeram

ceeram Oct 11, 2012

Member

fixed, thx

Owner

markstory commented Oct 12, 2012

This looks reasonable to me, I don't know where the IN is coming from now. But DboSource is twisty enough that there is probably a few more hiding in other parts of the code.

Member

ADmad commented Oct 12, 2012

@ceeram Please just ensure there are existing tests for IN for normal db fields so that there is no regression.

Member

ceeram commented Oct 12, 2012

Will do
Op 12 okt. 2012 09:52 schreef "ADmad" notifications@github.com het
volgende:

@ceeram https://github.com/ceeram Please just ensure there are existing
tests for IN for normal db fields so that there is no regression.


Reply to this email directly or view it on GitHubhttps://github.com/cakephp/cakephp/pull/897#issuecomment-9368977.

Member

ceeram commented Nov 1, 2012

added test for normal fields as well

Member

ceeram commented Nov 1, 2012

broken tests, needs to be fixed before merge!

Member

ceeram commented Nov 1, 2012

So this test breaks:

    $result = $this->Dbo->conditions(array('score' => array(2 => 1, 2, 10)));
    $expected = " WHERE score IN (1, 2, 10)";
  1. MysqlTest::testStringConditionsParsing
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -' WHERE score IN (1, 2, 10)'
    +' WHERE score IN (1, 2, 10)'

Is this really breaking it? i would think it fixes an incorrect current behavior

Member

ADmad commented Nov 1, 2012

Yeah doesn't look like a fail. Having the score field quoted is probably expected.

Owner

markstory commented Nov 1, 2012

Seems like a fail that shouldn't count as a real fail.

Member

ceeram commented Nov 1, 2012

ill fix the tests then :)

Member

ceeram commented Nov 2, 2012

fixed the tests

Member

ceeram commented Nov 2, 2012

Merged as single commit: 093275a

@ceeram ceeram closed this Nov 2, 2012

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