Skip to content

Commit 491c728

Browse files
authored
Add project context route (#531)
## Status closes RaspberryPiFoundation/digital-editor-issues#479 ## What's changed? - Added project context route to allow apps to request contextual data about a project. This will return the school, class and lesson ids for the project. - Stopped students from viewing their work if the associated lesson is no longer visible to students
1 parent ef08fba commit 491c728

File tree

9 files changed

+281
-13
lines changed

9 files changed

+281
-13
lines changed

app/controllers/api/projects/remixes_controller.rb

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module Projects
55
class RemixesController < ApiController
66
before_action :authorize_user
77
load_and_authorize_resource :school, only: :index
8+
before_action :load_and_authorize_remix, only: %i[show]
89

910
def index
1011
projects = Project.where(remixed_from_id: project.id).accessible_by(current_ability)
@@ -13,8 +14,6 @@ def index
1314
end
1415

1516
def show
16-
@project = Project.find_by!(remixed_from_id: project.id, user_id: current_user&.id)
17-
1817
render '/api/projects/show', formats: [:json]
1918
end
2019

@@ -40,6 +39,11 @@ def project
4039
@project ||= Project.find_by!(identifier: params[:project_id])
4140
end
4241

42+
def load_and_authorize_remix
43+
@project = Project.find_by!(remixed_from_id: project.id, user_id: current_user&.id)
44+
authorize! :show, @project
45+
end
46+
4347
def remix_params
4448
params.require(:project)
4549
.permit(:name,

app/controllers/api/projects_controller.rb

+6-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
module Api
66
class ProjectsController < ApiController
77
before_action :authorize_user, only: %i[create update index destroy]
8-
before_action :load_project, only: %i[show update destroy]
8+
before_action :load_project, only: %i[show update destroy show_context]
99
before_action :load_projects, only: %i[index]
1010
load_and_authorize_resource
1111
before_action :verify_lesson_belongs_to_school, only: :create
@@ -52,6 +52,11 @@ def destroy
5252
head :ok
5353
end
5454

55+
# Returns the identifier, school_id, lesson_id, and class_id of the project so the full context can be loaded
56+
def show_context
57+
render :context, formats: [:json]
58+
end
59+
5560
private
5661

5762
def verify_lesson_belongs_to_school

app/models/ability.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def define_school_owner_abilities(school:)
6060
can(%i[read update destroy], School, id: school.id)
6161
can(%i[read], :school_member)
6262
can(%i[read create update destroy], SchoolClass, school: { id: school.id })
63-
can(%i[read], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
63+
can(%i[read show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
6464
can(%i[read create create_batch destroy], ClassStudent, school_class: { school: { id: school.id } })
6565
can(%i[read create destroy], :school_owner)
6666
can(%i[read create destroy], :school_teacher)
@@ -85,7 +85,7 @@ def define_school_teacher_abilities(user:, school:)
8585
can(%i[create], Project) do |project|
8686
school_teacher_can_manage_project?(user:, school:, project:)
8787
end
88-
can(%i[read update], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
88+
can(%i[read update show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
8989
can(%i[read], Project,
9090
remixed_from_id: Project.where(school_id: school.id, remixed_from_id: nil, lesson_id: Lesson.where(school_class_id: ClassTeacher.where(teacher_id: user.id).select(:school_class_id))).pluck(:id))
9191
end
@@ -95,8 +95,8 @@ def define_school_student_abilities(user:, school:)
9595
can(%i[read], SchoolClass, school: { id: school.id }, students: { student_id: user.id })
9696
# Ensure no access to ClassMember resources, relationships otherwise allow access in some circumstances.
9797
can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { students: { student_id: user.id } })
98-
can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil)
99-
can(%i[read], Project, lesson: { school_id: school.id, school_class: { students: { student_id: user.id } } })
98+
can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil, remixed_from_id: Project.where(school_id: school.id, lesson_id: Lesson.where(visibility: 'students').select(:id)).pluck(:id))
99+
can(%i[read show_context], Project, lesson: { school_id: school.id, visibility: 'students', school_class: { students: { student_id: user.id } } })
100100
can(%i[show_finished set_finished], SchoolProject, project: { user_id: user.id, lesson_id: nil }, school_id: school.id)
101101
end
102102

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# frozen_string_literal: true
2+
3+
json.call(
4+
@project,
5+
:identifier,
6+
:school_id,
7+
:lesson_id
8+
)
9+
10+
json.class_id(@project.lesson.school_class_id)

config/routes.rb

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
resources :projects, only: %i[index show update destroy create] do
3636
get :finished, on: :member, to: 'school_projects#show_finished'
37+
get :context, on: :member, to: 'projects#show_context'
3738
put :finished, on: :member, to: 'school_projects#set_finished'
3839
resource :remix, only: %i[show create], controller: 'projects/remixes'
3940
resources :remixes, only: %i[index], controller: 'projects/remixes'

spec/features/project/creating_a_project_spec.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,12 @@
8080
expect(response).to have_http_status(:created)
8181
end
8282

83-
it 'responds 201 Created when the user is a school-student for the school' do
83+
it 'responds 403 Forbidden when the user is a school-student for the school' do
8484
student = create(:student, school:)
8585
authenticated_in_hydra_as(student)
8686

8787
post('/api/projects', headers:, params:)
88-
expect(response).to have_http_status(:created)
88+
expect(response).to have_http_status(:forbidden)
8989
end
9090

9191
it 'sets the lesson user to the specified user for school-owner users' do

spec/models/ability_spec.rb

+81-2
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@
140140
end
141141

142142
# rubocop:disable RSpec/MultipleMemoizedHelpers
143-
context 'with a teachers project where the lesson is visible to students' do
143+
context "with a teacher's project where the lesson is visible to students" do
144144
let(:user) { create(:user) }
145145
let(:school) { create(:school) }
146146
let(:teacher) { create(:teacher, school:) }
@@ -154,6 +154,7 @@
154154
end
155155

156156
it { is_expected.to be_able_to(:read, project) }
157+
it { is_expected.to be_able_to(:show_context, project) }
157158
it { is_expected.not_to be_able_to(:create, project) }
158159
it { is_expected.not_to be_able_to(:update, project) }
159160
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
@@ -166,6 +167,7 @@
166167
end
167168

168169
it { is_expected.to be_able_to(:read, project) }
170+
it { is_expected.to be_able_to(:show_context, project) }
169171
it { is_expected.not_to be_able_to(:create, project) }
170172
it { is_expected.to be_able_to(:update, project) }
171173
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
@@ -179,6 +181,7 @@
179181
end
180182

181183
it { is_expected.to be_able_to(:read, project) }
184+
it { is_expected.to be_able_to(:show_context, project) }
182185
it { is_expected.not_to be_able_to(:create, project) }
183186
it { is_expected.not_to be_able_to(:update, project) }
184187
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
@@ -191,6 +194,69 @@
191194
end
192195

193196
it { is_expected.not_to be_able_to(:read, project) }
197+
it { is_expected.not_to be_able_to(:show_context, project) }
198+
it { is_expected.not_to be_able_to(:create, project) }
199+
it { is_expected.not_to be_able_to(:update, project) }
200+
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
201+
it { is_expected.not_to be_able_to(:destroy, project) }
202+
end
203+
end
204+
205+
context 'with a teachers project where the lesson is not visible to students' do
206+
let(:user) { create(:user) }
207+
let(:school) { create(:school) }
208+
let(:teacher) { create(:teacher, school:) }
209+
let(:school_class) { build(:school_class, school:, teacher_ids: [teacher.id]) }
210+
let(:lesson) { build(:lesson, school:, school_class:, user_id: teacher.id, visibility: 'teachers') }
211+
let!(:project) { build(:project, school:, lesson:, user_id: teacher.id) }
212+
213+
context 'when user is a school owner' do
214+
before do
215+
create(:owner_role, user_id: user.id, school:)
216+
end
217+
218+
it { is_expected.to be_able_to(:read, project) }
219+
it { is_expected.to be_able_to(:show_context, project) }
220+
it { is_expected.not_to be_able_to(:create, project) }
221+
it { is_expected.not_to be_able_to(:update, project) }
222+
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
223+
it { is_expected.not_to be_able_to(:destroy, project) }
224+
end
225+
226+
context 'when user is a school teacher' do
227+
before do
228+
create(:teacher_role, user_id: user.id, school:)
229+
end
230+
231+
it { is_expected.to be_able_to(:read, project) }
232+
it { is_expected.to be_able_to(:show_context, project) }
233+
it { is_expected.not_to be_able_to(:create, project) }
234+
it { is_expected.to be_able_to(:update, project) }
235+
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
236+
it { is_expected.not_to be_able_to(:destroy, project) }
237+
end
238+
239+
context 'when user is a school student and belongs to the teachers class' do
240+
before do
241+
create(:student_role, user_id: user.id, school:)
242+
create(:class_student, school_class:, student_id: user.id)
243+
end
244+
245+
it { is_expected.not_to be_able_to(:read, project) }
246+
it { is_expected.not_to be_able_to(:show_context, project) }
247+
it { is_expected.not_to be_able_to(:create, project) }
248+
it { is_expected.not_to be_able_to(:update, project) }
249+
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
250+
it { is_expected.not_to be_able_to(:destroy, project) }
251+
end
252+
253+
context 'when user is a school student and does not belong to the teachers class' do
254+
before do
255+
create(:student_role, user_id: user.id, school:)
256+
end
257+
258+
it { is_expected.not_to be_able_to(:read, project) }
259+
it { is_expected.not_to be_able_to(:show_context, project) }
194260
it { is_expected.not_to be_able_to(:create, project) }
195261
it { is_expected.not_to be_able_to(:update, project) }
196262
it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
@@ -200,7 +266,7 @@
200266

201267
# TODO: Handle other visibilities
202268

203-
context 'with a remix of a teachers project' do
269+
context "with a remix of a teacher's project" do
204270
let(:school) { create(:school) }
205271
let(:student) { create(:student, school:) }
206272
let(:teacher) { create(:teacher, school:) }
@@ -221,6 +287,19 @@
221287
it { is_expected.to be_able_to(:set_finished, remixed_project.school_project) }
222288
end
223289

290+
context 'when user is a student and the lesson is not visible to students' do
291+
let(:user) { student }
292+
293+
before do
294+
lesson.update(visibility: 'teachers')
295+
end
296+
297+
it { is_expected.not_to be_able_to(:read, remixed_project) }
298+
it { is_expected.not_to be_able_to(:create, remixed_project) }
299+
it { is_expected.not_to be_able_to(:update, remixed_project) }
300+
it { is_expected.not_to be_able_to(:destroy, remixed_project) }
301+
end
302+
224303
context 'when user is teacher that does not own the orginal project' do
225304
let(:user) { create(:teacher, school:) }
226305

0 commit comments

Comments
 (0)