Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
da64de8
standardrb fix unsafely
caseyhelbling Apr 22, 2025
0d3dd77
Fix static controller spec
caseyhelbling Apr 22, 2025
91845e6
Adds standard rails gem
caseyhelbling Apr 22, 2025
d3d2ae2
fix safely
caseyhelbling Apr 22, 2025
9f67723
Fix unsafely
caseyhelbling Apr 22, 2025
2c50490
Fix remaining lint errors
caseyhelbling Apr 23, 2025
4338657
Get more tests passing
caseyhelbling Apr 23, 2025
8334741
Get more tests passing
caseyhelbling Apr 23, 2025
0a4513d
Fix more tests
caseyhelbling Apr 23, 2025
b43686d
More test fixes
caseyhelbling Apr 23, 2025
d4b0ef8
Fix bad rebase
caseyhelbling Apr 23, 2025
d36a8b1
Fix call signature
caseyhelbling Apr 23, 2025
88cd90a
Fix file name
caseyhelbling Apr 23, 2025
e3c6f28
Mark some tests pending that don't passing in CI env but do pass in t…
caseyhelbling Apr 24, 2025
f3bc07f
Merge branch 'run-standardrb' into feature/clh/add-standardrb
wwahammy Apr 28, 2025
9203ce9
Merge branch 'run-standardrb' into feature/clh/add-standardrb
wwahammy Apr 29, 2025
7ac3eef
Merge branch 'run-standardrb' into feature/clh/add-standardrb
wwahammy Apr 29, 2025
c2963cf
Merge branch 'supporter_level_goal' into feature/clh/add-standardrb
wwahammy Apr 30, 2025
aa35d11
Merge branch 'add-standard.yml' into feature/clh/add-standardrb
wwahammy Apr 30, 2025
82dd5bb
Merge branch 'supporter_level_goal' into feature/clh/add-standardrb
wwahammy Apr 30, 2025
5234251
Merge branch 'supporter_level_goal' into feature/clh/add-standardrb
wwahammy May 5, 2025
056babe
Merge branch 'supporter_level_goal' into feature/clh/add-standardrb
wwahammy May 17, 2025
093b8a8
Switch two files to belongs_to optional: false
wwahammy May 18, 2025
16fe3b3
Merge branch 'supporter_level_goal' into feature/clh/add-standardrb
wwahammy May 27, 2025
9f86ab6
Merge branch 'supporter_level_goal' into feature/clh/add-standardrb
wwahammy May 29, 2025
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
8 changes: 5 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@ def set_locale
end
end

# rubocop:disable Style/UnlessLogicalOperators
def redirect_to_maintenance
if Settings&.maintenance&.maintenance_mode && !current_user
unless self.class == Users::SessionsController &&
unless instance_of?(Users::SessionsController) &&
((Settings.maintenance.maintenance_token && params[:maintenance_token] == Settings.maintenance.maintenance_token) || params[:format] == "json")
redirect_to Settings.maintenance.maintenance_page,
allow_other_host: true
end
end
end
# rubocop:enable Style/UnlessLogicalOperators

protected

Expand Down Expand Up @@ -62,7 +64,7 @@ def render_json(&block)
rescue ExpiredTokenError => e
logger.info "422: #{e}".red.bold
result = {status: 422, json: {error: e.message}}
rescue Exception => e # a non-validation related exception
rescue => e # a non-validation related exception
logger.error "500: #{e}".red.bold
logger.error e.backtrace.take(5).map { |l| ">>".red.bold + " #{l}" }.join("\n").red
result = {status: 500, json: {error: e.message, backtrace: e.backtrace}}
Expand Down Expand Up @@ -147,7 +149,7 @@ def after_inactive_sign_up_path_for(resource)
private

def current_user_id
current_user && current_user.id
current_user&.id
end

