Skip to content

Commit e9d5529

Browse files
create-issue-branch[bot]danhalson
create-issue-branch[bot]
and
danhalson
authored
Students unable to update their own projects (#447)
closes #446 --------- Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com> Co-authored-by: Dan Halson <[email protected]>
1 parent 494b434 commit e9d5529

File tree

3 files changed

+114
-12
lines changed

3 files changed

+114
-12
lines changed

‎app/models/ability.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ def define_school_owner_abilities(school:)
6868
can(%i[read create create_batch update destroy], :school_student)
6969
can(%i[create create_copy], Lesson, school_id: school.id)
7070
can(%i[read update destroy], Lesson, school_id: school.id, visibility: %w[teachers students public])
71-
can(%i[create], Project, school_id: school.id)
7271
end
7372

7473
def define_school_teacher_abilities(user:, school:)
@@ -88,7 +87,7 @@ def define_school_teacher_abilities(user:, school:)
8887
can(%i[create], Project) do |project|
8988
school_teacher_can_manage_project?(user:, school:, project:)
9089
end
91-
can(%i[read], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
90+
can(%i[read update], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
9291
can(%i[read], Project,
9392
remixed_from_id: Project.where(user_id: user.id, school_id: school.id, remixed_from_id: nil).pluck(:id))
9493
end
@@ -98,7 +97,7 @@ def define_school_student_abilities(user:, school:)
9897
can(%i[read], SchoolClass, school: { id: school.id }, members: { student_id: user.id })
9998
# Ensure no access to ClassMember resources, relationships otherwise allow access in some circumstances.
10099
can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { members: { student_id: user.id } })
101-
can(%i[create], Project, school_id: school.id, user_id: user.id, lesson_id: nil)
100+
can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil)
102101
can(%i[read], Project, lesson: { school_id: school.id, school_class: { members: { student_id: user.id } } })
103102
can(%i[toggle_finished], Project) do |project|
104103
school_student_can_toggle_finished?(user:, school:, project:)

‎spec/features/project/creating_a_project_spec.rb

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
RSpec.describe 'Creating a project', type: :request do
66
before do
7-
authenticated_in_hydra_as(owner)
7+
authenticated_in_hydra_as(teacher)
88
mock_phrase_generation
99
end
1010

@@ -118,6 +118,7 @@
118118
context 'when the project is associated with a lesson' do
119119
let(:school) { create(:school) }
120120
let(:lesson) { create(:lesson, school:, user_id: teacher.id) }
121+
let(:lesson_created_by_owner) { create(:lesson, school:, user_id: owner.id) }
121122
let(:teacher) { create(:teacher, school:) }
122123

123124
let(:params) do
@@ -147,7 +148,16 @@
147148

148149
it 'responds 422 Unprocessable when when the user_id is not the owner of the lesson' do
149150
user_id = SecureRandom.uuid
150-
new_params = { project: params[:project].merge(user_id:) }
151+
project = {
152+
project: {
153+
name: 'Test Project',
154+
components: [],
155+
school_id: school.id,
156+
lesson_id: lesson_created_by_owner.id,
157+
user_id: teacher.id
158+
}
159+
}
160+
new_params = { project: project.merge(user_id:) }
151161

152162
post('/api/projects', headers:, params: new_params)
153163
expect(response).to have_http_status(:unprocessable_entity)

‎spec/models/ability_spec.rb

Lines changed: 100 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
let(:starter_project) { build(:project, user_id: nil) }
1212

1313
describe 'Project' do
14-
context 'when no user' do
14+
context 'with no user' do
1515
let(:user) { nil }
1616

1717
context 'with a starter project' do
@@ -31,7 +31,7 @@
3131
end
3232
end
3333

34-
context 'when user present' do
34+
context 'with a standard user' do
3535
let(:user) { build(:user, id: user_id) }
3636
let(:another_project) { build(:project) }
3737

@@ -58,8 +58,89 @@
5858
end
5959
end
6060

61+
context 'with a teacher' do
62+
let(:user) { build(:teacher, id: user_id) }
63+
let(:another_project) { build(:project) }
64+
65+
context 'with a starter project' do
66+
it { is_expected.not_to be_able_to(:index, starter_project) }
67+
it { is_expected.to be_able_to(:show, starter_project) }
68+
it { is_expected.not_to be_able_to(:create, starter_project) }
69+
it { is_expected.not_to be_able_to(:update, starter_project) }
70+
it { is_expected.not_to be_able_to(:destroy, starter_project) }
71+
end
72+
73+
context 'with own project' do
74+
it { is_expected.to be_able_to(:read, project) }
75+
it { is_expected.to be_able_to(:create, project) }
76+
it { is_expected.to be_able_to(:update, project) }
77+
it { is_expected.to be_able_to(:destroy, project) }
78+
end
79+
80+
context 'with another user\'s project' do
81+
it { is_expected.not_to be_able_to(:read, another_project) }
82+
it { is_expected.not_to be_able_to(:create, another_project) }
83+
it { is_expected.not_to be_able_to(:update, another_project) }
84+
it { is_expected.not_to be_able_to(:destroy, another_project) }
85+
end
86+
end
87+
88+
context 'with an owner' do
89+
let(:user) { build(:owner, id: user_id) }
90+
let(:another_project) { build(:project) }
91+
92+
context 'with a starter project' do
93+
it { is_expected.not_to be_able_to(:index, starter_project) }
94+
it { is_expected.to be_able_to(:show, starter_project) }
95+
it { is_expected.not_to be_able_to(:create, starter_project) }
96+
it { is_expected.not_to be_able_to(:update, starter_project) }
97+
it { is_expected.not_to be_able_to(:destroy, starter_project) }
98+
end
99+
100+
context 'with own project' do
101+
it { is_expected.to be_able_to(:read, project) }
102+
it { is_expected.to be_able_to(:create, project) }
103+
it { is_expected.to be_able_to(:update, project) }
104+
it { is_expected.to be_able_to(:destroy, project) }
105+
end
106+
107+
context 'with another user\'s project' do
108+
it { is_expected.not_to be_able_to(:read, another_project) }
109+
it { is_expected.not_to be_able_to(:create, another_project) }
110+
it { is_expected.not_to be_able_to(:update, another_project) }
111+
it { is_expected.not_to be_able_to(:destroy, another_project) }
112+
end
113+
end
114+
115+
context 'with a student' do
116+
let(:user) { build(:student, id: user_id) }
117+
let(:another_project) { build(:project) }
118+
119+
context 'with a starter project' do
120+
it { is_expected.not_to be_able_to(:index, starter_project) }
121+
it { is_expected.to be_able_to(:show, starter_project) }
122+
it { is_expected.not_to be_able_to(:create, starter_project) }
123+
it { is_expected.not_to be_able_to(:update, starter_project) }
124+
it { is_expected.not_to be_able_to(:destroy, starter_project) }
125+
end
126+
127+
context 'with own project' do
128+
it { is_expected.to be_able_to(:read, project) }
129+
it { is_expected.to be_able_to(:create, project) }
130+
it { is_expected.to be_able_to(:update, project) }
131+
it { is_expected.to be_able_to(:destroy, project) }
132+
end
133+
134+
context 'with another user\'s project' do
135+
it { is_expected.not_to be_able_to(:read, another_project) }
136+
it { is_expected.not_to be_able_to(:create, another_project) }
137+
it { is_expected.not_to be_able_to(:update, another_project) }
138+
it { is_expected.not_to be_able_to(:destroy, another_project) }
139+
end
140+
end
141+
61142
# rubocop:disable RSpec/MultipleMemoizedHelpers
62-
context 'when the project belongs to a school and the associated lesson is not private' do
143+
context 'with a teachers project where the lesson is visible to students' do
63144
let(:user) { create(:user) }
64145
let(:school) { create(:school) }
65146
let(:teacher) { create(:teacher, school:) }
@@ -73,6 +154,7 @@
73154
end
74155

75156
it { is_expected.to be_able_to(:read, school_project) }
157+
it { is_expected.not_to be_able_to(:create, school_project) }
76158
it { is_expected.not_to be_able_to(:update, school_project) }
77159
it { is_expected.not_to be_able_to(:toggle_finished, school_project) }
78160
it { is_expected.not_to be_able_to(:destroy, school_project) }
@@ -84,36 +166,41 @@
84166
end
85167

86168
it { is_expected.to be_able_to(:read, school_project) }
87-
it { is_expected.not_to be_able_to(:update, school_project) }
169+
it { is_expected.not_to be_able_to(:create, school_project) }
170+
it { is_expected.to be_able_to(:update, school_project) }
88171
it { is_expected.not_to be_able_to(:toggle_finished, school_project) }
89172
it { is_expected.not_to be_able_to(:destroy, school_project) }
90173
end
91174

92-
context 'when user is a school student and belongs to a class' do
175+
context 'when user is a school student and belongs to the teachers class' do
93176
before do
94177
create(:student_role, user_id: user.id, school:)
95178
create(:class_member, school_class:, student_id: user.id)
96179
end
97180

98181
it { is_expected.to be_able_to(:read, school_project) }
182+
it { is_expected.not_to be_able_to(:create, school_project) }
99183
it { is_expected.not_to be_able_to(:update, school_project) }
100184
it { is_expected.not_to be_able_to(:toggle_finished, school_project) }
101185
it { is_expected.not_to be_able_to(:destroy, school_project) }
102186
end
103187

104-
context 'when user is a school student and does not belong to a class' do
188+
context 'when user is a school student and does not belong to the teachers class' do
105189
before do
106190
create(:student_role, user_id: user.id, school:)
107191
end
108192

109193
it { is_expected.not_to be_able_to(:read, school_project) }
194+
it { is_expected.not_to be_able_to(:create, school_project) }
110195
it { is_expected.not_to be_able_to(:update, school_project) }
111196
it { is_expected.not_to be_able_to(:toggle_finished, school_project) }
112197
it { is_expected.not_to be_able_to(:destroy, school_project) }
113198
end
114199
end
115200

116-
context 'when the project belongs to a student' do
201+
# TODO: Handle other visibilities
202+
203+
context 'with a remix of a teachers project' do
117204
let(:school) { create(:school) }
118205
let(:student) { create(:student, school:) }
119206
let(:teacher) { create(:teacher, school:) }
@@ -126,13 +213,18 @@
126213
context 'when user is the student' do
127214
let(:user) { student }
128215

216+
it { is_expected.to be_able_to(:read, remixed_project) }
217+
it { is_expected.to be_able_to(:create, remixed_project) }
218+
it { is_expected.to be_able_to(:update, remixed_project) }
219+
it { is_expected.not_to be_able_to(:destroy, remixed_project) }
129220
it { is_expected.to be_able_to(:toggle_finished, remixed_project) }
130221
end
131222

132223
context 'when user is teacher that does not own the orginal project' do
133224
let(:user) { create(:teacher, school:) }
134225

135226
it { is_expected.not_to be_able_to(:read, remixed_project) }
227+
it { is_expected.not_to be_able_to(:create, remixed_project) }
136228
it { is_expected.not_to be_able_to(:update, remixed_project) }
137229
it { is_expected.not_to be_able_to(:destroy, remixed_project) }
138230
it { is_expected.not_to be_able_to(:toggle_finished, remixed_project) }
@@ -142,6 +234,7 @@
142234
let(:user) { teacher }
143235

144236
it { is_expected.to be_able_to(:read, remixed_project) }
237+
it { is_expected.not_to be_able_to(:create, remixed_project) }
145238
it { is_expected.not_to be_able_to(:update, remixed_project) }
146239
it { is_expected.not_to be_able_to(:destroy, remixed_project) }
147240
it { is_expected.not_to be_able_to(:toggle_finished, remixed_project) }

0 commit comments

Comments
 (0)