Skip to content

Commit c4f4aa0

Browse files
author
Shayon Mukherjee
committed
Standardize comparison of attribute as string when input is Hash or JSON
Adding `.to_s` properly follows the convention of not initializing `custom_attributes` or any other attribute which passes the reserve field `type`. Not having `.to_s`, meant if ApiResource is initialized by `User` or any other class, with a `Hash`, and the attribute contains the reserved `type` `Hash` attribute, it would incorrectly try to initialize the same property. Ex of how it would fail, if `User` class is initialized with custom_attributes that contains `type`: ```ruby Intercom::User.new(email: "[email protected]", custom_attributes: {"type"=>"ping"}) => Intercom::AttributeNotSetError: 'each' called on Intercom::Ping but it has not been set an attribute or does not exist as a method ... ``` A workaround would be that all keys were passed as strings, however this PR attempts at standardizing the comparison of any `attribute` always as string, without being dependent on input type (`Hash` or `JSON`). I took additional liberty and applied these changes for `type_field?` and `typed_property?` check for `metadata` attribute as well. `metadata`, because as per the specs, it can contain the reserved `type` `Hash` as well. Happy to tweak/modify/close, if the changes raise any concern :).
1 parent d96a580 commit c4f4aa0

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

lib/intercom/traits/api_resource.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,13 @@ def set_property(attribute, value)
7878
end
7979

8080
def custom_attribute_field?(attribute)
81-
attribute == 'custom_attributes'
81+
attribute.to_s == 'custom_attributes'
8282
end
83-
83+
8484
def message_from_field?(attribute, value)
8585
attribute.to_s == 'from' && value.is_a?(Hash) && value['type']
8686
end
87-
87+
8888
def message_to_field?(attribute, value)
8989
attribute.to_s == 'to' && value.is_a?(Hash) && value['type']
9090
end
@@ -94,7 +94,7 @@ def typed_property?(attribute, value)
9494
!custom_attribute_field?(attribute) &&
9595
!message_from_field?(attribute, value) &&
9696
!message_to_field?(attribute, value) &&
97-
attribute != 'metadata'
97+
attribute.to_s != 'metadata'
9898
end
9999

100100
def typed_value?(value)
@@ -107,7 +107,7 @@ def call_setter_for_attribute(attribute, value)
107107
end
108108

109109
def type_field?(attribute)
110-
attribute == 'type'
110+
attribute.to_s == 'type'
111111
end
112112

113113
def initialize_missing_flat_store_attributes

spec/unit/intercom/traits/api_resource_spec.rb

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,26 @@
1717
"color"=>"cyan"
1818
}}
1919
end
20+
21+
let(:object_hash) do
22+
{
23+
type: "company",
24+
id: "aaaaaaaaaaaaaaaaaaaaaaaa",
25+
app_id: "some-app-id",
26+
name: "SuperSuite",
27+
plan_id: 1,
28+
remote_company_id: "8",
29+
remote_created_at: 103201,
30+
created_at: 1374056196,
31+
user_count: 1,
32+
custom_attributes: { type: "ping" },
33+
metadata: {
34+
type: "user",
35+
color: "cyan"
36+
}
37+
}
38+
end
39+
2040
let(:api_resource) { DummyClass.new.extend(Intercom::Traits::ApiResource)}
2141

2242
before(:each) { api_resource.from_response(object_json) }
@@ -76,18 +96,29 @@
7696
proc { api_resource.send(:flubber=, 'a', 'b') }.must_raise NoMethodError
7797
end
7898

79-
it "an initialized ApiResource is equal to on generated from a response" do
99+
it "an initialized ApiResource is equal to one generated from a response" do
80100
class ConcreteApiResource; include Intercom::Traits::ApiResource; end
81101
initialized_api_resource = ConcreteApiResource.new(object_json)
82102
except(object_json, 'type').keys.each do |attribute|
83103
assert_equal initialized_api_resource.send(attribute), api_resource.send(attribute)
84104
end
85105
end
86-
106+
107+
it "initialized ApiResource using hash is equal to one generated from response" do
108+
class ConcreteApiResource; include Intercom::Traits::ApiResource; end
109+
110+
api_resource.from_hash(object_hash)
111+
initialized_api_resource = ConcreteApiResource.new(object_hash)
112+
113+
except(object_json, 'type').keys.each do |attribute|
114+
assert_equal initialized_api_resource.send(attribute), api_resource.send(attribute)
115+
end
116+
end
117+
87118
def except(h, *keys)
88119
keys.each { |key| h.delete(key) }
89120
h
90121
end
91-
122+
92123
class DummyClass; end
93124
end

0 commit comments

Comments
 (0)