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

Profile api test PR #3224

Closed
wants to merge 17 commits into from
Closed

Profile api test PR #3224

wants to merge 17 commits into from

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Aug 9, 2018

Just testing something for #3134!

@ghost ghost assigned jywarren Aug 9, 2018
@ghost ghost added the in progress label Aug 9, 2018
@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

Locally, this gives me, on mysql:

ActiveRecord::StatementInvalid:         ActiveRecord::StatementInvalid: Mysql2::Error: Expression #1 of ORDER BY clause is not in SELECT list, references column 'plots.node_revisions.timestamp' which is not in SELECT list; this is incompatible with DISTINCT: SELECT DISTINCT `node`.`nid` FROM `node` LEFT OUTER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` LEFT OUTER JOIN `community_tags` ON `community_tags`.`nid` = `node`.`nid` LEFT OUTER JOIN `term_data` ON `term_data`.`tid` = `community_tags`.`tid` WHERE (`node`.`nid`) IN (1, 8, 1, 2, 8, 9, 10, 15) AND ((created >= 1533839387 AND created <= 1533846587) OR (timestamp >= 1533839387  AND timestamp <= 1533846587)) ORDER BY node_revisions.timestamp DESC
            test/unit/user_test.rb:97:in `block in <class:UserTest>'

  23/23: [================================================] 100% Time: 00:00:07, Time: 00:00:07

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

That's related to this terrible obscure issue: https://dev.mysql.com/doc/refman/5.7/en/group-by-handling.html

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

Er, wait, not that one. But something similar, let me look...

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

The error is on this line: (line 97)

nodes_time = bob.content_followed_in_period(2.hours.ago,Time.now).pluck(:nid)

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

Huh... what do you know... a pretty similar error to in #3217...

plots2/app/models/user.rb

Lines 371 to 376 in 04d5bbc

Node.where(nid: node_ids)
.includes(:revision, :tag)
.references(:node_revision)
.where("(created >= #{start_time.to_i} AND created <= #{end_time.to_i}) OR (timestamp >= #{start_time.to_i} AND timestamp <= #{end_time.to_i})")
.order('node_revisions.timestamp DESC')
.distinct

@plotsbot
Copy link
Collaborator

plotsbot commented Aug 9, 2018

1 Error
🚫 There was a test failure at: Failure: test_search_profiles_without_order_by_and_default_sort_direction(Minitest::Result) [/app/test/functional/search_api_test.rb:93]: — expected +++ actual @@ -1 +1 @@ -“TEST” +”{"items":[{"docId":0,"docType":"user","docUrl":"/profile/steff3","docTitle":"steff3","docSummary":"","docScore":0},{"docId":0,"docType":"user","docUrl":"/profile/steff2","docTitle":"steff2","docSummary":"","docScore":0},{"docId":0,"docType":"user","docUrl":"/profile/steff1","docTitle":"steff1","docSummary":"","docScore":0}],"srchParams":{"srchString":"steff","seq":null,"showCount":null,"pageNum":null,"tagName":null}}”
2 Warnings
⚠️ New migrations added. Please update schema.rb.example by overwriting it with a copy of the up-to-date db/schema.rb. Also, be aware to preserve the MySQL-specific conditions for full-text indices.
⚠️ This pull request cannot be merged yet due to merge conflicts. Please resolve them – probably by rebasing – and ask for help (in the comments, or in the chatroom if you get into trouble!.
1 Message
📖 @jywarren Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

The Travis tests are failing on the functional test for search_api_test.rb --

There was a test error at: Failure: test_search_profiles_without_order_by_and_default_sort_direction(Minitest::Result) [/app/test/functional/search_api_test.rb:95]: NoMethodError: undefined method ‘[]’ for nil:NilClass test/functional/search_api_test.rb:95:in ‘block in '
--

on requesting: `get '/api/srch/profiles?srchString=steff'

I'm going to try this locally...

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

OK, locally, on mysql, i get, for http://localhost:3000/api/srch/profiles?srchString=jeff:

{"items":[{"docId":0,"docType":"user","docUrl":"/profile/jeffrey","docTitle":"jeffrey","docSummary":"","docScore":0},{"docId":0,"docType":"user","docUrl":"/profile/jeff","docTitle":"jeff","docSummary":"","docScore":0}],"srchParams":{"srchString":"jeff","seq":null,"showCount":null,"pageNum":null,"tagName":null}}

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

OK, so I tried putting a puts error logging statement just before this, so we see what's really in there:

                                                         
+  93 puts '#####################'                                                             
+  94 puts last_response.body                                                                  
   95      json = JSON.parse(last_response.body)                                               
   96                                                                                          
+  97 puts json                                                                                
+  98 puts '#####################'                                                             
   99      assert_equal "/profile/steff3",   json['items'][0]['docUrl']                        
  100      assert_equal "/profile/steff2",   json['items'][1]['docUrl']                        
  101      assert_equal "/profile/steff1",   json['items'][2]['docUrl']   

If this doesn't work, i'll test against just last_response.body first

@stefannibrasil
Copy link

@jywarren that was the error, yeah... if you also change this line https://github.com/publiclab/plots2/blob/master/test/unit/user_test.rb#L37

to 'jeff' or any other username on fixtures, it will show the same error.

@stefannibrasil
Copy link

Let me know anything that you find, I have to work now so I'll get back to this later, thanks for the help!

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018 via email

@jywarren
Copy link
Member Author

jywarren commented Aug 9, 2018

Restarting this build as it errored for unrelated reasons.

@stefannibrasil
Copy link

I restarted the build, but I realized your code is slightly different from our version, you will have some errors because of that . Could try your files with our latest commit that works?

you are on the using find_userscommit but that wans't working because we didn't fix the username index at that point... the version that is working is the commit giving better names for methods. I guess if we compare with the same code will be better to hack this =D

@@ -42,7 +43,7 @@ class User < ActiveRecord::Base
after_destroy :destroy_drupal_user

def self.search(query)
User.where('MATCH(username, bio) AGAINST(?)', query)
User.where('MATCH(username, bio) AGAINST (?)', query)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this didn't work, you need to use like this:

def self.search(query)
    User.where('MATCH(username) AGAINST((?) IN BOOLEAN MODE)', query + '*')
  end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I clicked on review instead of add a comment!

User.order('id DESC')
.where('username LIKE ? AND status = 1', '%' + input + '%')
User.where('username LIKE ? AND rusers.status = 1', '%' + input + '%')
.order('id DESC')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is our Search Scope find users method:

def self.find_users(query, limit)
    if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
      User.search_by_username(query)
          .where('rusers.status = ?', 1)
          .limit(limit)
    else
      User.where('username LIKE ? AND rusers.status = 1', '%' + input + '%')
          .limit(limit)
    end
  end

Stefanni and others added 2 commits August 10, 2018 22:23
…ilaaraujo/plots2 into profiles-endpoint-most-recently-people
@stefannibrasil
Copy link

can I close this one? :) @jywarren

@jywarren
Copy link
Member Author

jywarren commented Aug 15, 2018 via email

@stefannibrasil
Copy link

closed via #3134

@ghost ghost removed the in progress label Aug 15, 2018
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

Successfully merging this pull request may close these issues.

4 participants