introduce inApps[] filter for search via ajax query #12555
Conversation
| $this->options = $options; | ||
| } | ||
|
|
||
| /** | ||
| * get a value from the options array or null | ||
| * @param string $key | ||
| * @return string|array |
|
🚀 Test PASSed. 🚀 |
|
squasch? 🙈 👍 |
butonic
force-pushed
the
app_specific_search
branch
from
029318e
to
e52bf24
Compare
|
another review? @andrewsbrown maybe? |
|
I like it, 👍 . Side note: What's the rationale for lines 46 and 47 in lib/private/search.php? It looks like backwards compatibility but could you confirm and maybe document? The difference between forApps and inApps wasn't readily apparent... |
|
🚀 Test PASSed. 🚀 |
…s show up in files app only use more flexible return type check array with !empty instead of count
butonic
force-pushed
the
app_specific_search
branch
from
e52bf24
to
1d490b5
Compare
|
@andrewsbrown I moved the code to a documented function in the abstract search provider. |
|
🚀 Test PASSed. 🚀 |
|
The inspection completed: 7 new issues, 2 updated code elements |
butonic
changed the title
|
Thinking about moving this to https://github.com/owncloud/apps/tree/master/search |
|
Ok, instead of changing everything at once let us merge this. We can move the relevant code to the search app if it makes sense after we decided how to handle search/filter/find. I fear that discussion might take a while so I prefer to get app specific search ready asap. @PVince81 @schiesbn can I get another 👍 ? |
butonic
changed the title
| $this->initProviders(); | ||
| $results = array(); | ||
| foreach($this->providers as $provider) { | ||
| /** @var $provider Provider */ | ||
| $results = array_merge($results, $provider->search($query)); | ||
| if ($provider->providesResultsFor($inApps)) { |
There was a problem hiding this comment.
Is there a unit test that needs updating for this ?
|
I'm getting |
|
Hmmm... this happened while having |
|
The search providers are registered by the apps themselves. For backwards compatibility I only query selected providers when both a) the provider has been registered for a specific type and b) the query was for a specific type. Otherwise I fall back to querying all providers, in order to not loose results from old search providers. |
|
Got it, makes sense 👍 |
introduce inApps[] filter for search via ajax query
Make file results show up in files app only, allows limiting search results to specific apps. see #821
Gracefully handles old apps that don't set the apps option for a search provider.
@PVince81 do we have a JS api to get the currently active app? I currently use
var classList = document.getElementById('content').className.split(/\s+/);and grab the first class starting with 'app-' ... but that seems hackish to me.PRs for contacts, calendar and other apps to follow, as well as a PR to move results to the content area instead of the current tiny dropdown overlay.