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

7595 data query apply relation #970

Closed

Conversation

rlehmann
Copy link
Contributor

Has_many relations are also searched in the GridFieldAddExistingAutocompleter if they are defined in searchableFields.

TeamCity and others added 30 commits August 14, 2012 02:19
This reverts commit e003796.

Conflicts:

	tests/travis/_ss_environment.php
…g on the name of the admin and greeting message. position and height were being set inline so added !important to override this.
FIX Director::is_absolute_url() now ignores query string
BUG: Allow using instances for search filters.
Sean Harvey and others added 26 commits November 15, 2012 14:43
GridField uses createTag() which is marked for deprecation, rather
than have it used as the cornerstone of generating FormField templates,
use it as a helper in case fields generate HTML tags from PHP.
These have been moved to a module called "legacytablefields"
located at https://github.com/silverstripe-labs/legacytablefields
Removed h2 from outside breadcrumbs, and put it within the breadcrumbs
div

Goes alon the cms commit 4fab9b8
The original intention of this method was to be a rewrite
check by the installer, but it doesn't belong in
DatabaseAdmin, nor is it used by the installer anymore,
since the rewrite test is done by the installer directly.
Removing redundant DatabaseAdmin::testinstall()
We might decide to use it for other submissions than EditForm.
Conflicts:
	admin/code/CMSProfileController.php
	composer.json
	tests/model/DataObjectTest.php
…rrect-bc-nesting

BUG Incorrect html nesting of breadcrumbs
…itform-validation

Future-proof the submitForm for use with forms without validation.
Makes the editor API more flexible by triggering generic JS events that
can be used from entwine. This makes it easier to add event handlers to
the editor and cleans up the initialisation call from unrelated code.

The patching code also forwards editor changes to the textarea field,
which in perspective will allow the changetracker to react to changes in
this field as they happen.
… the relations name as an alias to the SQL query
@chillu
Copy link
Member

chillu commented Nov 23, 2012

Hey Roland, thanks for bringing this up again, but I have a couple of questions:

  • You have based your pull request on the wrong branch (should be against master), hence all the stray commits. In general, please doublecheck the pull request once created to avoid these kind of errors
  • You reference http://open.silverstripe.org/ticket/7595, but from what I can tell the request doesn't actually fix the DataList issue, but rather deals with GridField
  • We'll need some context on how this pull request is different from the one you've submitted a couple of weeks ago. If you remember, I fixed the GridField side of things with NEW Many-many relation data editing in GridFieldDetailForm #939, and answered your email enquiries. Do you have any questions regarding those? The pull request is still pending review, but that doesn't mean its a good idea to send a new pull request with a different solution without context :)

Closing off for now, please reopen if appropriate.

@chillu chillu closed this Nov 23, 2012
@chillu
Copy link
Member

chillu commented Nov 23, 2012

Had another look at the change, and dug up your original pull request (#756).
From what I can tell, its the same approach, so my concerns remain the same:

"We want to get away from duplication logic that's close to the model in controllers, the dot notation is a good example of that. We now allow disjunctive grouping of filters, see #808. Can I suggest that you try to rework your patch to use it? Haven't tried this myself, but the general idea is that you can just pass through field names with dot notation and it'll do its thing. If it doesn't, we need to fix the underlying DataList bugs first."

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.

None yet