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

ensures admin filter api also respects the project filter for site permissions #312

Merged
merged 1 commit into from
Dec 11, 2016
Merged
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
32 changes: 16 additions & 16 deletions lib/modules/access/by_permission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,15 @@ def analysis_jobs_items(analysis_job, user, system_mode = false, levels = Access

if system_mode
query = AnalysisJobsItem.system_query
.joins(audio_recording: :site)
.order(audio_recording_id: :asc)
.joins(audio_recording: :site)
.order(audio_recording_id: :asc)
else
analysis_job = Access::Core.validate_analysis_job(analysis_job)
query = AnalysisJobsItem
.joins(audio_recording: :site)
.order(audio_recording_id: :asc)
.joins(:analysis_job) # this join ensures only non-deleted results are returned
.where(analysis_jobs: {id: analysis_job.id})
.joins(audio_recording: :site)
.order(audio_recording_id: :asc)
.joins(:analysis_job) # this join ensures only non-deleted results are returned
.where(analysis_jobs: {id: analysis_job.id})
end

permission_sites(user, levels, query)
Expand Down Expand Up @@ -237,7 +237,6 @@ def permission_projects(user, levels)
def permission_sites(user, levels, query, project_ids = nil)

is_admin, query = permission_admin(user, levels, query)
return query if is_admin

=begin
EXISTS
Expand All @@ -252,14 +251,11 @@ def permission_sites(user, levels, query, project_ids = nil)
pt = Project.arel_table
ps = Arel::Table.new(:projects_sites)
st = Site.arel_table

# levels_modified is not used because permission_projects also
# calls calculate_levels, which would end up with the inverse results
levels_modified, exists = calculate_levels(levels)

# project permission will never be nil, which is the way it should work
# when being used as a part of a subquery rather than the whole subquery
permissions = permission_projects(user, levels)

permissions_by_site =
ps
.join(pt).on(ps[:project_id].eq(pt[:id]))
Expand All @@ -272,11 +268,15 @@ def permission_sites(user, levels, query, project_ids = nil)
.where(pt[:id].in(project_ids))
end

permissions_by_site =
permissions_by_site
.where(permissions)
.project(1)
.exists
unless is_admin
# project permission will never be nil, which is the way it should work
# when being used as a part of a subquery rather than the whole subquery
permissions = permission_projects(user, levels)
permissions_by_site = permissions_by_site.where(permissions)
end

permissions_by_site = permissions_by_site.project(1).exists

=begin
EXISTS
(SELECT 1
Expand Down
71 changes: 70 additions & 1 deletion spec/lib/filter/query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1413,9 +1413,12 @@ def create_filter(params)
expect(ids_actual).to match_array(ids_expected)
end

it 'overrides filter parameters that match text partial match field' do
it 'overrides filter parameters that match text partial match field for admin' do
# audio_recording needs a site, otherwise it won't be found
# in by_permission#permission_sites
audio_recording = FactoryGirl.create(
:audio_recording,
site: Site.first,
media_type: 'audio/mp3',
recorded_date: '2012-03-26 07:06:59',
duration_seconds: 120)
Expand All @@ -1440,6 +1443,36 @@ def create_filter(params)
expect(ids_actual).to match_array(ids_expected)
end

it 'overrides filter parameters that match text partial match field for writer' do
# audio_recording needs a site, otherwise it won't be found
# in by_permission#permission_sites
audio_recording = FactoryGirl.create(
:audio_recording,
site: Site.first,
media_type: 'audio/mp3',
recorded_date: '2012-03-26 07:06:59',
duration_seconds: 120)

filter = Filter::Query.new(
{filter: {
duration_seconds: {eq: 100},
or: {media_type: {contains: 'wav'}, status: {contains: 'wav'}}
},
filter_duration_seconds: 120,
filter_partial_match: 'mp3'
},
Access::ByPermission.audio_recordings(writer_user),
AudioRecording,
AudioRecording.filter_settings
)

expect(filter.filter).to eq({duration_seconds: {eq: 120}, or: {media_type: {contains: 'mp3'}, status: {contains: 'mp3'}}})

ids_actual = filter.query_full.pluck(:id)
ids_expected = [audio_recording.id]
expect(ids_actual).to match_array(ids_expected)
end

end

context 'available filter items' do
Expand Down Expand Up @@ -1519,4 +1552,40 @@ def create_filter(params)
end
end

context 'project with no sites' do
it 'returns no sites for admin' do
filter_hash = { filter: { } }
project_id = FactoryGirl.create(:project, creator: admin_user).id
filter_query = Access::ByPermission.sites(admin_user, Access::Core.levels, project_id)
filter = Filter::Query.new(
filter_hash,
filter_query,
Site,
Site.filter_settings
)

expect(filter.filter).to eq(filter_hash[:filter])

query = filter.query_full
expect(query.pluck(:id)).to match_array([])
end

it 'returns no sites for regular user' do
filter_hash = { filter: { } }
project_id = FactoryGirl.create(:project, creator: writer_user).id
filter_query = Access::ByPermission.sites(writer_user, Access::Core.levels, project_id)
filter = Filter::Query.new(
filter_hash,
filter_query,
Site,
Site.filter_settings
)

expect(filter.filter).to eq(filter_hash[:filter])

query = filter.query_full
expect(query.pluck(:id)).to match_array([])
end
end

end