Skip to content

Commit

Permalink
Move naming vote methods into NamingConsensus
Browse files Browse the repository at this point in the history
  • Loading branch information
@nimmolo
nimmolo committed Jan 5, 2024
1 parent 16c5a73 commit 0f07a44
Show file tree
Hide file tree
Showing 15 changed files with 221 additions and 332 deletions.
8 changes: 6 additions & 2 deletions app/controllers/observations/namings/votes_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ class VotesController < ApplicationController
# Linked from: observations/show
# Displayed on show obs via popup for JS users.
# Has its own route for non-js.
# Inputs: params[:id] (naming)
# Inputs: params[:naming_id] (naming)
# Outputs: @naming
def index
pass_query_params
@naming = find_or_goto_index(Naming, params[:naming_id].to_s)
obs = Observation.naming_includes.find(params[:observation_id])
@consensus = Observation::NamingConsensus.new(obs)

respond_to do |format|
format.turbo_stream do
Textile.register_name(@naming.name)
Expand All @@ -22,7 +25,8 @@ def index
render(partial: "shared/modal",
locals: {
identifier: identifier, title: title, subtitle: subtitle,
body: "observations/namings/votes/table", naming: @naming
body: "observations/namings/votes/table", naming: @naming,
consensus: @consensus
})
end
format.html
Expand Down
28 changes: 15 additions & 13 deletions app/controllers/observations/namings_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,21 @@ def update

def destroy
naming = Naming.includes([:votes]).find(params[:id].to_s)
if destroy_if_we_can(naming)
@observation = Observation.naming_includes.find(params[:observation_id])
@consensus = Observation::NamingConsensus.new(@observation)
if destroy_if_we_can(naming) # needs to know consensus before deleting
flash_notice(:runtime_destroy_naming_success.t(id: params[:id].to_s))
end
# Now, eager-load the obs without the deleted naming
obs = Observation.naming_includes.find(params[:observation_id])

respond_to do |format|
format.turbo_stream do
(obs, consensus, owner_name) = locals_for_update_observation(obs)
# Reload after delete
(obs, consensus, owner_name) = locals_for_update_observation
render(partial: "observations/namings/update_observation",
locals: { obs: obs, consensus: consensus,
owner_name: owner_name }) and return
end
format.html { default_redirect(obs) }
format.html { default_redirect(@observation) }
end
end

Expand Down Expand Up @@ -309,7 +310,7 @@ def name_not_changing?
end

def need_new_naming?
!(@naming.editable? || name_not_changing?)
!(@consensus.editable?(@naming) || name_not_changing?)
end

def add_reasons(reasons)
Expand Down Expand Up @@ -363,10 +364,6 @@ def change_vote_with_log
@consensus.change_vote_with_log(@naming, @vote.value)
end

def update_name(user, reasons, was_js_on)
@naming.update_name(@name, user, reasons, was_js_on)
end

def change_vote(new_val)
if new_val && (!@vote || @vote.value != new_val)
@consensus.change_vote(@naming, new_val)
Expand All @@ -382,18 +379,23 @@ def update_naming(reasons, was_js_on)
end