# Overload handle_unverified_request to ensure that
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/campaigns/donations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class DonationsController < ApplicationController
def index
respond_to do |format|
format.csv do
file_date = Date.today.strftime("%m-%d-%Y")
file_date = Time.zone.today.strftime("%m-%d-%Y")
donations = QueryDonations.campaign_export(current_campaign.id)
send_data(Format::Csv.from_vectors(donations), filename: "campaign-donations-#{file_date}.csv")
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/campaigns_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def create
json_saved campaign, "Campaign created! Well done."
else
profile_id = params[:campaign][:profile_id]
Profile.find(profile_id).update_attributes params[:profile]
Profile.find(profile_id).update params[:profile]
render json: CreatePeerToPeerCampaign.create(params[:campaign], profile_id)
end
end
Expand All @@ -76,7 +76,7 @@ def update
Time.use_zone(current_nonprofit.timezone || "UTC") do
params[:campaign][:end_datetime] = Chronic.parse(params[:campaign][:end_datetime]) if params[:campaign][:end_datetime].present?
end
current_campaign.update_attributes params[:campaign]
current_campaign.update params[:campaign]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard wants you to call update instead of update_attributes.


json_saved current_campaign, "Successfully updated!"
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ def leaf?
end
end

def self.create_from(root_tree_node)
er = ExpansionTree.new
er.root_node = root_tree_node
er
end

Comment on lines +406 to +411
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened here? Why is this public instead of private?

private

# given a set of SPaths, build a tree to describe
Expand All @@ -415,11 +421,5 @@ def parse_paths(paths = [])
end
end
end

def self.create_from(root_tree_node)
er = ExpansionTree.new
er.root_node = root_tree_node
er
end
end
end
2 changes: 1 addition & 1 deletion app/controllers/events_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def update
params[:event][:start_datetime] = Chronic.parse(params[:event][:start_datetime]) if params[:event][:start_datetime].present?
params[:event][:end_datetime] = Chronic.parse(params[:event][:end_datetime]) if params[:event][:end_datetime].present?
end
current_event.update_attributes(params[:event])
current_event.update(params[:event])
json_saved current_event, "Successfully updated"
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/image_attachments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def create
end

def remove
@image = ImageAttachment.select { |img| img.file_url == params[:src] }.first
@image = ImageAttachment.find { |img| img.file_url == params[:src] }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find instead of select . first

if @image
@image.destroy
render json: @image
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def modify
end

supporter_ids = if params[:selecting_all]
QuerySupporters.full_filter_expr(current_nonprofit.id, params[:query]).select("supporters.id").execute.map { |h| h["id"] }
QuerySupporters.full_filter_expr(current_nonprofit.id, params[:query]).select("supporters.id").execute.pluck("id")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Veify pluck works here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will. It would be nice if all of these were tested though.

else
params[:supporter_ids].map(&:to_i)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/nonprofits/nonprofit_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def mailchimp_landing
session.delete(:current_mailchimp_nonprofit_id)
begin
session[:mailchimp_access_token] = InsertNonprofitKeys.insert_mailchimp_access_token(@nonprofit.id, params[:code])
rescue Exception => e
rescue => e
flash[:notice] = "Unable to connect to your Mailchimp account, please try again. (Error: #{e})"
redirect_to "/settings"
return
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/nonprofits/payments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def show

def update
@payment = current_nonprofit.payments.find(params[:id])
@payment.update_attributes(params[:payment])
@payment.update(params[:payment])
json_saved @payment
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/nonprofits/stripe_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def begin_verification
StripeAccountUtils.find_or_create(current_nonprofit.id)
current_nonprofit.reload

status = NonprofitVerificationProcessStatus.where("stripe_account_id = ?", current_nonprofit.stripe_account_id).first
status = NonprofitVerificationProcessStatus.where(stripe_account_id: current_nonprofit.stripe_account_id).first
status ||= NonprofitVerificationProcessStatus.new(stripe_account_id: current_nonprofit.stripe_account_id)

unless status.started_at
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/nonprofits/supporter_notes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def create

# put /nonprofits/:nonprofit_id/supporters/:supporter_id/supporter_notes/:id
def update
params[:supporter_note][:user_id] ||= current_user && current_user.id
params[:supporter_note][:user_id] ||= current_user&.id
params[:supporter_note][:id] = params[:id]
render_json { UpdateSupporterNotes.update(params[:supporter_note]) }
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/nonprofits/supporters_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Nonprofits
class SupportersController < ApplicationController
include Controllers::NonprofitHelper

before_action :authenticate_nonprofit_user!, except: [:new, :create]
before_action :authenticate_nonprofit_user!, except: [:create]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change?


before_action :validate_allowed!, only: [:create]
rescue_from ::TempBlockError, with: :handle_temp_block_error
Expand All @@ -19,7 +19,7 @@ def index
end

