This makes it impossible to filter the notifications you need.
For example, try to search on"cache" or "entity" in the keyword field on http://drupal.org/list-changes/drupal

CommentFileSizeAuthor
#12 1401136-changerecords-v2.patch61.12 KBjhodgdon
#5 1401136.patch62.19 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

Project: Drupal.org site moderators » Drupal.org customizations
Version: » 6.x-3.x-dev
Component: Other » Code

Moving to the proper queue.

jhodgdon built this initially so might have some ideas, but since this is just a View, anyone should feel free to download http://drupal.org/project/drupalorg_testing and give it a whirl.

dww’s picture

That's because views uses the core search index, but on d.o we use Solr and most nodes aren't indexed by core search see http://drupal.org/project/coresearches for more. Issue queue views work because of a special-case to index issue nodes and comments. See the search_index subdir in the project_issue project.

Since there's a relatively tiny number of these change notice nodes it should be no problem to index them. I see 2 options:

A) Add some similar code to force these nodes to be indexed into the drupalorg feature module that defines the node type and views.

B) Generalize the project_issue_search_index.module so that you just configure what node types you want it to index. Could be split out into a separate project quite easily.

I'd prefer B but I can't do it myself right now, so I don't want to tell someone else they have to do it that way. If I can get a chance to mess with this, I'll assign this to myself.

Cheers,
-Derek

jhodgdon’s picture

I know this issue was reported quite a while ago (months ago) but I can't find the other issue :( ... Anyway this one has information on how to resolve it -- thanks dww!

jhodgdon’s picture

Assigned: Unassigned » jhodgdon
Issue tags: +docs infrastructure

I'm taking this on...

jhodgdon’s picture

FileSize
62.19 KB

Here's a patch that does option (c): In place of using search keywords, have fields for Title and Description keywords, which don't rely on search.module at all.

jhodgdon’s picture

Status: Active » Needs review

This can be tested at
http://docs-infra-drupal.redesign.devdrupal.org/list-changes/drupal

After revising the view in the UI and then exporting to make the patch in #5, I applied the patch on my test site, cleared the cache, and reverted the view to code in the Views UI. So it should be a valid test of the patch.

I think it is working fine. Other testing would be welcome. :)

xjm’s picture

I tested a bit on the dev site and I agree it's working fine. Maybe there's a nicer way in the long term, but I vote for "working at all" in the short term. :)

dww’s picture

Clever! ;) That'd probably avoid the problems I explained at #2. However, I have two concerns:

a) The UX is a little more crappy this way. "You mean I have to tell you exactly which field to search in?! c'mon..." Obviously, something that works at all is better UX than what we have now, but I'm just sayin'. ;)

b) If we're not using search.module and therefore {search_index} I'm pretty sure that means this query is going to be awful for the DB servers. :( Since body lives in {node_revision} while title, status, type, etc all live in {node}, we're going to have a WHERE spanning multiple tables, which means it can't be indexed. And an unindexed query on {node_revisions} doing RLIKE on the body field is going to be *painfully* expensive and slow. I could be wrong, but that's how I understand things would be working here. Even though the query wouldn't be fully indexed even if it was using {search_index}, at least it'd be = not RLIKE, so that's way faster. I think it'd be important to see an EXPLAIN on the resulting query before we continue with this approach.

Sorry I haven't have time to do anything else with this myself.

Thanks,
-Derek

jhodgdon’s picture

The description field is a CCK text field, not the node body field, if that makes any difference. Probably it doesn't for efficiency... but there are tons of other filters on that page that will also be going against other CCK text fields, so I don't think this makes the query any worse?

I agree the UX isn't great, but the people using this are developers anyway so probably can live with it.

dww’s picture

Oh, I just assumed node body. Yeah, a separate CCK field makes a huge difference, actually! In that case, we're JOIN'ing on {content_type_changenotice} which is going to vastly shrink the result set from {node} in the first place. Then, when dealing with WHERE, the RLIKE is only on the relatively tiny number of rows in {content_type_changenotice} instead of on the giant {node_revision} table itself. Probably nothing to worry about here. False alarm!

Still wouldn't hurt to see the EXPLAIN, but yeah, I'm much less concerned about the performance of this now.

Thanks!
-Derek

jhodgdon’s picture

Here's another idea: I could remove the Title keywords field and just leave Description? That would most likely cover most of the use cases (I think any keyword in the title is likely to be in the description). Thoughts?

jhodgdon’s picture

Here's a new version that just has the Description keywords. It's a bit cleaner, and does not seem to have problems finding issues.

jhodgdon’s picture

On my test box, I just reverted the previous patch, applied the new patch, reverted the view to using the view in code, and cleared the cache for good measure.

If you want to see this in action, go to
http://docs-infra-drupal.redesign.devdrupal.org/list-changes/drupal (u/p = drupal/drupal)
Try a search for "entity" or "entities" as an example. I think it's fine... definitely better than what we have now!

xjm’s picture

I tested a few terms on the dev site and it seems to work well. Definitely better UX that way too.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

OK, I think it is fine too. Tentatively RTBC unless maintainers object. :)

jhodgdon’s picture

Issue tags: +Developer improvements

tagging

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed and running on staging.devdrupal.org.

jhodgdon’s picture

Thanks! apparently this is on d.o too... let's see. Yes! Keyword searching works, finally. :)

webchick’s picture

WOOHOO!! :D

Great work Jennifer! Thanks for the speedy deployment, Neil!

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 6526cf1 on 6.x-3.x, 7.x-3.x-dev authored by jhodgdon, committed by drumm:
    [#1401136] Fix change notification "keyword" function