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

Fix/refactor a few memoizations #4945

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,31 @@ class ApplicationController < ActionController::Base
rescue_from ActiveRecord::RecordNotFound, with: :not_found!

def current_organization
return @current_organization if @current_organization
return nil unless current_role
return @current_organization if defined? @current_organization

return current_role.resource if current_role&.resource&.is_a?(Organization)
return @current_organization = nil unless current_role
return @current_organization = current_role.resource if current_role&.resource&.is_a?(Organization)

Organization.find_by(short_name: params[:organization_name])
@current_organization = Organization.find_by(short_name: params[:organization_name])
end
helper_method :current_organization

def current_partner
return nil unless current_role
return nil if current_role.name.to_sym != Role::PARTNER
return @current_partner if defined? @current_partner

current_role.resource
return @current_partner = nil unless current_role
return @current_partner = nil if current_role.name.to_sym != Role::PARTNER

@current_partner = current_role.resource
end
helper_method :current_partner

def current_role
return @role if @role
return nil unless current_user
return @role if defined? @role

@role = Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user)
return @role = nil unless current_user

@role
@role = Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user)
end

def dashboard_path_from_current_role
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,13 @@ def remove_category
private

def clean_item_value_in_cents
return nil unless params[:item][:value_in_cents]
return unless params[:item][:value_in_cents]

params[:item][:value_in_cents] = params[:item][:value_in_cents].gsub(/[$,.]/, "")
end

def clean_item_value_in_dollars
return nil unless params[:item][:value_in_dollars]
return unless params[:item][:value_in_dollars]

params[:item][:value_in_cents] = params[:item][:value_in_dollars].gsub(/[$,]/, "").to_d * 100
params[:item].delete(:value_in_dollars)
Expand Down
1 change: 0 additions & 1 deletion app/services/calendar_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,5 @@ def self.time_zones
end

@zones = zones.map { |z| ["#{z.name} #{z.formatted_offset}", z.tzinfo.name] }
@zones
end
end
16 changes: 7 additions & 9 deletions app/services/distribution_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ def success?
private

def distribution_organization
@distribution_organization ||= distribution&.organization
return @distribution_organization if defined? @distribution_organization

@distribution_organization = distribution&.organization
end

def set_error(error)
Expand All @@ -35,21 +37,17 @@ def set_error(error)

def distribution
# Return distribution if it has already been defined
return @distribution if @distribution
return @distribution if defined? @distribution

# Otherwise try to get this value with possibly
# provided distribution_id from initialize
if @distribution_id.present?
@distribution = Distribution.find(@distribution_id)
end
@distribution = @distribution_id.present? ? Distribution.find(@distribution_id) : nil
end

def distribution_id
return @distribution_id if @distribution_id
return @distribution_id if defined? @distribution_id

if distribution.present?
@distribution_id = distribution.id
end
@distribution_id = distribution&.id
end
end

2 changes: 0 additions & 2 deletions app/services/exports/export_request_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ def build_row_data(request)
end

def all_item_requests
return @all_item_requests if @all_item_requests
@all_item_requests ||= Partners::ItemRequest.where(request: requests).includes(item: :request_units)
@all_item_requests
end
end
end
Loading