format.csv do
file_date = Date.today.strftime("%m-%d-%Y")
file_date = Time.zone.today.strftime("%m-%d-%Y")
supporters = QuerySupporters.for_export(params[:nonprofit_id], params)
send_data(Format::Csv.from_vectors(supporters), filename: "supporters-#{file_date}.csv")
end
Expand Down Expand Up @@ -82,7 +82,7 @@ def update

def bulk_delete
supporter_ids = if params[:selecting_all]
QuerySupporters.full_filter_expr(current_nonprofit.id, params[:query]).select("supporters.id").execute.map { |h| h["id"] }
QuerySupporters.full_filter_expr(current_nonprofit.id, params[:query]).select("supporters.id").execute.pluck("id")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pluck?

Copy link
Member

@wwahammy wwahammy Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be ideal if we had a spec to verify this Controller action

else
params[:supporter_ids].map(&:to_i)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/nonprofits/tag_joins_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def index
# selected supporters' tags or all supporters' tags
def modify
supporter_ids = if params[:selecting_all]
QuerySupporters.full_filter_expr(current_nonprofit.id, params[:query]).select("supporters.id").execute.map { |h| h["id"] }
QuerySupporters.full_filter_expr(current_nonprofit.id, params[:query]).select("supporters.id").execute.pluck("id")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pluck?

Copy link
Member

@wwahammy wwahammy Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be ideal if we had a spec to verify this Controller action

else
params[:supporter_ids].map(&:to_i)
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/nonprofits_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def create

def update
flash[:notice] = "Update successful!"
current_nonprofit.update_attributes params[:nonprofit].except(:verification_status)
current_nonprofit.update params[:nonprofit].except(:verification_status)
current_nonprofit.clear_cache
json_saved current_nonprofit
end
Expand Down Expand Up @@ -129,7 +129,7 @@ def countries_list(locale)
all_countries = ISO3166::Country.translations(locale)

if Settings.intntl.all_countries
countries = all_countries.select { |code, name| Settings.intntl.all_countries.include? code }
countries = all_countries.slice(*Settings.intntl.all_countries)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slice over select

countries = countries.map { |code, name| [code.upcase, name] }.sort_by { |a| a[1] }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort_by

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about sort_by?

countries.push([Settings.intntl.other_country.upcase, I18n.t("nonprofits.donate.info.supporter.other_country")]) if Settings.intntl.other_country
countries
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def update
else
current_user.profile
end
@profile.update_attributes(params[:profile])
@profile.update(params[:profile])
json_saved @profile, "Profile updated"
end

Expand Down
6 changes: 3 additions & 3 deletions app/controllers/recurring_donations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def update
end

def update_amount
rd = RecurringDonation.where("id = ?", params[:id]).first
rd = RecurringDonation.where(id: params[:id]).first
if rd && params[:edit_token] == rd["edit_token"]
begin
amount_response = UpdateRecurringDonations.update_amount(rd, params[:token], params[:amount], params[:fee_covered])
Expand All @@ -61,8 +61,8 @@ def update_amount

def print_currency(cents, unit = "EUR", sign = true)
dollars = cents.to_f / 100.0
dollars = view_context.number_to_currency(dollars, unit: "#{unit}", precision: (dollars.round == dollars) ? 0 : 2)
dollars = dollars[1..-1] if !sign
dollars = view_context.number_to_currency(dollars, unit: unit.to_s, precision: (dollars.round == dollars) ? 0 : 2)
dollars = dollars[1..] if !sign
dollars
end
end
30 changes: 19 additions & 11 deletions app/controllers/static_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,27 @@ def terms_and_privacy
end

def ccs
ccs_method = (!Settings.ccs) ? "local_tar_gz" : Settings.ccs.ccs_method
if ccs_method == "local_tar_gz"
temp_file = "#{Rails.root.join("tmp/#{Time.current.to_i}.tar.gz")}"
result = Kernel.system("git archive --format=tar.gz -o #{temp_file} HEAD")
if result
send_file(temp_file, type: "application/gzip")
else
head 500
end
elsif ccs_method == "github"
git_hash = File.read("#{Rails.root.join("CCS_HASH")}")
if Settings.ccs&.ccs_method.presence == "github"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty significant manual refactor of this method ... deserves close review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks right

redirect_to "https://github.com/#{Settings.ccs.options.account}/#{Settings.ccs.options.repo}/tree/#{git_hash}",
allow_other_host: true
elsif create_archive
send_file(temp_file, type: "application/gzip")
else
head 500
end
end

