Using typeahead when adding users to a team

[jloehel]

Fixes: #39

Signed-off-by: Jürgen Löhel [email protected]

[salzig]

You could do this in database.

team_user_ids = @team.team_users.pluck(:user_id)
User.enabled.where.not(id: team_user_ids).where(User.arel_table[:username].matches("#@query%")).pluck(:username)

[mssola]

I agree with @salzig

[mssola]

This branch has conflicts because I've merged #534

[salzig]

IMHO: its a code smell to fetch and build objects where only a single attribute is needed.

[jloehel]

Bootstrap Typeahead for Rails (https://github.com/Nerian/bootstrap-typeahead-rails) uses typeahead 0.10.5.1 and for this version it is necessary to build objects. Should I switch to a newer version of typeahead? (0.11.1)

[salzig]

no, i don't mean those hashes. I was thinking about those ActiveRecord Objects that get build right before you map them down to hashes. Given there are 100 results by search_from_query you're building 100 ActiveRecord Objects containing all information stored for those results, just to map them down to username. A huge change would be to pluck the username, so ActiveRecord is only fetching the username from database.

matches = User.search_from_query(@team.member_ids,"#{@query}%").pluck(:username).map { |username| { username: username } }

[salzig]

sry, hadn't looked at the implementation of search_from_query, and hadn't expect it, by the name, to return just matching usernames.

[mssola]

remove this file. See comment above.

[jloehel]

Should I add the error message directly to the the application_controller.rb?

def deny_access
  @status = 401
  respond_to do |format|
    format.html { render template: "errors/401", status: 401, layout: "errors" }
    format.js { render js: @status }
    format.json { render json: @status }
  end
end

[mssola]

by above I meant below :P See my comment below about render nothing: true

[mssola]

The only problem that I got, and sorry for not noticing this before, is that one side-effect of this is that you've changed the size of the input element for the name (now it's much smaller). Could you change that?

Besides that, it looks good to me.

[mssola]

LGTM

[flavio]

Good job @jloehel! LGMT

[salzig]

beside comment in routes.rb:9 LGTM.

Well done!

[mssola]

@salzig is right. Please remove this comment.

[mssola]

Good job 👏

[flavio]

@mssola or @jloehel: could you also update the changelog?

[mssola]

@flavio let me do it