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

Overriding search method to make a column searchable seems to remove my action buttons #1641

Closed
scottjs opened this issue Sep 18, 2018 · 9 comments
Labels

Comments

@scottjs
Copy link
Contributor

scottjs commented Sep 18, 2018

Bug report

I want to make a column searchable in the CRUD panel but not show it in the list of columns. I found some notes here that appears to allow me to use $this->crud->addColumn() to append additional columns to the search.

However, when I do this my action buttons seem to get replaced with this custom column and I'm not sure why.

What I did:

public function search()
{
    $this->crud->addColumn([
        'name' => 'primary_contact_id',
        'label' => 'Primary Contact',
        'type' => 'select',
        'entity' => 'primary',
        'attribute' => 'firstname'
    ]);

    return parent::search();
}

What I expected to happen:

No visible changes to the column view but allows the search field to also search for the primary contact name.

What happened:

The action buttons were removed and replaced with my custom column instead.

Before:
screen shot 2018-09-18 at 14 55 03

After:
screen shot 2018-09-18 at 14 54 44

What I've already tried to fix it:

Overriding the entire search method to see if it's an ordering issue, but I'm not able to work out how this works internally to try other things.

Backpack, Laravel, PHP, DB version:

Backpack: 3.4
Laravel: 5.6
PHP: 7.1.20

@welcome
Copy link

welcome bot commented Sep 18, 2018

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication mediums:

  • Bug Reports, Feature Requests - Github Issues (here);
  • Quick help (How do I do X) - Gitter Chatroom;
  • Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

--
Justin Case
The Backpack Robot

@tswonke
Copy link
Contributor

tswonke commented Sep 18, 2018

Yes, same to me.

Just looking for the same: Using the search in a table field that is not declared as a column.

@jsvini
Copy link
Contributor

jsvini commented Sep 20, 2018

You could add a custom search logic in a column to include other columns in the search, without overriding the search method:

$this->crud->addColumn( [
	'name'        => 'my_field',
	'label'       => 'My visible column',
	'type'        => 'text',
	'searchLogic' => function ( $query, $column, $searchTerm ) {
		$query->orWhere( $column['name'], 'like', '%' . $searchTerm . '%' );
		// add other fields
		$query->orWhere( 'my_hidden_field', 'like', '%' . $searchTerm . '%' );
	}
] );

@scottjs
Copy link
Contributor Author

scottjs commented Sep 20, 2018

You could add a custom search logic in a column to include other columns in the search, without overriding the search method:

$this->crud->addColumn( [
	'name'        => 'my_field',
	'label'       => 'My visible column',
	'type'        => 'text',
	'searchLogic' => function ( $query, $column, $searchTerm ) {
		$query->orWhere( $column['name'], 'like', '%' . $searchTerm . '%' );
		// add other fields
		$query->orWhere( 'my_hidden_field', 'like', '%' . $searchTerm . '%' );
	}
] );

Thanks for the reply :) Yeah that's what I ended up doing in the end and is actually working fairly well. I just thought it was weird in this case to attach unrelated search logic to a random field just to modify the search query.

Overriding the search method seemed like a cleaner solution and the fact that the action buttons disappeared definitely seems a bit strange.

Thanks,
Scott.

@tswonke
Copy link
Contributor

tswonke commented Sep 21, 2018

Yeah, this is a very good "quick&dirty" solution :)

@tabacitu
Copy link
Member

@jsvini that’s a nice hack, thank you :-) I think we should provide an official solution to this, since it’s a pretty common scenario.

Solution A

The most intuitive solution would probably be to allow developers to add “invisible” columns, that get searched, but aren’t visible in the list view. We could:

  • have a “visible” attribute on the columns, which is true by default;
  • use getVisibleColumns() instead of getColumns() in the list view, but keep using getColumns() in search;

So it would be as easy as:

$this->crud->addColumn([
   'name' => 'description', // The db column name
   'label' => “Description", // Table column heading
   'visible' => false, // show or hide the column from the list view
]);

Solution B

A solution with less impact on existing features would be to have something like addColumnToSearch(), which would just add it to another property on the $crud, say… $additionalSearchColumns, which would get merged with the other columns in applySearchTerm() alone. So you’d use it like:

$this->crud->addColumnToSearch([
   'name' => 'description', // The db column name
   'label' => “Description", // Table column heading
]);

Maybe a better name would be addSearchableAttribute() or… I don’t know.


What do you guys think? Any better solutions?

@tswonke
Copy link
Contributor

tswonke commented Oct 24, 2018

IMO we shouldn't use any kind of fake column here. I think this is misleading. It never shows up, or do I miss something? Why do we need "label" for this?

I tend to something like your addSearchableAttribute() (or a similar name), but without using our columns array.

Maybe something like this with 3 ways to use:

// simple string
$this->crud->addSearchableAttribute('column_name');

// array of simple strings
$this->crud->addSearchableAttribute(['column_name1', 'column_name2']);

// closure for special searches
$this->crud->addSearchableAttribute(function() {...});

@tabacitu
Copy link
Member

@tswonke you make a good point... It’s not actually a column, it’s just an attribute you make searchable.

@tabacitu
Copy link
Member

tabacitu commented Nov 16, 2018

@tswonke , @scottjs @jsvini I've inadvertently pushed a solution for this yesterday, in the 3.5 release (also #1706). I don't think this is the perfect solution we were discussing above, but it is a solution. You can now do:

$this->crud->addColumn([
   'name' => 'description',
   'visibleInTable' => false, 
   'visibleInModal' => false,
   'visibleInExport' => false,
   'visibleInShow' => false,
]);

and it will be searchable, but show up NOWHERE in the ListEntries nor Show operation.

I'll close the issue for now, since we do have a soluton. Let me know if you think it's not a particularly good one, and we'll create a PR to add addSearchableAttribute() like @tswonke proposed - that was definitely cleaner. And we'll move the conversation there.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants