Add styling and additional logic to search for users

[Alexander-Dubrawski]

This PR closes #127 and closes #98 .

In the navbar is now an icon that is redirecting the user to the search page.

The styling is now looking as follows:

Desktop:

Mobile:

[felixauringer]

I added some suggestions that assign the instance variables in the view as the users controller would do

[felixauringer]

The code looks great now! The PR has also a size that is very good reviewable. Just one question:

  def search
    @users = User.search(params[:search]).where.not(id: current_user.id)
    @users_to_add = @users.reject do |user|
      current_user.sent_contact_request?(user)
    end
  end

It looks like users_to_add is a subset of users. Did you try out whether users that aren't contacts of (or requested by) the current user are displayed twice?
If yes, I would suggest something like the following

  def search
    other_users = User.search(params[:search]).where.not(id: current_user.id)
    @users_to_add = other_users.reject do |user|
      current_user.sent_contact_request?(user)
    end
    @users = other_users.select do |user|
      current_user.sent_contact_request?(user)
    end
  end

[Alexander-Dubrawski]

The code looks great now! The PR has also a size that is very good reviewable.

Sorry for this chaotic PR of mine 🙈 . I really don't know why I implemented stuff that was already existing. But thanks for all your feedback and patience.

It looks like users_to_add is a subset of users. Did you try out whether users that aren't contacts of (or requested by) the current user are displayed twice?

You are absolutely right. Good eye. I adjusted the code.

[felixauringer]

Nice to finally finish this 🎉 I'm sorry for having so much comments but thank you for implementing all of it :)

[frcroth]

This is what /users/search looks for me on the deployed version

[frcroth]

@Alexander-Dubrawski

[Alexander-Dubrawski]

This is what /users/search looks for me on the deployed version

I can not reproduce the error. @frcroth what exactly did you do? (For example: login, click the search icon, ...)

[felixauringer]

This is what /users/search looks for me on the deployed version

Same for me, I will try to investigate the reason

[Alexander-Dubrawski]

Same for me, I will try to investigate the reason

@felixauringer thanks that would be really nice. If you can not find the error, please ping me and I can also have a look this evening.

[felixauringer]

Works locally for me, so I will have a look at the heroku logs

[felixauringer]

The relevant part is

ActionView::Template::Error (Can't resolve image into URL: to_model delegated to attachment, but attachment is nil):

2021-02-06T15:00:46.081546+00:00 app[web.1]:      3:     <div class="container">

2021-02-06T15:00:46.081546+00:00 app[web.1]:      4:       <div class="row">

2021-02-06T15:00:46.081547+00:00 app[web.1]:      5:         <div class="col-xs">

2021-02-06T15:00:46.081548+00:00 app[web.1]:      6:           <%= image_tag user.avatar, height: 65, width: 65, class: "rounded-circle" %>

2021-02-06T15:00:46.081548+00:00 app[web.1]:      7:         </div>

2021-02-06T15:00:46.081554+00:00 app[web.1]:      8:         <a class="col-8" href=<%= user_path(user) %>>

2021-02-06T15:00:46.081555+00:00 app[web.1]:      9:           <h5 class="card-title"><%= user.display_name %></h5>

2021-02-06T15:00:46.081556+00:00 app[web.1]:   

2021-02-06T15:00:46.081556+00:00 app[web.1]:  app/views/users/_card.html.erb:6

2021-02-06T15:00:46.081556+00:00 app[web.1]:  app/views/users/search.html.erb:14

2021-02-06T15:00:46.081557+00:00 app[web.1]:  app/views/users/search.html.erb:13:in `each'

2021-02-06T15:00:46.081557+00:00 app[web.1]:  app/views/users/search.html.erb:13

[felixauringer]

So again problems with the avatars. This PR did not touch _card.html.erb so this problem was likely there before merging this one

[felixauringer]

That's executed on production data:

User.all.select { |user| user.avatar.nil? }.length
D, [2021-02-06T15:08:13.259421 #4] DEBUG -- :   User Load (1.8ms)  SELECT "users".* FROM "users"
=> 0

So the migration worked as expected

[felixauringer]

I added an issue for this: #191