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

Feat query gen 2.2 #162

Closed
wants to merge 27 commits into from
Closed

Feat query gen 2.2 #162

wants to merge 27 commits into from

Conversation

stnguyen90
Copy link
Contributor

@stnguyen90 stnguyen90 commented Aug 8, 2022

Based on feedback in previous PR, update Query class to be used like:

// signature of constructor is (string $method, string $attribute = '', array $values = [])
$query = new Query('equal', 'actors', ['Brad Pitt', 'Johnny Depp']);

$this->assertEquals('equal', $query->getMethod());
$this->assertEquals('actors', $query->getAttribute());
$this->assertEquals('Brad Pitt', $query->getValues()[0]);

// other examples
$query = new Query('lessThan', 'score', [10]);
$query = new Query('orderDesc', 'score');
$query = new Query('limit', values: [10]);

// static helper methods
$query = Query::equal('title', ['Iron Man']); // the only helper method that accepts arrays
$query = Query::greaterThan('score', 10); // value is not array
$query = Query::search('search', 'John Doe'); // also not array
$query = Query::orderAsc('score');
$query = Query::limit(10);
$query = Query::cursorAfter('cursor');

Update parsing of queries to be like:

$query = Query::parse('equal("actors", ["Brad Pitt", "Johnny Depp"])');

$this->assertEquals('equal', $query->getMethod());
$this->assertEquals('actors', $query->getAttribute());
$this->assertEquals("Brad Pitt", $query->getValues()[0]);
$this->assertEquals("Johnny Depp", $query->getValues()[1]);

$query = Query::parse('equal("actors", "Brad Pitt")');

$this->assertEquals('equal', $query->getMethod());
$this->assertEquals('actors', $query->getAttribute());
$this->assertEquals("Brad Pitt", $query->getValues()[0]);

@stnguyen90 stnguyen90 marked this pull request as draft August 8, 2022 22:11
/**
* Helper method to create Query with contains method
*/
public static function contains(string $attribute, $value): self
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should contains accept array $values or 1 $value? Probably an array, right? Since you may want to check if the array attribute contains value1 or value2?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An array sounds good to me

/**
* Helper method to create Query with search method
*/
public static function search(string $attribute, $value): self
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, we can do an OR search by passing 1 string with each value separated by space. Should this signature be changed to array $values, though, and then we do implode(' ', $values) server side?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On MYSQL Fulltext can take a few attributes such as
SELECT * FROM collection WHERE MATCH(column1, column2) AGAINST ('text')
I know today there is no support, only a single attribute

/**
* Helper method to create Query with cursorBefore method
*/
public static function cursorBefore(string $value): self
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this string $value should be Document $value, right?

/**
* Helper method to create Query with orderDesc method
*/
public static function orderDesc(string $attribute): self
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some of our test cases, we order by "natural order" where we specify direction but no attribute. Is it to weird to do Query::orderDesc('') for those cases?

@stnguyen90 stnguyen90 mentioned this pull request Aug 9, 2022
@stnguyen90 stnguyen90 closed this Aug 11, 2022
@stnguyen90 stnguyen90 deleted the feat-query-gen-2.2 branch August 14, 2022 06:55
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.

4 participants