Skip to content

Commit

Permalink
Remove remaining warnings on unpermitted params
Browse files Browse the repository at this point in the history
  • Loading branch information
MrSerth committed Oct 15, 2024
1 parent 4ad3ca4 commit 7385f93
Show file tree
Hide file tree
Showing 16 changed files with 51 additions and 39 deletions.
2 changes: 1 addition & 1 deletion app/assets/javascripts/labels_index.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class LabelsTable
$.ajax({
url: "/labels/#{id}",
type: "PATCH",
data: {color: new_color}
data: {label: {color: new_color}}
})
);
$.when.apply($, requests).then(@reload_table);
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/task_parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def task_params
end

def group_tasks_params
params.permit(group_tasks: {group_ids: []})[:group_tasks]
params.require(:group_tasks).permit(group_ids: [])
end

def import_confirm_params
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/labels_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class LabelsController < ApplicationController
def index; end

def update
if @label.update(params.permit(:color))
if @label.update(label_params)
render json: @label.to_h
else
flash.now[:alert] = @label.errors.full_messages
Expand Down Expand Up @@ -49,6 +49,10 @@ def search # rubocop:disable Metrics/AbcSize

private

def label_params
params.require(:label).permit(:color)
end

def load_and_authorize_label
@label = Label.find(params[:id])
authorize @label
Expand Down
7 changes: 2 additions & 5 deletions app/controllers/messages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,10 @@ def new

# rubocop:disable Metrics/AbcSize
def create
recipient = params.require(:message).delete(:recipient)
@message = Message.new(message_params)
@message.sender = current_user
@message.recipient = if params[:message][:recipient]
User.find_by(email: params[:message][:recipient])
else
User.find(params[:message][:recipient_hidden])
end
@message.recipient = User.find_by(email: recipient) if recipient.present?
authorize @message

if @message.save
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/users/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ class RegistrationsController < Devise::RegistrationsController

# PUT /resource
def update
avatar_present = params.require(:user).delete(:avatar_present)
super do |resource|
resource.avatar.purge if params[:user][:avatar].nil? && params[:user][:avatar_present] == 'false'
resource.avatar.purge if params[:user][:avatar].nil? && avatar_present == 'false'
if resource.password_set_changed?
# If a user tried to set a password but failed, we need to reset the password_set flag.
# Further, we need to remove the current_password error, since the user didn't enter their current password.
Expand Down
2 changes: 1 addition & 1 deletion app/views/messages/_form.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
= render('shared/form_errors', object: @message)

- if @recipient
= f.hidden_field :recipient_hidden, value: @recipient.id
= f.hidden_field :recipient_id, value: @recipient.id
- else
.field-element
= f.label :recipient, Message.human_attribute_name('recipient'), class: 'form-label'
Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/account_links_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
let(:account_link) { create(:account_link, user:) }
let(:account_link_from_another_user) { create(:account_link, user: another_user) }

let(:valid_attributes) { attributes_for(:account_link).merge(user:) }
let(:valid_attributes) { attributes_for(:account_link) }
let(:invalid_attributes) do
{api_key: ''}
end
Expand Down Expand Up @@ -65,14 +65,14 @@

context 'with valid attributes' do
it 'updates the requested account_link' do
account_link = AccountLink.create! valid_attributes
account_link = AccountLink.create! valid_attributes.merge(user:)
put :update, params: empty_params.merge(id: account_link.to_param, account_link: new_attributes)
account_link.reload
expect(account_link.api_key).to eq(new_attributes[:api_key])
end

it 'redirects to the user' do
account_link = AccountLink.create! valid_attributes
account_link = AccountLink.create! valid_attributes.merge(user:)
put :update, params: empty_params.merge(id: account_link.to_param, account_link: valid_attributes)
expect(response).to redirect_to(user)
end
Expand All @@ -82,7 +82,7 @@
describe 'DELETE #destroy' do
include_examples 'destroy examples', klass: AccountLink, resource: :account_link
it 'with valid attributes redirects to the user' do
account_link = AccountLink.create! valid_attributes
account_link = AccountLink.create! valid_attributes.merge(user:)
delete :destroy, params: empty_params.merge(id: account_link.to_param)
expect(response).to redirect_to(user)
end
Expand Down
15 changes: 10 additions & 5 deletions spec/controllers/collections_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
render_views

let(:valid_attributes) do
attributes_for(:collection)
build(:collection).attributes.except('id', 'created_at', 'updated_at')
end

let(:invalid_attributes) do
Expand Down Expand Up @@ -274,6 +274,11 @@

context 'with valid params' do
let(:collection_params) { valid_attributes }
let(:collection_tasks_attributes) do
collection.collection_tasks.map do |collection_task|
collection_task.attributes.symbolize_keys.except(:collection_id, :created_at, :task_id, :updated_at)
end
end

it 'assigns the requested collection as @collection' do
put_update
Expand All @@ -296,8 +301,8 @@
context 'with new task order' do
let(:collection_params) do
{collection_tasks_attributes: {
'0': collection.collection_tasks.first.attributes.merge(rank: 0),
'1': collection.collection_tasks.second.attributes.merge(rank: 1),
'0': collection_tasks_attributes.first.merge(rank: 0),
'1': collection_tasks_attributes.second.merge(rank: 1),
}}
end

Expand All @@ -309,8 +314,8 @@
context 'when removing a task' do
let(:collection_params) do
{collection_tasks_attributes: {
'0': collection.collection_tasks.first.attributes.merge(rank: 0),
'1': collection.collection_tasks.second.attributes.merge(_destroy: 1),
'0': collection_tasks_attributes.first.merge(rank: 0),
'1': collection_tasks_attributes.second.merge(_destroy: 1),
}}
end

Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/groups_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
let(:admin_locale) { :de }

