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

Autcomplete feature #149

Closed
wants to merge 6 commits into from

Conversation

joshlopes
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? ?
Deprecations? ?
Tests pass? ?
Fixed tickets
License yes
Doc PR yes

joshlopes added 5 commits May 26, 2015 10:50
…hAdmiClass on the fixFieldDescription as ORM -- Added getFieldName on Filter to respect the parents associations mappings
Conflicts:
	Builder/DatagridBuilder.php
	Filter/ModelFilter.php
	composer.json
@joshlopes
Copy link
Contributor Author

This comes from the old PR #137 , please review & if possible fix tests in the new merge thingy or i will fix it later when i get myself some time.

@dunglas this SonataCI - Quick Check is new ? is missing something on the project for it to work ?

@soullivaneuh
Copy link
Member

this SonataCI - Quick Check is new ?

Yes, new tools under testing. Check for Travis for now.

CS need to be fixed, please rebase your PR to make travis green.

@@ -116,6 +121,16 @@ public function addFilter(DatagridInterface $datagrid, $type = null, FieldDescri
$admin->addFilterFieldDescription($fieldDescription->getName(), $fieldDescription);

$fieldDescription->mergeOption('field_options', array('required' => false));

if ($type === 'doctrine_mongo_autocomplete') {
Copy link
Member

Choose a reason for hiding this comment

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

Yoda condition.

@joshlopes
Copy link
Contributor Author

@soullivaneuh done.

@@ -19,17 +19,35 @@
protected $active = false;

/**
* {@inheritdoc}
* @param mixed $queryBuilder
Copy link
Member

Choose a reason for hiding this comment

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

Why removing inheritdoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason, should i put it back ?

Copy link
Member

Choose a reason for hiding this comment

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

Of course. If this method as a parent, it must have this tag.

@soullivaneuh
Copy link
Member

According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning.

If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch.

So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one).

Regards.

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.

2 participants