def change_naming
return unless update_name(@user,
params.dig(:naming, :reasons),
return unless update_name(params.dig(:naming, :reasons),
params[:was_js_on] == "yes")

flash_notice(:runtime_naming_updated_at.t)
change_vote(params.dig(:naming, :vote, :value).to_i)
end

def update_name(reasons, was_js_on)
@consensus.clean_votes(@naming, @name, @user)
@naming.create_reasons(reasons, was_js_on)
@naming.update_object(@name, @naming.changed?)
end

def destroy_if_we_can(naming)
if !check_permission!(naming)
flash_error(:runtime_destroy_naming_denied.t(id: naming.id))
elsif !in_admin_mode? && !naming.deletable?
elsif !in_admin_mode? && !@consensus.deletable?(naming)
flash_warning(:runtime_destroy_naming_someone_else.t)
elsif !naming.destroy
flash_error(:runtime_destroy_naming_failed.t(id: naming.id))
Expand Down
16 changes: 10 additions & 6 deletions app/controllers/species_lists/shared_private_methods.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ module SharedPrivateMethods
############################################################################

def find_species_list!
find_or_goto_index(SpeciesList, params[:id].to_s)
# find_or_goto_index(SpeciesList, params[:id].to_s)
SpeciesList.show_includes.safe_find(params[:id].to_s) ||
flash_error_and_goto_index(SpeciesList, params[:id].to_s)
end

# Validate list of names, and if successful, create observations.
Expand Down Expand Up @@ -401,11 +403,13 @@ def init_member_vars_for_edit(spl)
return unless (obs = spl_obss.last)

# Not sure how to check vote efficiently...
@member_vote = begin
obs.namings.first.users_vote(@user).value
rescue StandardError
Vote.maximum_vote
end
consensus = Observation::NamingConsensus.new(obs)
@member_vote =
begin
consensus.users_vote(consensus.namings.first, @user).value
rescue StandardError
Vote.maximum_vote
end
init_member_notes_for_edit(spl_obss)
if all_obs_same_lat_lon_alt?(spl_obss)
@member_lat = obs.lat
Expand Down
3 changes: 1 addition & 2 deletions app/helpers/namings_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ def naming_header_row_content

# NEW - needs a current consensus object
# n+1 should be ok
# but need: obs.consensus_naming and obs.users_favorite?
def observation_namings_table_rows(consensus)
namings = consensus.namings.sort_by(&:created_at)
any_names = consensus.namings&.length&.positive?
Expand All @@ -89,7 +88,7 @@ def observation_namings_table_rows(consensus)
# NEW - needs a current consensus object
# N+1: obs.consensus_naming and observation.owners_favorite?
def naming_row_content(consensus, naming)
vote = naming.users_vote(User.current) || Vote.new(value: 0)
vote = consensus.users_vote(naming, User.current) || Vote.new(value: 0)
consensus_favorite = consensus.consensus_naming
favorite = consensus.owners_favorite?(naming)

Expand Down
189 changes: 30 additions & 159 deletions app/models/naming.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,10 @@
# unique_text_name:: Same as above, with id added to make unique.
# unique_format_name:: Same as above, with id added to make unique.
#
# ==== Voting
# ==== Votes
# votes:: List of Vote's attached to this Naming.
# vote_sum:: Straight sum of Vote's for this Naming.
# vote_percent:: Convert cached Vote score to a percentage.
# user_voted?:: Has a given User voted on this Naming?
# users_vote:: Get a given User's Vote on this Naming.
# users_favorite?:: Is this Naming the given User's favorite?
# change_vote:: Call Observation#change_vote.
# editable?:: Can owner change this Naming's Name?
# deletable?:: Can owner delete this Naming?
# calc_vote_table:: (Used by show_votes.rhtml.)
#
# == Callbacks
#
Expand All @@ -50,7 +43,6 @@
# enforce_default_reasons:: Make sure default reasons are set in if none given.
#
################################################################################
# rubocop:disable Metrics/ClassLength
class Naming < AbstractModel
belongs_to :observation
belongs_to :name
Expand Down Expand Up @@ -85,16 +77,6 @@ def self.construct(args, observation)
naming
end

# N+1: This method does unnecessary lookups. Delete
# def self.from_params(params)
# if params[:id].blank?
# observation = Observation.find(params[:observation_id])
# observation.consensus_naming
# else
# find(params[:id].to_s)
# end
# end

# Update naming and log changes.
def update_object(new_name, log)
self.name = new_name
Expand Down Expand Up @@ -237,37 +219,9 @@ def log_destruction
end
end

def init_reasons(args = nil)
result = {}
reasons_array.each do |reason|
num = reason.num

# Use naming's reasons by default.
result[num] = reason

# Override with values in params.
next unless args

if (x = args[num.to_s])
check = x[:check]
notes = x[:notes]
# Reason is "used" if checked or notes non-empty.
if (check == "1") ||
notes.present?
reason.notes = notes
else
reason.delete
end
else
reason.delete
end
end
result
end

##############################################################################
#
# :section: Voting
# :section: Votes
#
##############################################################################

Expand All @@ -286,116 +240,6 @@ def vote_percent
Vote.percent(vote_cache)
end

# Has a given User voted for this naming?
def user_voted?(user)
!!users_vote(user)
end

# Retrieve a given User's vote for this naming.
# N+1: find_each. Also, we should already have the include here.
def users_vote(user)
# votes.includes(:user).find_each { |v| return v if v.user_id == user.id }
votes.select { |v| return v if v.user_id == user.id }
nil
end

# Is this Naming the given User's favorite Naming for this Observation?
def users_favorite?(user)
votes.any? { |v| v.user_id == user.id && v.favorite }
end

# It is rare, but a single user can end up with multiple votes, for example,
# if two names are merged and a user had voted for both names.
# N+1: Calling Vote.where is a guaranteed new query. Use the votes we have
def owners_vote
# Vote.where(naming_id: id, user_id: user_id).order("value desc").first
votes.select { |v| v.user_id == user_id }.sort_by! { |v| v[:value] }.last
end

# Change User's Vote on this Naming. (Uses Observation#change_vote.)
# def change_vote(value, user = User.current)
# observation.change_vote(self, value, user)
# end

def update_name(new_name, user, reasons, was_js_on)
clean_votes(new_name, user)
create_reasons(reasons, was_js_on)
update_object(new_name, changed?)
end

def clean_votes(new_name, user)
return unless new_name != name

votes.each do |vote|
vote.destroy if vote.user_id != user.id
end
end

# Has anyone voted (positively) on this? We don't want people changing
# the name for namings that the community has voted on. Returns true if no
# one has.
def editable?
votes.each do |v|
return false if (v.user_id != user_id) && v.value.positive?
end
true
end

# Has anyone given this their strongest (positive) vote? We don't want
# people destroying namings that someone else likes best. Returns true if no
# one has.
def deletable?
votes.each do |v|
return false if (v.user_id != user_id) && v.value.positive? && v.favorite
end
true
end

# Create a table the number of User's who cast each level of Vote.
# (This refreshes the vote_cache while it's at it.)
#
# table = naming.calc_vote_table
# for key, record in table
# str = key.l
# num = record[:num] # Number of users who voted near this level.
# weight = record[:wgt] # Sum of users' weights.
# value = record[:value] # Value of this level of vote (arbitrary scale)
# votes = record[:votes] # List of actual votes.
# end
#
# n+1: ? it's a Vote table lookup, but only executed on VotesController#index
def calc_vote_table
# Initialize table.
table = {}
Vote.opinion_menu.each do |str, val|
table[str] = {
num: 0,
wgt: 0.0,
value: val,
votes: []
}
end

# Gather votes, doing a weighted sum in the process.
tot_sum = 0
tot_wgt = 0
votes.includes([:user]).find_each do |v|
str = Vote.confidence(v.value)
wgt = v.user_weight
table[str][:num] += 1
table[str][:wgt] += wgt
table[str][:votes] << v
tot_sum += v.value * wgt
tot_wgt += wgt
end
val = tot_sum.to_f / (tot_wgt + 1.0)

# Update vote_cache if it's wrong.
update!(vote_cache: val) if (vote_cache - val).abs > 1e-4

table
end

##############################################################################
#
# :section: Reasons
Expand Down Expand Up @@ -435,6 +279,34 @@ def reasons_hash
result
end

def init_reasons(args = nil)
result = {}
reasons_array.each do |reason|
num = reason.num

# Use naming's reasons by default.
result[num] = reason

# Override with values in params.
next unless args

if (x = args[num.to_s])
check = x[:check]
notes = x[:notes]
# Reason is "used" if checked or notes non-empty.
if (check == "1") ||
notes.present?
reason.notes = notes
else
reason.delete
end
else
reason.delete
end
end
result
end

# Update reasons given Hash of notes values.
def update_reasons(hash)
reasons_array.each do |reason|
Expand Down Expand Up @@ -549,4 +421,3 @@ def check_requirements # :nodoc:
!User.current
end
end
# rubocop:enable Metrics/ClassLength

0 comments on commit 0f07a44

Please sign in to comment.