convert data values using to_json #23
Conversation
misaka
force-pushed
the
convert-moar-json
branch
from
def9d5a to
4a29d62
Compare
dfe-analytics.gemspec
Outdated
| @@ -36,6 +36,7 @@ Gem::Specification.new do |spec| | |||
| spec.add_development_dependency 'rubocop-rspec', '~> 2' | |||
| spec.add_development_dependency 'solargraph' | |||
| spec.add_development_dependency 'sqlite3' | |||
| spec.add_development_dependency 'store_model' | |||
There was a problem hiding this comment.
Do we have to have this whole thing? Isn't the point the interface — ie that SomeObject implements to_json?
There was a problem hiding this comment.
eg
ObjectWithToJson = Struct.new(:colour) do
def to_json
JSON.generate({colour: colour})
end
endor something?
There was a problem hiding this comment.
It's not quite to_json, I changed the implementation to check for as_json. We could hand-roll an as_json but I thought it would be better test to just use pre-built library. Happy to discuss.
There was a problem hiding this comment.
ObjectWithAsJson = Struct.new(:colour) do
def as_json
{colour: colour}
end
end?
There was a problem hiding this comment.
^ imo this says "your object should support as_json", using the gem says "we support this particular gem", and I think I prefer the former?
There was a problem hiding this comment.
Considering the spec is failing in CI because of this ... maybe hand-rolling will actually end up just being easier. sigh
There was a problem hiding this comment.
I've adopted the Struct approach, thanks @duncanjbrown !
|
One notable issue that this brings out is that if we send through an object with PII in it, it will be happily serialised and sent through to BQ. After we merge this PR, we should consider whether we need to add a layer of protection to ensure entities included as part of other entities only include the permitted fields. |
|
One result of this change is that in the data sent by Register, dates will no longer have extra double-quotes around them, something originally introduced by an earlier attempt to get around the same problem this PR addresses. I don't think this will be an issue as the Dataform code optionally deals with extra double-quotes, but it's worth double-checking this. |
misaka
force-pushed
the
convert-moar-json
branch
from
4a29d62 to
a27132a
Compare
When converting event data into the JSON payload to send to BQ, have special handling for Hash objects. However, some data will come in as on object that isn't immediately convertable to something JSON-friendly, such as models (e.g. StoreModel or ActiveModel objects). This change converts everything to a more standard JSON-friendly representation before processing for sending to BQ, meaning that models are converted to a Hash object which we can then to_json as needed.
misaka
force-pushed
the
convert-moar-json
branch
from
a27132a to
3b7b8e1
Compare
When converting event data into the JSON payload to send to BQ, we used
to selectively only convert Hash values to JSON with "to_json". However,
some data values passed through for sending to BigQery may not present as a
Hash, e.g. trainees.progress in Register, causing inseration errors in
BQ.
This change makes us more lenient and will try to convert any value to
JSON if it implements the "to_json" method.