Search not working for Staff users


(David) #1

Hello,

We’ve recently upgraded to rails4, and we are mostly up to date as we run frequent merges against discourse/discourse master.

We’ve come to realize after a few weeks that the search feature is broken, but only for “Staff” users.

The search function is split into three kinds : users, categories and topics. Both user/category search work for “Staff”, but “topic” returns empty results.

In our particular case, almost all of our categories are marked as “secure”, and we only have one “public” category.

In fact, what happens is that “Staff” search look ups dont search inside “secure” categories at all.

For instance, after some logging, I’ve noticed that in lib/search.rb, secure_category_ids is empty when a “Staff” user does a look up.

def posts_query(limit)
[...]
 if secure_category_ids.present?
        posts = posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted) OR (categories.id IN (?))", secure_category_ids).references(:categories)
 else
        posts = posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted)").references(:categories)
 end

Then I looked into app/models/user.rb and discovered that the following returns [] for Staff users, in the context of search query (For example, from the ListController context, it returns the correct result).

def secure_category_ids
    cats = self.staff? ? Category.where(read_restricted: true) : secure_categories.references(:categories)
    cats.pluck('categories.id').sort
end 

The query being executed for this particular context Category.where(read_restricted: true).pluck('categories.id').sort is :

SELECT categories.id FROM "categories" LEFT OUTER JOIN "category_search_data" ON "category_search_data"."category_id" = "categories"."id" WHERE (category_search_data.search_data @@ TO_TSQUERY('english', 'data:*')) AND "categories"."read_restricted" = 't' ORDER BY topics_month 

When we are listing topics instead, the resulting query is :

 SELECT categories.id FROM "categories" WHERE "categories"."read_restricted" = 't'

So it appears that there is a bunch of noise that is being added to the query.

Then I discovered that in lib/search.rb, we have :

def category_search
      categories = Category.includes(:category_search_data)
                           .where("category_search_data.search_data @@ #{ts_query}")
                           .references(:category_search_data)
                           .order("topics_month DESC")
                           .secured(@guardian)
                           .limit(@limit)

      categories.each do |c|
        @results.add_result(SearchResult.from_category(c))
      end
    end

The above code appears to be where the noise comes from. After all, we do a category_search before doing a topic_search.

    def find_grouped_results
      [...]
        user_search
        category_search
        topic_search

But I don’t understand how the queries can conflict with each other. Is the scope not supposed to be different?

As a matter of fact, the category_search code runs the proper query, and category results are properly displayed to ‘Staff’ users.


(Sam Saffron) #2

@eviltrout something is weird here, can you have a look?


(Robin Ward) #3

Sam why should you never see that topic? You have permission to see it and it seems search respects that. As an anon user for example it doesn’t show up.


(Sam Saffron) #4

my point is, it should see the topic first when results are expanded and collapsed.


(Sam Saffron) #5

Oh my … probably a bug in Rails, worked around it for now

https://github.com/discourse/discourse/commit/1649f56529eafdf073c74c85f74238972202f2e1


(Sam Saffron) #6

A repro is here:

Some of the Rails team say this is expected behavior, we should open a ticket for Rails.


(rafaelfranca) #7

It is expected behavior to me. You are chaining two scopes in the same class and this is expected to append the last one in the query.

A lot of people use this behavior so I’m afraid that we can’t change it easily.


(rafaelfranca) #8

Just to make clear that is exactly the same as:

Post.where(title: "one").scoping do
  Post.count
end

Here the related code: rails/delegation.rb at 682d7c7035fed76c42ba6fefa38973387e80409e · rails/rails · GitHub


(Robin Ward) #9

Since this commit I can’t seem to search for that “you should never” topic at all. I think it might be restricting too much?


(Sam Saffron) #10

ha ?


(Robin Ward) #11

I swear yesterday typing “you should” made the results come up. But I can’t prove it! Thanks I’ll go from here.


(Sam Saffron) #12

you and should are both stop words :slight_smile: maybe its time to experiment with a trigram based search


(Jeff Atwood) #13

Can this be archived @eviltrout?


(Sam Saffron) #14

This can be archived, “you” and “should” being stop is totally unrelated.


(Sam Saffron) #15