let(:valid_post_attributes) do
attributes_for(:group, users: [user])
build(:group, users: [user]).attributes.except('id', 'created_at', 'updated_at')
end

let(:invalid_attributes) do
Expand Down
17 changes: 10 additions & 7 deletions spec/controllers/tasks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@
let(:valid_params) do
{
title: 'title',
descriptions_attributes: {'0' => {text: 'description', primary: true}},
description: 'description',
programming_language_id: create(:programming_language, :ruby).id,
license_id: create(:license),
language: 'de',
Expand Down Expand Up @@ -387,13 +387,12 @@
let(:existing_label) { create(:label) }
let(:new_label) { create(:label) }

let!(:task) { create(:task, valid_attributes) }
let!(:task) { create(:task, valid_attributes.merge(user:)) }
let(:valid_attributes) do
{
user:,
title: 'title',
license: create(:license, name: 'old_license'),
labels: [existing_label],
license_id: create(:license, name: 'old_license').id,
label_names: [existing_label.name],
}
end
let(:changed_attributes) do
Expand Down Expand Up @@ -471,9 +470,13 @@

let(:test) { build(:test) }
let(:new_testing_framework) { create(:testing_framework) }
let!(:task) { create(:task, valid_attributes.merge(tests: [test])) }
let!(:task) { create(:task, valid_attributes.merge(tests: [test], user:)) }

let(:tests_attributes) { {'0': test.attributes.symbolize_keys.merge(title: 'new_test_title', testing_framework_id: new_testing_framework.id)} }
let(:tests_attributes) do
{'0': test.attributes.symbolize_keys
.except(:configuration, :created_at, :meta_data, :task_id, :updated_at)
.merge(title: 'new_test_title', testing_framework_id: new_testing_framework.id)}
end

it 'updates the requested task' do
expect { put_update }.to change { task.reload.title }.to('new_title')
Expand Down
2 changes: 2 additions & 0 deletions spec/factories/collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
FactoryBot.define do
factory :collection do
title { 'Some Collection' }
description { 'Some Description' }
visibility_level { :private }
users { build_list(:user, 1) }
tasks { build_list(:task, 2) }
end
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/collections_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
let(:user) { create(:user) }
let(:task) { create(:task) }
let(:collection) { create(:collection, title: 'Some Collection', users: [user], tasks: [task]) }
let(:collection_params) { attributes_for(:collection) }
let(:collection_params) { build(:collection).attributes.except('id', 'created_at', 'updated_at') }

before do
sign_in user
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/groups_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
context 'when logged in' do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:group_params) { attributes_for(:group) }
let(:group_params) { build(:group).attributes.except('id', 'created_at', 'updated_at') }

before do
group.add(user, role: :admin)
Expand Down
8 changes: 4 additions & 4 deletions spec/requests/tasks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
let(:valid_params) do
{
title: 'title',
descriptions_attributes: {'0' => {text: 'description', primary: true}},
programming_language: create(:programming_language, :ruby).id,
description: 'description',
programming_language_id: create(:programming_language, :ruby).id,
license_id: create(:license).id,
language: 'de',
}
Expand All @@ -19,8 +19,8 @@
let(:update_params) do
{
title: 'new_title',
descriptions_attributes: {'0' => {text: 'description'}},
programming_language: create(:programming_language, :ruby).id,
description: 'description',
programming_language_id: create(:programming_language, :ruby).id,
license_id: create(:license).id,
}
end
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/users_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
end

context 'when logged in' do
let(:user_params) { attributes_for(:user, current_password: user.password) }
let(:user_params) { attributes_for(:user, current_password: user.password).except(:confirmed_at) }

before do
sign_in user
Expand Down
10 changes: 5 additions & 5 deletions spec/support/shared_examples/standard_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

RSpec.shared_examples 'edit examples' do |klass:, resource:|
it "assigns the requested #{resource} as @#{resource}" do
object = klass.create! valid_attributes
object = klass.create! valid_attributes.merge(user:)
get :edit, params: empty_params.merge(id: object.to_param)
expect(assigns(resource)).to eq(object)
end
Expand Down Expand Up @@ -70,21 +70,21 @@
RSpec.shared_examples 'update examples' do |klass:, resource:|
context 'with valid params' do
it "assigns the requested #{resource} as @#{resource}" do
object = klass.create! valid_attributes
object = klass.create! valid_attributes.merge(user:)
put :update, params: empty_params.merge(:id => object.to_param, resource => valid_attributes)
expect(assigns(resource)).to eq(object)
end
end

context 'with invalid params' do
it "assigns the #{resource} as @#{resource}" do
object = klass.create! valid_attributes
object = klass.create! valid_attributes.merge(user:)
put :update, params: empty_params.merge(:id => object.to_param, resource => invalid_attributes)
expect(assigns(resource)).to eq(object)
end

it "re-renders the 'edit' template" do
object = klass.create! valid_attributes
object = klass.create! valid_attributes.merge(user:)
put :update, params: empty_params.merge(:id => object.to_param, resource => invalid_attributes)
expect(response).to render_template('edit')
end
Expand All @@ -93,7 +93,7 @@

RSpec.shared_examples 'destroy examples' do |klass:, resource:|
it "destroys the requested #{resource}" do
object = klass.create! valid_attributes
object = klass.create! valid_attributes.merge(user:)
expect do
delete :destroy, params: empty_params.merge(id: object.to_param)
end.to change(klass, :count).by(-1)
Expand Down

0 comments on commit 7385f93

Please sign in to comment.