Skip to content

Commit

Permalink
Merge pull request #43756 from code-dot-org/revert-43737-jamescodeorg…
Browse files Browse the repository at this point in the history
…/disable-caching-apis

Revert "Prevent api responses that depend on current_user from being cached"
  • Loading branch information
jamescodeorg committed Nov 22, 2021
2 parents 4e0a9f3 + e5ec65d commit 3a9cf5b
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 38 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
class Api::V1::Projects::PersonalProjectsController < ApplicationController
# GET /api/v1/projects/personal/
def index
prevent_caching
return head :forbidden unless current_user
render json: ProjectsList.fetch_personal_projects(current_user.id)
rescue ArgumentError => e
Expand Down
1 change: 0 additions & 1 deletion dashboard/app/controllers/api/v1/sections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class Api::V1::SectionsController < Api::V1::JsonApiController
# GET /api/v1/sections
# Get the set of sections owned by the current user
def index
prevent_caching
render json: current_user.sections.map(&:summarize)
end

Expand Down
1 change: 0 additions & 1 deletion dashboard/app/controllers/api/v1/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def load_user

# GET /api/v1/users/current
def current
prevent_caching
if current_user
render json: {
id: current_user.id,
Expand Down
10 changes: 1 addition & 9 deletions dashboard/app/controllers/api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ def import_google_classroom
end

def user_menu
prevent_caching
show_pairing_dialog = !!session.delete(:show_pairing_dialog)
@user_header_options = {}
@user_header_options[:current_user] = current_user
Expand Down Expand Up @@ -208,7 +207,6 @@ def update_lockable_state

# For a given user, gets the lockable state for each student in each of their sections
def lockable_state
prevent_caching
unless current_user
render json: {}
return
Expand All @@ -234,7 +232,6 @@ def lockable_state
use_database_pool section_progress: :persistent

def section_progress
prevent_caching
section = load_section
script = load_script(section)

Expand Down Expand Up @@ -315,7 +312,6 @@ def section_progress
# If not specified, the API will default to a page size of 50, providing the first page
# of students
def section_level_progress
prevent_caching
section = load_section
script = load_script(section)

Expand Down Expand Up @@ -349,7 +345,6 @@ def section_level_progress
# GET /api/teacher_panel_progress/:section_id
# Get complete details of a particular section for the teacher panel progress
def teacher_panel_progress
prevent_caching
section = load_section
script = load_script(section)

Expand Down Expand Up @@ -388,7 +383,6 @@ def teacher_panel_progress

# Get /api/teacher_panel_section
def teacher_panel_section
prevent_caching
teacher_sections = current_user&.sections

if teacher_sections.blank?
Expand Down Expand Up @@ -430,7 +424,6 @@ def script_standards

# Return a JSON summary of the user's progress for params[:script].
def user_progress
prevent_caching
if current_user
if params[:user_id].present?
user = User.find(params[:user_id])
Expand All @@ -457,7 +450,6 @@ def user_progress
# Returns app_options values that are user-specific. This is used on cached
# levels.
def user_app_options
prevent_caching
response = {}

script = Script.get_from_cache(params[:script])
Expand Down Expand Up @@ -514,7 +506,7 @@ def user_app_options
# given user. This code is analogous to parts of LevelsHelper#app_options.
# TODO: Eliminate this logic from LevelsHelper#app_options or refactor methods
# to share code.
private def progress_app_options(script, level, user)
def progress_app_options(script, level, user)
response = {}

user_level = user.last_attempt(level, script)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class Api::V1::Projects::PersonalProjectsControllerTest < ActionDispatch::Integr
sign_in(@project_owner)
get "/api/v1/projects/personal/"
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]
personal_projects_list = JSON.parse(@response.body)
assert_equal 1, personal_projects_list.size
project_row = personal_projects_list.first
Expand Down
2 changes: 0 additions & 2 deletions dashboard/test/controllers/api/v1/users_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ class Api::V1::UsersControllerTest < ActionController::TestCase
test "a get request to get current returns signed out user info" do
get :current
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]
response = JSON.parse(@response.body)
assert_equal false, response["is_signed_in"]
end
Expand All @@ -178,7 +177,6 @@ class Api::V1::UsersControllerTest < ActionController::TestCase
sign_in(teacher)
get :current
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]
response = JSON.parse(@response.body)
assert_equal true, response["is_signed_in"]
assert_equal teacher.id, response["id"]
Expand Down
24 changes: 1 addition & 23 deletions dashboard/test/controllers/api_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,6 @@ class ApiControllerTest < ActionController::TestCase
script_id: script.id
}
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]
body = JSON.parse(response.body)

assert_equal [@section.id.to_s, @flappy_section.id.to_s, @allthings_section.id.to_s], body.keys, "entry for each section"
Expand Down Expand Up @@ -826,7 +825,6 @@ def get_student_response(script, level, lesson, student_number)

get :user_progress, params: {script: @script.name}
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]

body = JSON.parse(response.body)
assert_equal true, body['signedIn']
Expand Down Expand Up @@ -889,8 +887,6 @@ def get_student_response(script, level, lesson, student_number)
level: @level.id
}
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]

body = JSON.parse(response.body)
assert_equal true, body['signedIn']
assert_equal false, body['disableSocialShare']
Expand Down Expand Up @@ -1150,7 +1146,6 @@ def get_student_response(script, level, lesson, student_number)
get :section_progress, params: {section_id: @flappy_section.id}
end
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]

data = JSON.parse(@response.body)
expected = {
Expand Down Expand Up @@ -1240,12 +1235,6 @@ def get_student_response(script, level, lesson, student_number)
assert_equal 1, data['students'].length
end

test 'section_level_progress response should not be cached by the browser' do
get :section_level_progress, params: {section_id: @section.id, page: 1, per: 2}
assert_response :success
assert_match "no-store", response.headers["Cache-Control"]
end

test "should get paginated section level progress" do
get :section_level_progress, params: {section_id: @section.id, page: 1, per: 2}
assert_response :success
Expand Down Expand Up @@ -1386,7 +1375,6 @@ def get_student_response(script, level, lesson, student_number)
}

assert_response :success
assert_match "no-store", response.headers["Cache-Control"]

response = JSON.parse(@response.body)

Expand Down Expand Up @@ -1458,9 +1446,8 @@ def get_student_response(script, level, lesson, student_number)
}

assert_response :success
assert_match "no-store", response.headers["Cache-Control"]

response = JSON.parse(@response.body)

assert_equal @section.id, response["id"]
assert_equal @teacher.name, response["teacherName"]
assert_equal 7, response["students"].length
Expand Down Expand Up @@ -1543,15 +1530,6 @@ def get_student_response(script, level, lesson, student_number)
assert_equal expected_response, response
end

test 'user_menu response should not be cached by the browser' do
sign_in create(:student)

get :user_menu

assert_response :success
assert_match "no-store", response.headers["Cache-Control"]
end

test "user menu should open pairing dialog if asked to in the session" do
sign_in create(:student)

Expand Down

0 comments on commit 3a9cf5b

Please sign in to comment.