Where group by dpDesignz · Pull Request #186 · ezSQL/ezsql · GitHub

Resolves #164

UPDATE: There are no longer breaking changes

Example code from pdo\pdo_mysqlTest.php file:

$db->selecting(
    'unit_test', 
    '*', 
    where(
        eq('active', '1'),
        whereGroup(
           like('test_key', '%1%', _OR),
           like('test_key', '%3%')
        )
    )
);

or another example without the function

$db->selecting(
    'unit_test', 
    '*', 
    where(
        eq('active', '1'),
        like('test_key', '%1%', _OR, '('),
        like('test_key', '%3%', null, ')')
    )
);

The combiner will add itself at the end of the group.

Created the initial function but came across an issue where the $whereClause doesn't work in the $where method when using an OR combiner, so need to fix that first

Methods and functions complete but failed testing in phpunit so needing to fix

PHPUnit tests succeeded.

Cleaned up trailing combiner in tests

Method eq has 5 arguments (exceeds 4 allowed). Consider refactoring.

All these bring in another required field? Whereas ...$args already covers that. You should try adding additional logic in the other methods instead to get the same results you are after.

The logic could be some kind of token/trigger word, to extract to get the desired action.
This is far beyond a breaking change, unnecessary, the logic should include a way for the current code to continue to work. The current tests should still pass.

Method neq has 5 arguments (exceeds 4 allowed). Consider refactoring.

Method ne has 5 arguments (exceeds 4 allowed). Consider refactoring.

Method lt has 5 arguments (exceeds 4 allowed). Consider refactoring.

Method lte has 5 arguments (exceeds 4 allowed). Consider refactoring.

you will need to come up with a solution for the bc, to have tests pass as is.

Changed from breaking changes to use the existing extra `$args`. All existing PHPUnit tests run without failing.

Function flattenWhereConditions has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

This function should be in the class method being used in. It's a private helper function that should not be called directly by user/developer.

Very good point. I have moved this to the ezQuery file as a private method.

Function whereGroup has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

What you think about just calling it group or grouping?
The only place it can be used in is with where already.
Repeating the same word within the same function call trying to stay away from, and keep naming somewhat similar to regular SQL dialect.

Good idea. Have updated the code and tests. I didn't want to call it group as that felt too close to groupBy for me and I thought it might get confused, but I like grouping.

Incorrectly made a modification to the return of the `where` method instead of the `whereGroup` method

you will need to come up with a solution for the bc, to have tests pass as is.

Thanks, I didn't even really think of using the existing $args, that makes way more sense. It took me most of the day to work out how it worked in the first place. Definitely learned a lot. 😄

Updated the code to use the existing options to avoid breaking changes.

You will need to re-sync with master branch, the CI travis and appveyor systems and github see a conflicting file, and Travis still has a fail.

Synced with master branch to reduce conflicts

You will need to re-sync with master branch, the CI travis and appveyor systems and github see a conflicting file, and Travis still has a fail.

Thanks. Still learning how this all works with open source and larger projects. I've only really used GitHub for small personal projects until recently.

Both CI checks passed :)

Changed the method from `whereGroup` to `grouping` and moved the flatten function to be a private method in the `ezQuery` file as per @techno-express recommendations.

Function grouping has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

Added a brief example in the readme file underneath the shortcut methods. Will add a detailed example in the wiki once approved.

dpDesignz added 4 commits April 21, 2020 15:37

Initial function creation

a283b6d

Created the initial function but came across an issue where the $whereClause doesn't work in the $where method when using an OR combiner, so need to fix that first

Testing phase

009a75d

Methods and functions complete but failed testing in phpunit so needing to fix

Tests completed

b50ca9d

PHPUnit tests succeeded.

Cleaned up trailing in tests

6ef9eff

Cleaned up trailing combiner in tests

qlty-cloud-legacy Bot reviewed Apr 21, 2020

View reviewed changes

Changed from breaking changes to user $args

cc7a8cd

Changed from breaking changes to use the existing extra `$args`. All existing PHPUnit tests run without failing.

qlty-cloud-legacy Bot reviewed Apr 22, 2020

View reviewed changes

Fixed incorrect return change

fd67871

Incorrectly made a modification to the return of the `where` method instead of the `whereGroup` method

dpDesignz added 3 commits April 23, 2020 11:49

Merge remote-tracking branch 'upstream/master' into where-group

97b18b3

Synced with master for conflicts

b35291a

Synced with master branch to reduce conflicts

Synced with master for conflicts

1637812

Synced with master branch to reduce conflicts

Changed to grouping

f990c88

Changed the method from `whereGroup` to `grouping` and moved the flatten function to be a private method in the `ezQuery` file as per @techno-express recommendations.

qlty-cloud-legacy Bot reviewed Apr 23, 2020

View reviewed changes

Added brief example to the readme

8b83dd7

Added a brief example in the readme file underneath the shortcut methods. Will add a detailed example in the wiki once approved.

TheTechsTech merged commit c843017 into ezSQL:master Apr 24, 2020

                    * Creates an equality comparison expression with the given arguments.
                    */
-                  function eq($x, $y, $and = null, ...$args)
+                  function eq($x, $y, $and = null, $group = null, ...$args)

                    * Creates a non equality comparison expression with the given arguments.
                    */
-                  function neq($x, $y, $and = null, ...$args)
+                  function neq($x, $y, $and = null, $group = null, ...$args)

                    * Creates the other non equality comparison expression with the given arguments.
                    */
-                  function ne($x, $y, $and = null, ...$args)
+                  function ne($x, $y, $and = null, $group = null, ...$args)

                    * Creates a lower-than comparison expression with the given arguments.
                    */
-                  function lt($x, $y, $and = null, ...$args)
+                  function lt($x, $y, $and = null, $group = null, ...$args)

                    * Creates a lower-than-equal comparison expression with the given arguments.
                    */
-                  function lte($x, $y, $and = null, ...$args)
+                  function lte($x, $y, $and = null, $group = null, ...$args)

                           : false;
                   }
+                  function flattenWhereConditions($whereConditions)

+                      }
+                  }
+                  public function whereGroup(...$whereConditions)

+                      }
+                  }
+                  public function grouping(...$whereConditions)

Where group by dpDesignz · Pull Request #186 · ezSQL/ezsql · GitHub

Uh oh!

Where group - #186

Where group#186TheTechsTech merged 11 commits intoezSQL:masterezSQL/ezsql:masterfrom dpDesignz:where-groupdpDesignz/ezsql:where-groupCopy head branch name to clipboard

Conversation

dpDesignz commented Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qlty-cloud-legacy Bot Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

TheTechsTech Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy Bot Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy Bot Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy Bot Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy Bot Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

TheTechsTech commented Apr 21, 2020

Uh oh!

qlty-cloud-legacy Bot Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

TheTechsTech Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

dpDesignz Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy Bot Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

TheTechsTech Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

dpDesignz Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

dpDesignz commented Apr 22, 2020

Uh oh!

TheTechsTech commented Apr 22, 2020

Uh oh!

dpDesignz commented Apr 22, 2020

Uh oh!

qlty-cloud-legacy Bot Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Where group#186
TheTechsTech merged 11 commits into
ezSQL:masterfrom
dpDesignz:where-group

dpDesignz commented Apr 21, 2020 •
edited
Loading