Skip to content

Commit 25976d1

Browse files
authored
Namespace serializers for consuming controller (#538)
This change moves all serializers into the same namespace (`Api::V1`) as the controllers which consume those serializers. In doing so it fixes a bug which currently breaks almost any operation in staging, most notably user signup, by falling back to `Object#as_json` to serialize all API responses, not complying at all with our API spec. `active_model_serializers` [looks for the serializers in this namespace by default](https://github.com/rails-api/active_model_serializers/blob/v0.9.8/lib/action_controller/serialization.rb#L62-L80), but this worked on Ruby 2.6.5: ``` [1] pry(#<Api::V1::RegistrationsController>)> r = Registration.create!(params)=> #<Registration:0x00007f96688f4f20 @birth_date=nil, @captcha_response= "dummy-captcha-response", @errors= #<ActiveModel::Errors:0x00007f96688f4188 @base= #<Registration:0x00007f96688f4f20 ...>, @errors=[]>, @screen_name="user", @user= #<User id: 1, email: "[email protected]", authentication_token: "03441d861500b01606fa928521b971eb", created_at: "2021-10-14 21:14:00.673552000 +0000", updated_at: "2021-10-14 21:14:00.673552000 +0000">, @user_params= {"email"=>"[email protected]", "password"=>"secret123", "password_confirmation"=>"secret123"}> [2] pry(#<Api::V1::RegistrationsController>)> default_serializer(r)=> RegistrationSerializer ``` Since Ruby 2.6.6, [a change to constant lookup](ruby/ruby@15eba4e) was introduced which means that the appropriate class can't be found: ``` [2] pry(#<Api::V1::RegistrationsController>)> default_serializer(r) => nil [3] pry(#<Api::V1::RegistrationsController>)> ActiveModel::Serializer.serializer_for(r, namespace: namespace_for_serializer) => nil ``` The reason is that while in both Ruby versions, the constant name that we try to look up is namespaced ([This namespacing behaviour is documented](https://github.com/rails-api/active_model_serializers/blob/v0.10.6/docs/general/rendering.md#namespace) in later releases, but the behaviour is similar in v0.9.x.): ``` [4] pry(#<Api::V1::RegistrationsController>)> ActiveModel::Serializer.send(:build_serializer_class, r, namespace: namespace_for_serializer) => "Api::V1::RegistrationSerializer" ``` this name [is passed to Object#const_get](https://github.com/rails-api/active_model_serializers/blob/ff1a36ffbc82c6070905707b29978a7b897dea81/lib/active_model/serializable/utils.rb#L6) which behaves more strictly since Ruby 2.6.6: Ruby 2.6.5: ``` [5] pry(#<Api::V1::RegistrationsController>)> Object.const_get "Api::V1::RegistrationSerializer" => RegistrationSerializer ``` Ruby 2.6.6: ``` [5] pry(#<Api::V1::RegistrationsController>)> Object.const_get "Api::V1::RegistrationSerializer" => nil ``` It is considered appropriate to namespace the serializers together with the consuming controllers because together they constitute a cohesive definition of an API version; should an API V2 be introduced, it should likely have distinct serializers.
1 parent 8d3b9dd commit 25976d1

File tree

118 files changed

+1680
-1356
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

118 files changed

+1680
-1356
lines changed
Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,28 @@
1-
class Api::V1::AwsSesController < ApplicationController
2-
skip_authorize_resource only: [:mail_it, :notification]
3-
skip_before_action :authenticate_user!, only: [:mail_it, :notification]
1+
module Api
2+
module V1
3+
class AwsSesController < ApplicationController
4+
skip_authorize_resource only: [:mail_it, :notification]
5+
skip_before_action :authenticate_user!, only: [:mail_it, :notification]
46

5-
def notification
6-
message_type = request.headers["x-amz-sns-message-type"]
7-
# sns_topic = request.headers['x-amz-sns-topic-arn']
8-
raw_post = request.raw_post
7+
def notification
8+
message_type = request.headers["x-amz-sns-message-type"]
9+
# sns_topic = request.headers['x-amz-sns-topic-arn']
10+
raw_post = request.raw_post
911

10-
if message_type.include? "Confirmation"
11-
send_subscription_confirmation(raw_post)
12-
elsif message_type.include? "Notification"
13-
EmailRejectDispatcher.perform_async(raw_post)
14-
end
12+
if message_type.include? "Confirmation"
13+
send_subscription_confirmation(raw_post)
14+
elsif message_type.include? "Notification"
15+
EmailRejectDispatcher.perform_async(raw_post)
16+
end
1517

16-
render nothing: true, status: 200
17-
end
18+
render nothing: true, status: 200
19+
end
1820

19-
def send_subscription_confirmation(raw_post)
20-
json = JSON.parse(raw_post)
21+
def send_subscription_confirmation(raw_post)
22+
json = JSON.parse(raw_post)
2123

22-
open(json["SubscribeURL"])
24+
open(json["SubscribeURL"])
25+
end
26+
end
2327
end
2428
end
Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
class Api::V1::ChartListsController < ApplicationController
2-
def show
3-
render json: ChartListService.new(current_user: current_user).as_json
1+
module Api
2+
module V1
3+
class ChartListsController < ApplicationController
4+
def show
5+
render json: ChartListService.new(current_user: current_user).as_json
6+
end
7+
end
48
end
59
end
Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
1-
class Api::V1::ChartsController < ApplicationController
2-
def show
3-
chart = Chart.new(chart_params)
1+
module Api
2+
module V1
3+
class ChartsController < ApplicationController
4+
def show
5+
chart = Chart.new(chart_params)
46

5-
# FIXME
6-
# rubocop:disable Style/SignalException
7-
fail(ActiveRecord::RecordInvalid, chart) if chart.invalid?
8-
# rubocop:enable Style/SignalException
7+
# FIXME
8+
# rubocop:disable Style/SignalException
9+
fail(ActiveRecord::RecordInvalid, chart) if chart.invalid?
10+
# rubocop:enable Style/SignalException
911

10-
render json: chart
11-
end
12+
render json: chart
13+
end
1214

13-
def chart_params
14-
includes_params = {
15-
tags: [],
16-
foods: [],
17-
symptoms: [],
18-
conditions: [],
19-
treatments: [],
20-
weathersMeasures: [],
21-
harveyBradshawIndices: []
22-
}
15+
def chart_params
16+
includes_params = {
17+
tags: [],
18+
foods: [],
19+
symptoms: [],
20+
conditions: [],
21+
treatments: [],
22+
weathersMeasures: [],
23+
harveyBradshawIndices: []
24+
}
2325

24-
params.permit(:id, :start_at, :end_at, includes: includes_params).tap do |whitelist|
25-
whitelist[:user] = current_user
26+
params.permit(:id, :start_at, :end_at, includes: includes_params).tap do |whitelist|
27+
whitelist[:user] = current_user
28+
end
29+
end
2630
end
2731
end
2832
end
Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,31 @@
1-
class Api::V1::ChartsPatternController < ApplicationController
2-
skip_before_action :authenticate_user!, only: [:index]
1+
module Api
2+
module V1
3+
class ChartsPatternController < ApplicationController
4+
skip_before_action :authenticate_user!, only: [:index]
35

4-
def index
5-
offset = charts_pattern_params[:offset].to_i
6-
start_at = (charts_pattern_params[:start_at].to_date - offset.days).to_s
6+
def index
7+
offset = charts_pattern_params[:offset].to_i
8+
start_at = (charts_pattern_params[:start_at].to_date - offset.days).to_s
79

8-
end_date = charts_pattern_params[:end_at].to_date
9-
end_at = (Time.current.to_date == end_date ? end_date : (end_date + offset.days)).to_s
10+
end_date = charts_pattern_params[:end_at].to_date
11+
end_at = (Time.current.to_date == end_date ? end_date : (end_date + offset.days)).to_s
1012

11-
@patterns = Pattern.where(id: {'$in': charts_pattern_params[:pattern_ids] || []})
13+
@patterns = Pattern.where(id: {'$in': charts_pattern_params[:pattern_ids] || []})
1214

13-
@extended_patterns = @patterns.map do |pattern|
14-
pattern.extend(PatternExtender).form_chart_data(start_at: start_at,
15-
end_at: end_at,
16-
pattern: pattern)
17-
end
15+
@extended_patterns = @patterns.map do |pattern|
16+
pattern.extend(PatternExtender).form_chart_data(start_at: start_at,
17+
end_at: end_at,
18+
pattern: pattern)
19+
end
1820

19-
render json: @extended_patterns, meta: {color_ids: Flaredown::Colorable::IDS}
20-
end
21+
render json: @extended_patterns, meta: {color_ids: Flaredown::Colorable::IDS}
22+
end
2123

22-
private
24+
private
2325

24-
def charts_pattern_params
25-
params.permit(:start_at, :end_at, :offset, pattern_ids: [])
26+
def charts_pattern_params
27+
params.permit(:start_at, :end_at, :offset, pattern_ids: [])
28+
end
29+
end
2630
end
2731
end
Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,35 @@
1-
class Api::V1::CheckinsController < ApplicationController
2-
def index
3-
date = params[:date]
1+
module Api
2+
module V1
3+
class CheckinsController < ApplicationController
4+
def index
5+
date = params[:date]
46

5-
if date.blank? && params.require(:page)
6-
render json: current_user.checkins.where(:note.nin => [nil, ""]).order_by(date: :desc).page(params[:page]).per(10)
7-
else
8-
render json: current_user.checkins.where(date: Date.parse(date))
9-
end
10-
end
7+
if date.blank? && params.require(:page)
8+
render json: current_user.checkins.where(:note.nin => [nil, ""]).order_by(date: :desc).page(params[:page]).per(10)
9+
else
10+
render json: current_user.checkins.where(date: Date.parse(date))
11+
end
12+
end
1113

12-
def show
13-
render json: Checkin.find(id)
14-
end
14+
def show
15+
render json: Checkin.find(id)
16+
end
1517

16-
def create
17-
date = params.require(:checkin).require(:date)
18-
checkin = Checkin::Creator.new(current_user.id, Date.parse(date)).create!
19-
render json: checkin
20-
end
18+
def create
19+
date = params.require(:checkin).require(:date)
20+
checkin = Checkin::Creator.new(current_user.id, Date.parse(date)).create!
21+
render json: checkin
22+
end
2123

22-
def update
23-
render json: Checkin::Updater.new(current_user, params).update!
24-
end
24+
def update
25+
render json: Checkin::Updater.new(current_user, params).update!
26+
end
2527

26-
private
28+
private
2729

28-
def id
29-
params.require(:id)
30+
def id
31+
params.require(:id)
32+
end
33+
end
3034
end
3135
end
Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,45 @@
1-
class Api::V1::CommentsController < ApplicationController
2-
load_and_authorize_resource
3-
skip_before_action :authenticate_user!, only: [:index]
1+
module Api
2+
module V1
3+
class CommentsController < ApplicationController
4+
load_and_authorize_resource
5+
skip_before_action :authenticate_user!, only: [:index]
6+
7+
def index
8+
render json: @comments.where(:id.in => params[:ids]).order_by(created_at: :asc)
9+
end
410

5-
def index
6-
render json: @comments.where(:id.in => params[:ids]).order_by(created_at: :asc)
7-
end
11+
def show
12+
render json: @comment
13+
end
814

9-
def show
10-
render json: @comment
11-
end
15+
def create
16+
@comment.encrypted_user_id = current_user.encrypted_id
17+
18+
if @comment.save
19+
UpdatePostCountersJob.perform_async(parent_id: create_params[:post_id], parent_type: "Post")
1220

13-
def create
14-
@comment.encrypted_user_id = current_user.encrypted_id
21+
unless @comment.encrypted_user_id == @comment.post.encrypted_user_id
22+
Notification.create(
23+
kind: :comment,
24+
notificateable: @comment,
25+
encrypted_user_id: @comment.encrypted_user_id,
26+
encrypted_notify_user_id: @comment.post.encrypted_user_id
27+
)
28+
end
1529

16-
if @comment.save
17-
UpdatePostCountersJob.perform_async(parent_id: create_params[:post_id], parent_type: "Post")
30+
DiscussionMention.perform_async(current_user.encrypted_id, @comment.id.to_s)
1831

19-
unless @comment.encrypted_user_id == @comment.post.encrypted_user_id
20-
Notification.create(
21-
kind: :comment,
22-
notificateable: @comment,
23-
encrypted_user_id: @comment.encrypted_user_id,
24-
encrypted_notify_user_id: @comment.post.encrypted_user_id
25-
)
32+
render json: @comment, status: :created
33+
else
34+
render json: {errors: @comment.errors}, status: :unprocessable_entity
35+
end
2636
end
2737

28-
DiscussionMention.perform_async(current_user.encrypted_id, @comment.id.to_s)
38+
private
2939

30-
render json: @comment, status: :created
31-
else
32-
render json: {errors: @comment.errors}, status: :unprocessable_entity
40+
def create_params
41+
params.require(:comment).permit(:body, :post_id)
42+
end
3343
end
3444
end
35-
36-
private
37-
38-
def create_params
39-
params.require(:comment).permit(:body, :post_id)
40-
end
4145
end
Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,33 @@
1-
class Api::V1::ConditionsController < ApplicationController
2-
load_and_authorize_resource
3-
skip_before_action :authenticate_user!, only: [:show]
1+
module Api
2+
module V1
3+
class ConditionsController < ApplicationController
4+
load_and_authorize_resource
5+
skip_before_action :authenticate_user!, only: [:show]
46

5-
def index
6-
@conditions = @conditions.includes(:translations)
7-
@conditions = ids.present? ? @conditions.where(id: ids) : @conditions.order(:name).limit(50)
7+
def index
8+
@conditions = @conditions.includes(:translations)
9+
@conditions = ids.present? ? @conditions.where(id: ids) : @conditions.order(:name).limit(50)
810

9-
render json: @conditions
10-
end
11+
render json: @conditions
12+
end
1113

12-
def show
13-
render json: @condition
14-
end
14+
def show
15+
render json: @condition
16+
end
1517

16-
def create
17-
render json: TrackableCreator.new(@condition, current_user).create!
18-
end
18+
def create
19+
render json: TrackableCreator.new(@condition, current_user).create!
20+
end
1921

20-
private
22+
private
2123

22-
def create_params
23-
params.require(:condition).permit(:name)
24-
end
24+
def create_params
25+
params.require(:condition).permit(:name)
26+
end
2527

26-
def ids
27-
@ids ||= params[:ids] if params[:ids].is_a?(Array)
28+
def ids
29+
@ids ||= params[:ids] if params[:ids].is_a?(Array)
30+
end
31+
end
2832
end
2933
end

0 commit comments

Comments
 (0)