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

Ver. 2.6.2 Mass Classification fails to classify events #291

Closed
jamisonmaxwell opened this issue Jun 3, 2013 · 13 comments
Closed

Ver. 2.6.2 Mass Classification fails to classify events #291

jamisonmaxwell opened this issue Jun 3, 2013 · 13 comments

Comments

@jamisonmaxwell
Copy link

I have tried this on two separate installs with the latest version, but I can't seem to get mass classification to function again. I am more than willing to provide any logs, but don't really know where to start.

@jthoel
Copy link

jthoel commented Jun 5, 2013

We are seeing the same problem, and I can send a debug log directly to a dev if it would help.

@miketanderson
Copy link
Contributor

I just upgraded to 2.6.2 and am seeing the same problem. Let me know if I can provide anything to help debug.

@joshuah
Copy link

joshuah commented Jun 6, 2013

I am having the same issue. Also having issues classifying search results.

@ardevd
Copy link

ardevd commented Jun 7, 2013

Same issue here.. Classifications in general are not working very well.

@gehrhorn
Copy link
Contributor

gehrhorn commented Jun 7, 2013

Can anyone validate that this was working in 2.6.1? (To put it better, what is the last version where this was working?)

@jthoel
Copy link

jthoel commented Jun 7, 2013

Yes.. it was working fine and dandy on 2.6.1

@gehrhorn
Copy link
Contributor

gehrhorn commented Jun 7, 2013

Between 2.6.1 and 2.6.2 the only files that changed were

ChangeLog.md                             |   12 ++++
 Gemfile.lock                             |    2 -
 README.md                                |   15 +----
 app/views/page/_graph_signature.html.erb |    2 +-
 app/views/saved_searches/view.html.erb   |    7 +-
 app/views/sensors/index.js.erb           |   14 ++++-
 lib/snorby/jobs/cache_helper.rb          |   14 ++++
 lib/snorby/search.rb                     |  100 ++++++++++++++++++++++++++++--
 lib/snorby/version.rb                    |    2 +-
 lib/snorby/worker.rb                     |    2 +-
 public/assets/snorby.css                 |    2 +-
 public/assets/snorby.css.gz              |  Bin 16624 -> 16634 bytes
 public/assets/snorby.js                  |    2 +-
 public/assets/snorby.js.gz               |  Bin 143058 -> 143100 bytes
 public/javascripts/snorby.js             |   12 ++++
 public/stylesheets/snorby.css            |   12 +++-

I think it's most likely that the culprit is in one of

 app/views/page/_graph_signature.html.erb |    2 +-
 app/views/saved_searches/view.html.erb   |    7 +-
 app/views/sensors/index.js.erb           |   14 ++++-
 lib/snorby/jobs/cache_helper.rb          |   14 ++++
 lib/snorby/search.rb                     |  100 ++++++++++++++++++++++++++++--
 lib/snorby/version.rb                    |    2 +-
 lib/snorby/worker.rb                     |    2 +-
 public/assets/snorby.js                  |    2 +-
 public/javascripts/snorby.js             |   12 ++++

and probably not the first two. I'm going to poke around these files.

@gehrhorn
Copy link
Contributor

gehrhorn commented Jun 7, 2013

@miketanderson and I are working on this and we think the problem may be in events_controller.rb

# This is in def mass_action, which starts on line 182
    sql = Snorby::Search.build("true", true, options)
    ids = Event.get_collection_id_string(sql)
    if params[:jobqueue]
         Delayed::Job.enqueue(Snorby::Jobs::MassClassification.new(ids, params[:classification_id], User.current_user.id))
     else
          Event.update_classification(ids, params[:classification_id], User.current_user.id)
      end

I don't think that ids variable is getting set correctly. We think that the search function may not be working correctly, perhaps from the edits to search.rb

if column == :classification && value.to_i == 0
  if operator == :is
    operator = :isnull
  else
    operator = :notnull
  end
  value = "NULL"
end

if column == :has_note
  if value.to_i == 1
    case operator
    when :is
      operator = :gt
      value = 0
    when :is_not
      operator = :lt
      value = 1
    end

  else
    case operator
    when :is
      operator = :lt
      value = 1
    when :is_not
      operator = :gt
      value = 0
    end

  end
end

tmp_sql = "#{COLUMN[column][x]} #{OPERATOR[operator]}"

@miketanderson
Copy link
Contributor

It appears the SQL query for mass classification is inverting the classification IS NOT NULL vs IS NULL.

Specifically the two instances of this snippet:

  if operator == :is
    operator = :isnull
  else
    operator = :notnull
  end

I switched both instances of this to

  if operator == :is
    operator = :notnull
  else
    operator = :isnull
  end

and mass classification worked again.

I do not know if this would break any of the new functionality, but this appears to be the source of the mass classification bug.

@jthoel
Copy link

jthoel commented Jun 7, 2013

And this is in the file 'events_controller.rb'?

@miketanderson
Copy link
Contributor

I was tacking on to the report by @gehrhorn - the file is lib/snorby/search.rb starting on line 338 and again on line 390.

@jthoel
Copy link

jthoel commented Jun 7, 2013

Awesome.. I'll test it here. and update.

UPDATE - Seems to work great. Thanks guys!

@joshuah
Copy link

joshuah commented Jun 7, 2013

@miketanderson Your fix seems to work for me.

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

No branches or pull requests

7 participants