Skip to content

Commit 7d3433d

Browse files
authored
Merge branch 'main' into prevent-students-calling-profile-api-and-getting-500
2 parents 5aab468 + 003712f commit 7d3433d

10 files changed

Lines changed: 113 additions & 41 deletions

File tree

app/controllers/api/school_classes_controller.rb

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@ class SchoolClassesController < ApiController
77
before_action :load_and_authorize_school_class
88

99
def index
10-
school_classes = @school.classes.accessible_by(current_ability)
11-
school_classes = school_classes.joins(:teachers).where(teachers: { teacher_id: current_user.id }) if params[:my_classes] == 'true'
10+
school_classes = accessible_school_classes
11+
school_classes = school_classes.joins(:teachers).where(teachers: { teacher_id: current_user&.id }) if params[:my_classes] == 'true'
1212
@school_classes_with_teachers = school_classes.with_teachers
13-
render :index, formats: [:json], status: :ok
13+
14+
if current_user&.school_teacher?(@school) || current_user&.school_owner?(@school)
15+
render :teacher_index, formats: [:json], status: :ok
16+
else
17+
render :student_index, formats: [:json], status: :ok
18+
end
1419
end
1520

1621
def show
@@ -101,6 +106,14 @@ def find_or_create_school_class(school_class_params)
101106
)
102107
end
103108

109+
def accessible_school_classes
110+
if current_user&.school_teacher?(@school) || current_user&.school_owner?(@school)
111+
@school.classes.accessible_by(current_ability).includes(lessons: { project: { remixes: { school_project: :school_project_transitions } } })
112+
else
113+
@school.classes.accessible_by(current_ability)
114+
end
115+
end
116+
104117
def create_school_students(school_students_params, school_class)
105118
return { school_students: [], errors: nil } unless school_class.present? && school_students_params.present?
106119

app/models/school_class.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,21 @@ def assign_class_code
5858
errors.add(:code, 'could not be generated')
5959
end
6060

61+
def submitted_count
62+
return 0 if lessons.empty?
63+
64+
Lesson
65+
.joins(project: { remixes: { school_project: :school_project_transitions } })
66+
.where(school_class_id: id)
67+
.where(
68+
school_project_transitions: {
69+
to_state: 'submitted',
70+
most_recent: true
71+
}
72+
)
73+
.count
74+
end
75+
6176
private
6277

6378
def school_class_has_at_least_one_teacher
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# frozen_string_literal: true
2+
3+
json.call(
4+
school_class,
5+
:id,
6+
:description,
7+
:school_id,
8+
:name,
9+
:created_at,
10+
:updated_at,
11+
:import_origin,
12+
:import_id
13+
)
14+
15+
json.teachers(teachers) do |teacher|
16+
json.partial! '/api/school_teachers/school_teacher', teacher:, include_email: false
17+
end

app/views/api/school_classes/index.json.jbuilder

Lines changed: 0 additions & 19 deletions
This file was deleted.

app/views/api/school_classes/show.json.jbuilder

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,6 @@
22

33
school_class, teachers = @school_class_with_teachers
44

5-
json.call(
6-
school_class,
7-
:id,
8-
:description,
9-
:school_id,
10-
:name,
11-
:code,
12-
:created_at,
13-
:updated_at,
14-
:import_origin,
15-
:import_id
16-
)
5+
json.partial! 'school_class', school_class: school_class, teachers: teachers
176

18-
json.teachers(teachers) do |teacher|
19-
json.partial! '/api/school_teachers/school_teacher', teacher:, include_email: false
20-
end
7+
json.code school_class.code
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# frozen_string_literal: true
2+
3+
json.array!(@school_classes_with_teachers) do |school_class, teachers|
4+
json.partial! 'school_class', school_class: school_class, teachers: teachers
5+
end
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# frozen_string_literal: true
2+
3+
json.array!(@school_classes_with_teachers) do |school_class, teachers|
4+
json.partial! 'school_class', school_class: school_class, teachers: teachers
5+
6+
json.submitted_count(school_class.submitted_count)
7+
end
Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
# frozen_string_literal: true
22

3-
json.call(teacher, :id, :name)
4-
json.type('teacher')
3+
if teacher.present?
4+
json.call(teacher, :id, :name)
5+
json.type('teacher')
56

6-
include_email = local_assigns.fetch(:include_email, true)
7+
include_email = local_assigns.fetch(:include_email, true)
78

8-
json.email(teacher.email) if include_email
9+
json.email(teacher.email) if include_email
10+
end

spec/features/school_class/listing_school_classes_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,28 @@
5353
expect(data.first[:teachers].first).to be_nil
5454
end
5555

56+
it 'includes submitted_count if user is a school-teacher' do
57+
authenticated_in_hydra_as(teacher)
58+
get("/api/schools/#{school.id}/classes", headers:)
59+
data = JSON.parse(response.body, symbolize_names: true)
60+
expect(data.first).to have_key(:submitted_count)
61+
end
62+
63+
it 'includes submitted_count if user is a school-owner' do
64+
authenticated_in_hydra_as(owner)
65+
get("/api/schools/#{school.id}/classes", headers:)
66+
data = JSON.parse(response.body, symbolize_names: true)
67+
expect(data.first).to have_key(:submitted_count)
68+
end
69+
70+
it 'does not include submitted_count if user is a school-student' do
71+
authenticated_in_hydra_as(student)
72+
stub_user_info_api_for(teacher)
73+
get("/api/schools/#{school.id}/classes", headers:)
74+
data = JSON.parse(response.body, symbolize_names: true)
75+
expect(data.first).not_to have_key(:submitted_count)
76+
end
77+
5678
it "does not include school classes that the school-teacher doesn't teach" do
5779
teacher = create(:teacher, school:)
5880
authenticated_in_hydra_as(teacher)

spec/models/school_class_spec.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,29 @@
262262
end
263263
end
264264

265+
describe '#submitted_count' do
266+
it 'returns 0 if there are no lessons' do
267+
school_class = create(:school_class, teacher_ids: [teacher.id], school:)
268+
expect(school_class.submitted_count).to eq(0)
269+
end
270+
271+
it 'returns the sum of submitted counts from all lessons' do
272+
school_class = create(:school_class, teacher_ids: [teacher.id], school:)
273+
274+
lesson_1 = create(:lesson, school_class:, user_id: teacher.id)
275+
remix_1 = create(:project, school:, remixed_from_id: lesson_1.project.id, user_id: student.id)
276+
remix_1.school_project.transition_status_to!(:submitted, remix_1.user_id)
277+
278+
lesson_2 = create(:lesson, school_class:, user_id: teacher.id)
279+
remix_2 = create(:project, school:, remixed_from_id: lesson_2.project.id, user_id: student.id)
280+
remix_2.school_project.transition_status_to!(:submitted, remix_2.user_id)
281+
remix_3 = create(:project, school:, remixed_from_id: lesson_2.project.id, user_id: student.id)
282+
remix_3.school_project.transition_status_to!(:submitted, remix_3.user_id)
283+
284+
expect(school_class.submitted_count).to eq(3)
285+
end
286+
end
287+
265288
describe 'auditing' do
266289
subject(:school_class) { create(:school_class, teacher_ids: [teacher.id], school:) }
267290

0 commit comments

Comments
 (0)