private

def git_hash
@git_hash ||= File.read(Rails.root.join("CCS_HASH").to_s)
end

def temp_file
@temp_file ||= Rails.root.join("tmp/#{Time.current.to_i}.tar.gz").to_s
end

def create_archive
Kernel.system("git archive --format=tar.gz -o #{temp_file} HEAD")
end
end
6 changes: 3 additions & 3 deletions app/controllers/super_admins_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def search_profiles
def search_fullcontact
begin
result = FullContact.person(email: params[:search])
rescue Exception
rescue
result = ""
end
render json: [result]
Expand All @@ -29,7 +29,7 @@ def resend_user_confirmation
profile_id: {required: true, is_integer: true}
})

profile = Profile.includes(:user).where("id = ?", params[:profile_id]).first
profile = Profile.includes(:user).where(id: params[:profile_id]).first
unless profile.user
raise ArgumentError.new("#{params[:profile_id]} is a profile without a valid user")
end
Expand All @@ -56,7 +56,7 @@ def recurring_donations_without_cards
}
}

send_data(csv_out, filename: "recurring_donations_without_cards-#{Time.now.to_date}.csv")
send_data(csv_out, filename: "recurring_donations_without_cards-#{Time.zone.now.to_date}.csv")
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/ticket_levels_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def create
end

def update
current_ticket_level.update_attributes params[:ticket_level]
current_ticket_level.update params[:ticket_level]
json_saved current_ticket_level, "Ticket level updated"
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/tickets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def index
respond_to do |format|
format.html
format.csv do
file_date = Date.today.strftime("%m-%d-%Y")
file_date = Time.zone.today.strftime("%m-%d-%Y")
filename = "tickets-#{file_date}"
@tickets = QueryTickets.for_export(@event.id, params)
send_data(Format::Csv.from_vectors(@tickets), filename: "#{filename}.csv")
Expand All @@ -44,7 +44,7 @@ def index

# PUT nonprofits/:nonprofit_id/events/:event_id/tickets/:id/add_note
def add_note
current_nonprofit.tickets.find(params[:id]).update_attributes(note: params[:ticket][:note])
current_nonprofit.tickets.find(params[:id]).update(note: params[:ticket][:note])
render json: {}
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/confirmations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def exists
def confirm
@user = User.find(params[:id])

if @user.valid? && @user.update_attributes(params[:user].except(:confirmation_token))
if @user.valid? && @user.update(params[:user].except(:confirmation_token))
flash[:notice] = "Your account is all set!"
sign_in @user
redirect_to session[:donor_signup_url] || root_url
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def update
if current_user.pending_password && params[:user][:password] && params[:user][:password_confirmation]
params[:user][:pending_password] = false
end
success = current_user.update_attributes(params[:user])
success = current_user.update(params[:user])
errs = current_user.errors.full_messages
else
success = false
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ def devise_mapping

def print_currency(cents, unit = "EUR", sign = true)
dollars = cents.to_f / 100.0
dollars = number_to_currency(dollars, unit: "#{unit}", precision: (dollars.round == dollars) ? 0 : 2)
dollars = dollars[1..-1] if !sign
dollars = number_to_currency(dollars, unit: unit.to_s, precision: (dollars.round == dollars) ? 0 : 2)
dollars = dollars[1..] if !sign
dollars
end

Expand Down
2 changes: 1 addition & 1 deletion app/helpers/card_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
module CardHelper
def expiration_years
(0..15).map { |n| (Date.today + n.years).year }
(0..15).map { |n| (Time.zone.today + n.years).year }
end
end
2 changes: 1 addition & 1 deletion app/jobs/inline_job.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
# newer versions of Rails use an ApplicationJob so let's be cool like them
class InlineJob < ActiveJob::Base
class InlineJob < ApplicationJob
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inherit ApplicatoinJob ... Should there be a queue_adapter call?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what was going on here. That said, I'm not sure we can safely switch this to ApplicationJob. I think we should switch this back and evaluate at a different time.

:inline
end
2 changes: 1 addition & 1 deletion app/jobs/mailchimp_nonprofit_user_add_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later

class MailchimpNonprofitUserAddJob < ActiveJob::Base
class MailchimpNonprofitUserAddJob < ApplicationJob
queue_as :default

def perform(user, nonprofit)
Expand Down
Loading
Loading