stripe-ruby/.rubocop_todo.yml
Brandur 256556efa0 Fix replacement of non-metadata embedded StripeObjects
So we have a bit of a problem right now when it comes to replacing a
`StripeObject` that's embedded in an API resource.

Most of the time when someone does this, they want to _replace_ an
object embedded in another object. Take setting a source on a
subscription for example:

``` ruby
subscription.source = {
  object: 'card',
  number: 123,
}
subscription.save
```

In the case above, the serialized parameters should come out as:

```
source[object]=card&source[number]=123
```

That should apply even if the previous source had something else set on
it which we're not going to set this time -- say an optional parameter
like `source[address_state]`. Those should not be present at all in the
final serialized parameters.

(Another example is setting a `payout_schedule` as seen in #631 which is
PR is intended to address.)

There is an exception to this rule in the form of metadata though.
Metadata is a bit of a strange case in that the API will treat it as
additive, so if we send `metadata[foo]`, that will set the `foo` key,
but it won't overwrite any other keys that were already present.

This is a problem because when a user fully sets `metadata` to a new
object in Ruby, what they're probably trying to do is _replace_ it
rather than add to it. For example:

``` ruby
subscription.metadata
=> { old: 'bar' }

subscription.metadata = {
  new: 'baz'
}
subscription.save
```

To accomplish what the user is probably trying to do, we actually need
to send `metadata[old]=&metadata[new]=baz` so that we empty the value of
`old` while simultaneously setting `new` to `baz`.

In summary, metadata behaves different from other embedded objects in a
fairly fundamental way, and because the code is currently only set up to
handle the metadata case, it's not behaving correctly when other types
of objects are being set. A lot of the time emptying values like we do
for `metadata` is benign, but as we've seen in #631, sometimes it's not.

In this patch, I modify serialization to only empty out object values
when we see that parameter is `metadata`.

I'm really not crazy about the implementation here _at all_, but I'm
having trouble thinking of a better way to do it. One possibility is to
introduce a new class annotation like `empty_embedded_object :metadata`,
but that will have to go everywhere and might be error-prone in case
someone forgets it on a new resource type. If anyone has a suggestion
for an alternative (or can let me know if I'm missing something), I'd
love to hear it.

This PR is an alternate to #631.
2018-04-03 16:52:14 -07:00

61 lines
1.4 KiB
YAML

# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2018-02-23 14:17:07 +0100 using RuboCop version 0.50.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.
# Offense count: 19
Metrics/AbcSize:
Max: 45
# Offense count: 27
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/BlockLength:
Max: 469
# Offense count: 8
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 597
# Offense count: 11
Metrics/CyclomaticComplexity:
Max: 15
# Offense count: 259
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Metrics/LineLength:
Max: 310
# Offense count: 32
# Configuration parameters: CountComments.
Metrics/MethodLength:
Max: 45
# Offense count: 1
# Configuration parameters: CountComments.
Metrics/ModuleLength:
Max: 305
# Offense count: 6
# Configuration parameters: CountKeywordArgs.
Metrics/ParameterLists:
Max: 7
# Offense count: 5
Metrics/PerceivedComplexity:
Max: 17
# Offense count: 2
Style/ClassVars:
Exclude:
- 'lib/stripe/stripe_object.rb'
- 'test/stripe/api_resource_test.rb'
# Offense count: 53
Style/Documentation:
Enabled: false