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

Allow to map field to multiple columns #31

Merged
merged 1 commit into from
Nov 29, 2016
Merged

Allow to map field to multiple columns #31

merged 1 commit into from
Nov 29, 2016

Conversation

sstok
Copy link
Member

@sstok sstok commented Nov 28, 2016

This is known as the combined field search, eg. name will search in both first and last name.

The OnNotSuccessfulTrait needs to be solved in another pull request (pior to merging this pr).
And documentation needs to be updated.

Note: The processFieldValues can be optimized to store all fields as combined-field and removing the is_array check. BUT! This will break with the ORM processor that reuses this class, and even updating the ORM processor will not prevent older versions of the ORM processor to install this version of the DBAL package (with breaking changes). I would rather not take this risk, and leave this as a to-do for RollerworksSearch 2.0 👍

I'm really happy with how easy it turned out to be 😄

@@ -52,7 +53,7 @@
*
* @param Connection $connection
* @param QueryPlatformInterface $queryPlatform
* @param QueryField[] $fields
* @param array $fields
Copy link
Member Author

Choose a reason for hiding this comment

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

All CS will be fixed after merging as there are also other CS problems that need to be resolved.

// Initialize from the search-field configuration.
if ([] === $mappings) {
/** @var array $mappings */
$mappings = $fieldConfig->getOption('doctrine_dbal_mappings', []);
Copy link
Member Author

Choose a reason for hiding this comment

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

This option needs to be added, tested and documented.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually removed this as there are no related mapping options for DBAL.
And Model mapping is deprecated in the core package.

['column' => 'last_name', 'type' => 'string', 'alias' => 'c'],
]);

$whereBuilder->setCombinedField('customer-name', [
Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicate code.

This is known as the combined field search, eg. name will search in both
first and last name.
@sstok sstok merged commit 8e2a5d5 into rollerworks:master Nov 29, 2016
@sstok sstok deleted the combined-fields branch November 29, 2016 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant