Skip to content

Commit

Permalink
now support books shared with multiple teams
Browse files Browse the repository at this point in the history
  • Loading branch information
epugh committed Dec 13, 2023
1 parent ccfb038 commit 2b5884b
Show file tree
Hide file tree
Showing 20 changed files with 135 additions and 44 deletions.
7 changes: 3 additions & 4 deletions app/controllers/api/v1/books_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,9 @@ def show

def create
@book = Book.new(book_params)

team = Team.find_by(id: params[:book][:team_id])
@book.teams << team
if @book.save
# first = 1 == current_user.cases.count
# Analytics::Tracker.track_case_created_event current_user, @case, first
respond_with @book
else
render json: @book.errors, status: :bad_request
Expand Down Expand Up @@ -92,7 +91,7 @@ def destroy
private

def book_params
params.require(:book).permit(:team_id, :scorer_id, :selection_strategy_id, :name, :support_implicit_judgements,
params.require(:book).permit(:scorer_id, :selection_strategy_id, :name, :support_implicit_judgements,
:show_rank)
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/import/books_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create

@book = Book.new

@book.team = Team.find(team_id)
@book.teams << Team.find(team_id)

scorer_name = params_to_use[:scorer][:name]
unless Scorer.exists?(name: scorer_name)
Expand Down
27 changes: 22 additions & 5 deletions app/controllers/books_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class BooksController < ApplicationController
respond_to :html

def index
@books = current_user.books_involved_with.includes([ :team, :scorer, :selection_strategy ])
@books = current_user.books_involved_with.includes([ :teams, :scorer, :selection_strategy ])
respond_with(@books)
end

Expand Down Expand Up @@ -68,7 +68,20 @@ def create
end

def update
@book.update(book_params)
# this logic is crazy, but basically we don't want to touch the teams that are associated with
# an book that the current_user CAN NOT see, so we clear out of the relationship all the ones
# they can see, and then repopulate it from the list of ids checked. Checkboxes suck.
team_ids_belonging_to_user = current_user.teams.pluck(:id)
teams = @book.teams.reject { |t| team_ids_belonging_to_user.include?(t.id) }
@book.teams.clear
book_params[:team_ids].each do |team_id|
teams << Team.find(team_id)
end

@book.teams.replace(teams)

@book.update(book_params.except(:team_ids))

respond_with(@book)
end

Expand Down Expand Up @@ -147,7 +160,8 @@ def combine

# rubocop:disable Metrics/MethodLength
def assign_anonymous
assignee = @book.team.members.find_by(id: params[:assignee_id])
# assignee = @book.team.members.find_by(id: params[:assignee_id])
assignee = User.find_by(id: params[:assignee_id])
@book.judgements.where(user: nil).find_each do |judgement|
judgement.user = assignee
# if we are mapping a user to a judgement,
Expand Down Expand Up @@ -235,8 +249,11 @@ def find_user
end

def book_params
params.require(:book).permit(:team_id, :scorer_id, :selection_strategy_id, :name, :support_implicit_judgements,
:show_rank)
params_to_use = params.require(:book).permit(:scorer_id, :selection_strategy_id, :name,
:support_implicit_judgements,
:show_rank, team_ids: [])
params_to_use[:team_ids].compact_blank!
params_to_use
end
end

Expand Down
25 changes: 21 additions & 4 deletions app/models/book.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
# support_implicit_judgements :boolean
# created_at :datetime not null
# updated_at :datetime not null
# owner_id :integer
# scorer_id :integer
# selection_strategy_id :bigint not null
# team_id :integer
#
# Indexes
#
Expand All @@ -24,7 +24,12 @@
#
class Book < ApplicationRecord
# Associations
belongs_to :team
# belongs_to :team
# rubocop:disable Rails/HasAndBelongsToMany
has_and_belongs_to_many :teams,
join_table: 'teams_books'
# rubocop:enable Rails/HasAndBelongsToMany

belongs_to :selection_strategy
belongs_to :scorer
has_many :query_doc_pairs, dependent: :destroy, autosave: true
Expand All @@ -40,14 +45,26 @@ class Book < ApplicationRecord
inverse_of: :book

# Scopes
scope :for_user, ->(user) {
scope :for_user_via_teams, ->(user) {
joins('
LEFT OUTER JOIN `teams` ON `teams`.`id` = `books`.`team_id`
LEFT OUTER JOIN `teams_books` ON `teams_books`.`book_id` = `books`.`id`
LEFT OUTER JOIN `teams` ON `teams`.`id` = `teams_books`.`team_id`
LEFT OUTER JOIN `teams_members` ON `teams_members`.`team_id` = `teams`.`id`
LEFT OUTER JOIN `users` ON `users`.`id` = `teams_members`.`member_id`
').where('
`teams_members`.`member_id` = ?
', user.id)
.order(name: :desc)
}

scope :for_user_directly_owned, ->(user) {
where('
`books`.`owner_id` = ?
', user.id)
}

scope :for_user, ->(user) {
ids = for_user_via_teams(user).pluck(:id) + for_user_directly_owned(user).pluck(:id)
where(id: ids.uniq)
}
end
4 changes: 2 additions & 2 deletions app/models/case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,13 @@ class Case < ApplicationRecord
', user.id)
}

scope :public_cases, -> { where(public: true) }

scope :for_user, ->(user) {
ids = for_user_via_teams(user).pluck(:id) + for_user_directly_owned(user).pluck(:id)
where(id: ids.uniq)
}

scope :public_cases, -> { where(public: true) }

# return cases sorted by recently either updated or viewed by the user
scope :recent, -> {
left_outer_joins(:metadata)
Expand Down
10 changes: 7 additions & 3 deletions app/models/team.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@ class Team < ApplicationRecord

has_and_belongs_to_many :search_endpoints,
join_table: 'teams_search_endpoints'

has_and_belongs_to_many :books,
join_table: 'teams_books'

# rubocop:enable Rails/HasAndBelongsToMany

has_many :books, -> { order(name: :asc) },
dependent: :destroy,
inverse_of: :team
# has_many :books, -> { order(name: :asc) },
# dependent: :destroy,
# inverse_of: :team

# has_many :search_endpoints, -> { order(name: :asc) },
# dependent: :destroy,
Expand Down
13 changes: 9 additions & 4 deletions app/views/books/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@
<div class="form-text">The book's name often reflects the goal for the judgements, like <em>Baseline Measurement</em> or a specific problem.</div>
</div>

<div class="mb-3">
<%= form.label :team_id, class: 'form-label' %>
<%= form.collection_select(:team_id, current_user.teams, :id, :name, { prompt: true }, required: true, class: 'form-control') %>
<div class="form-text">Books are meant to be shared with human raters and relevance engineers, access to the book is controlled by what team you pick.</div>
<div class="mb-3">
<%= form.label :teams, "Teams to Share this Book With", class: 'form-label' %>
<%= form.collection_check_boxes(:team_ids, @current_user.teams, :id, :name) do |b| %>
<div class="collection-check-box">
<%= b.check_box %>
<%= b.label %>
</div>
<% end %>
<div class="form-text">Books are meant to be shared with human raters and relevance engineers, and you do this by picking which teams can use them.</div>
</div>

<div class="mb-3">
Expand Down
4 changes: 2 additions & 2 deletions app/views/books/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
<%= form_for(@book, url: assign_anonymous_book_path(@book)) do |form| %>
<div class="mb-3">
<%= form.label :assignee_id, class: 'form-label' %>
<%= select_tag "assignee_id", options_from_collection_for_select(@book.team.members, :id, :name, params.dig(:post, :category_id)), required: true, prompt: "Please select assignee", class: 'form-control' %>
<%= select_tag "assignee_id", options_from_collection_for_select(@book.teams.flat_map(&:members).uniq, :id, :name, params.dig(:post, :category_id)), required: true, prompt: "Please select assignee", class: 'form-control' %>
</div>
<p>
<div class="actions">
Expand All @@ -89,7 +89,7 @@
<%= form_for(@book, url: delete_ratings_by_assignee_book_path(@book), method: :delete, data: { confirm: "Are you sure?" }) do |form| %>
<div class="mb-3">
<%= form.label :user_id, class: 'form-label' %>
<%= select_tag "user_id", options_from_collection_for_select(@book.team.members, :id, :name, params.dig(:post, :category_id)), required: true, prompt: "Please select judge", class: 'form-control' %>
<%= select_tag "user_id", options_from_collection_for_select(@book.teams.flat_map(&:members).uniq, :id, :name, params.dig(:post, :category_id)), required: true, prompt: "Please select judge", class: 'form-control' %>
</div>
<p>
<div class="actions">
Expand Down
4 changes: 2 additions & 2 deletions app/views/books/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
<thead>
<tr>
<th>Name</th>
<th>Team</th>
<th>Teams</th>
<th>Scorer</th>
<th>Selection strategy</th>
<th></th>
Expand All @@ -37,7 +37,7 @@
<% @books.each do |book| %>
<tr>
<td><%= link_to book.name, book_path(book) %></td>
<td><%= book.team.name %></td>
<td><%= book.teams.pluck(:name).join(',') %></td>
<td><%= book.scorer.name %> </td>
<td><%= book.selection_strategy.name %> </td>
<td><span class="badge bg-primary rounded-pill"><%= book.rated_query_doc_pairs.count %> rated query/doc pairs</span></td>
Expand Down
6 changes: 4 additions & 2 deletions app/views/books/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ Books organize all the query/doc pairs that are needed for evaluating your searc
<% end %>

<p>
<strong>Team:</strong>
<%= link_to @book.team.name, teams_core_path(@book.team) %>
<strong>Teams:</strong>
<% @book.teams.each do |team| %>
<%= link_to team.name, teams_core_path(team) %>
<% end %>
</p>

<p>
Expand Down
15 changes: 15 additions & 0 deletions db/migrate/20231212112445_create_books_join_table.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class CreateBooksJoinTable < ActiveRecord::Migration[7.1]
def change
create_join_table :books, :teams, table_name: :teams_books do |t|
end

CreateBooksJoinTable.connection.execute(
"
insert into teams_books (team_id, book_id) select team_id, id from books
"
)

remove_column :books, :team_id

end
end
18 changes: 18 additions & 0 deletions db/migrate/20231213180159_add_owner_id_to_books.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class AddOwnerIdToBooks < ActiveRecord::Migration[7.1]
def change
add_column :books, :owner_id, :integer

AddOwnerIdToBooks.connection.execute(
"
UPDATE books
SET owner_id = (
SELECT tm.member_id
FROM teams_books tb, teams_members tm
WHERE tb.book_id = books.id
AND tb.team_id = tm.team_id
LIMIT 1
);
"
)
end
end
15 changes: 12 additions & 3 deletions db/sample_data_seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def print_user_info info

tmdb_es_endpoint = SearchEndpoint.find_or_create_by search_engine: :es, endpoint_url: "http://quepid-elasticsearch.dev.o19s.com:9206/tmdb/_search", api_method: 'POST'


print_step "End of seeding search endpoints................"

# Users
Expand Down Expand Up @@ -127,6 +126,15 @@ def print_user_info info
realistic_activity_user = seed_user user_params
print_user_info user_params

# go ahead and assign the end point to this person.
statedecoded_solr_endpoint.owner = realistic_activity_user
tmdb_solr_endpoint.owner = realistic_activity_user
tmdb_es_endpoint.owner = realistic_activity_user

statedecoded_solr_endpoint.save
tmdb_solr_endpoint.save
tmdb_es_endpoint.save

######################################
# OSC Team Owner
######################################
Expand Down Expand Up @@ -289,7 +297,6 @@ def print_case_info the_case

new_try = tens_of_queries_case.tries.build try_params


try_number = tens_of_queries_case.last_try_number + 1

new_try.try_number = try_number
Expand Down Expand Up @@ -340,7 +347,9 @@ def print_case_info the_case
# Books
print_step "Seeding books................"

book = Book.where(name: "Book of Ratings", team:osc, scorer: Scorer.system_default_scorer, selection_strategy: SelectionStrategy.find_by(name:'Multiple Raters')).first_or_create
book = Book.where(name: "Book of Ratings", scorer: Scorer.system_default_scorer, selection_strategy: SelectionStrategy.find_by(name:'Multiple Raters')).first_or_create
book.teams << osc
boo.save

# this code copied from populate_controller.rb and should be in a service...
# has a hacked in judgement creator...
Expand Down
9 changes: 7 additions & 2 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions test/controllers/api/v1/books_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class BooksControllerTest < ActionController::TestCase
end

describe 'Updating book' do
let(:the_book) { books(:book_of_star_wars_judgements) }
let(:the_book) { books(:james_bond_movies) }

describe 'when book does not exist' do
test 'returns not found error' do
Expand All @@ -61,7 +61,7 @@ class BooksControllerTest < ActionController::TestCase
describe 'Deleting a book' do
describe 'when it is the last/only case' do
let(:doug) { users(:doug) }
let(:the_book) { books(:book_of_star_wars_judgements) }
let(:the_book) { books(:james_bond_movies) }

before do
login_user doug
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/api/v1/team_books_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class TeamBooksControllerTest < ActionController::TestCase
ids = books.map { |book| book['id'] }

assert_includes ids, book1.id
# assert_includes ids, book2.id
assert_includes ids, book2.id
end
end
end
Expand Down
Loading

0 comments on commit 2b5884b

Please sign in to comment.