Skip to content

Commit

Permalink
fix original verification migration + remove unneccessary update fore…
Browse files Browse the repository at this point in the history
…ign key migration

fix verification request spec
  • Loading branch information
andrew-1234 committed Feb 5, 2025
1 parent 7aa7e93 commit 2833d93
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 65 deletions.
2 changes: 0 additions & 2 deletions app/controllers/verifications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ def filter

private

# Required parameters for verification
# @return [ActionController::Parameters, ActionController::ParameterMissing]
def verification_params
params.require(:verification).permit(
:confirmed, :audio_event_id, :tag_id, :creator_id, :updater_id
Expand Down
39 changes: 15 additions & 24 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -840,27 +840,21 @@ def to_verification(user, _is_guest)
check_audio_event(user, verification.audio_event.audio_recording.site, verification.audio_event)
end

# beware: code smell below. duplication of logic across the blocks. e.g.
# both update and create require :writer level permissions, but only update
# requires creator_id to match user_id. Because creator_id doesn't exist
# during the check, it fails if included, hence, separate blocks.

# the the `reader` user should NOT be able to POST /verifications/ (create)
# the `writer` user SHOULD be able to POST /verifications/ (create)
can [:create], Verification do |verification|
can [:create, :update], Verification do |verification|
check_model(verification)
Access::Core.check_orphan_site!(verification.audio_event.audio_recording.site)
Access::Core.can_any?(user, :writer,
verification.audio_event.audio_recording.site.projects)
end

# the `writer` user SHOULD be able to POST /verifications/{id} (update) IF they
# are the creator (this is `:another_writer` in the verifications_spec)
can [:update], Verification do |verification|
check_model(verification)
Access::Core.check_orphan_site!(verification.audio_event.audio_recording.site)
Access::Core.can_any?(user, :writer,
verification.audio_event.audio_recording.site.projects) && verification.creator_id == user&.id
has_writer_access = Access::Core.can_any?(
user, :writer,
verification.audio_event.audio_recording.site.projects
)

# only check creator_id for persisted records (updates)
if verification.persisted?
has_writer_access && verification.creator_id == user&.id
else
has_writer_access
end
end

# 1. Are you an owner?
Expand All @@ -871,16 +865,13 @@ def to_verification(user, _is_guest)
next false if user_level.blank? || user_level.compact.blank?

actual_highest = Access::Core.highest(user_level)
case actual_highest
when :owner
Access::Core.can_any?(user, :writer,
verification.audio_event.audio_recording.site.projects)
when :writer
if actual_highest == :owner
true
elsif actual_highest == :writer
verification.creator_id == user&.id
else
false
end
end
end
end
# indent
2 changes: 1 addition & 1 deletion db/migrate/20250120064731_create_verifications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def change
create_enum :confirmation, ['true', 'false', 'unsure', 'skip']

create_table :verifications do |t|
t.references :audio_event, null: false, foreign_key: true
t.references :audio_event, null: false, foreign_key: { on_delete: :cascade }
t.references :tag, null: false, foreign_key: true
t.column :creator_id, :integer, null: false
t.column :updater_id, :integer
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4350,7 +4350,6 @@ ALTER TABLE ONLY public.tags
SET search_path TO "$user", public;

INSERT INTO "schema_migrations" (version) VALUES
('20250203074104'),
('20250120064731'),
('20250113012304'),
('20241106015941'),
Expand Down
54 changes: 25 additions & 29 deletions spec/requests/verifications_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,44 +3,40 @@
describe 'Verifications' do
create_entire_hierarchy

before do
create(:verification, audio_event:, creator: owner_user, confirmed: 'true')
create(:verification, audio_event:, creator: writer_user, confirmed: 'false')
end

it 'a reader can list verifications' do
create(:verification)
get '/verifications', **api_headers(reader_token)
expect(response).to have_http_status(:ok)
expect_at_least_one_item
end

context 'when filtering' do
before do
create(:verification, creator: reader_user, confirmed: 'true')
create(:verification, creator: reader_user, confirmed: 'false')
end

let(:verification_skip) { create(:verification, creator: writer_user, confirmed: 'skip') }

it 'can filter verifications by confirmed' do
filter = {
filter: {
confirmed: { eq: verification_skip.confirmed }
}
it 'can filter verifications by confirmed status' do
filter = {
filter: {
confirmed: { eq: 'true' }
}
get '/verifications/filter', params: filter, **api_headers(reader_token)
}
get '/verifications/filter', params: filter, **api_headers(writer_token)

expect(response).to have_http_status(:ok)
expect_number_of_items(1)
expect(api_data).to include(a_hash_including(confirmed: verification_skip.confirmed))
end
expect(response).to have_http_status(:ok)
expect_number_of_items(2)
expect(api_data).to include(a_hash_including(confirmed: 'true'))
end

it 'can filter verifications by creator' do
filter = {
filter: {
creator_id: { not_eq: verification_skip.creator_id }
}
it 'can filter verifications by creator' do
filter = {
filter: {
creator_id: { not_eq: owner_user.id }
}
get '/verifications/filter', params: filter, **api_headers(reader_token)
}
get '/verifications/filter', params: filter, **api_headers(writer_token)

expect(response).to have_http_status(:ok)
expect_number_of_items(2)
expect(api_data).to include(a_hash_including(creator_id: reader_user.id))
end
expect(response).to have_http_status(:ok)
expect_number_of_items(2)
expect(api_data).to include(a_hash_including(creator_id: writer_user.id))
end
end

0 comments on commit 2833d93

Please sign in to comment.