Custom resource attributes

[ryanwjackson]

Is your feature request related to a problem? Please describe.
NetSuite (and other ledgers) support customer fields/attributes on resources. Right now, you must specify each attribute per resource (and in operation validations).

Thoughts/Ideas

1. ResourceAttributeSet
   Right now we store all attributes in a ResourceAttributeSet object. We could modify resources to have an .attributes instance method instead of attributes being directly accessible. Then we can have that set accept any key/value pair.

2. Add a custom_attributes: {} keyword to resource initializer

3. Add a way to specify custom attributes when calling LedgerSync.register_resource (or somewhere when instantiating LedgerSync)

This would require devs to know and specify their custom attributes. NetSuite does provide a dynamic schema/metadata through their API. I don't know that this holds for other ledgers that offer this level of customization. In theory, they may just return custom attributes as part of the record's response body and we may not have a formal way to detect. So perhaps being intentional and having devs define these attributes makes sense.

4. Add raw body to resource

Ideally, resources will remain adaptor independent. But there is data loss (even now) for all the attributes we aren't defining. You can access this data through the Response object body, but it's pretty inconvenient. Perhaps we can add an adaptor_body attribute to resources or something?

Appendix
For all of these we would have to determine how to handle operation validations, which right now require that you define a validation for every attribute.

[ryanwjackson]

@mattgmarcus @jozefvaclavik

[ryanwjackson]

I've been investigating how this works, and I want to enumerate some options I have come up with. For clarity, we use the term attribute (not property, field, etc.). Some ledgers use different lingo or may even use them interchangeably.

First, let's discuss where/how we use attributes today:

1. They are explicitly defined on each resource with a type.
2. They must be included in the validation section of each operation for that resource.

Challenges:

1. The enforced validation and type definition ensures we are intentional about each attribute value.
2. Different ledgers will have different attributes and/or the implementation using the lib may required different fields.
3. How do we validate when an attribute value is set, but that attribute is not used in the operation (e.g. you set subsidiary but QBO does not use this field)?

Thoughts:

1. At a minimum, we need a way to store the raw output of the API against the resource so all fields are accessible, explicitly defined or not.

Options

Option 1

Modify ResourceAttributeSet to support custom attributes so that we could do something like the following:

resource = LedgerSync::Customer.new

resource.attributes # returns a `ResourceAttributeSet`
resource.attributes.name # returns the name of the customer
resource.name # raises MethodNotDefined error, because we will move it to `attributes`

resource.custom_attributes # a `ResourceAttributeSet` of user- or lib- defined attributes

We can allow operations access to both sets so they can perform the necessary validations.

Option 2

Create a CustomAttribute resource and use it via references_many:

resource.custom_attributes # returns an array of CustomAttributes

Operations could validate on these with the current pattern. One benefit of this approach is that NetSuite allows you to manage custom attributes (fields as they call them) via the API (e.g. create custom field, edit custom field, etc.). Since in this approach they would be resources, we could build operations around them as well. This may be required anyway, given that when you onboard a customer you may need to create custom attributes for your integration to work.

Option 3

We could turn ResourceAttributeSet into a catch-all, similar to FactoryBot. Essentially you could do the following:

resource.attributes.name = 'Ryan'
resource.attributes.foo = :bar
resource.attributes.whatever = 123

We would not be able to declare every attribute on the resource or in operation validations. There is flexibility in this approach, but I admit that it seems very "monkey-patchy." I recently tried to patch FactoryBot for this lib, and it was a nightmare. To achieve the catch-all nature, you must override method_missing, explicitly allowing the methods you want defined and then being clever about setting/getting the rest.

Open Questions

• How do we determine if an attribute is custom or not when pulling a record from the NS REST API?
• Ensure we can edit the values of customer attributes on a given resource. I assume you can, but I haven't tested yet.

[mattgmarcus]

I feel like I'm leaning towards #1 or #2. I don't really want developers to have to write code for the custom attributes. If there's a good way to manage this through configuration, that could be nice. For example, if we had a CustomAttribute model, we could do something like

attr = CustomAttribute.new
attr.type = :int 
attr.value = 2

resource.custom_attributes << attr

And then write that to the ledger using their API

Let's talk about this tomorrow

[ryanwjackson]

Random thoughts for discussion:

• There also may be a way to make this part of the config.rb of an Adaptor

If devs will be using different custom attributes, then the operations need to be able to account for that. For example, a Create needs to know you want to save that CustomAttribute. Or perhaps we can also make the operation.perform method take a block:

operation.perform do |adaptor:, resource:|
  # Do something here.
end

One challenge there is that each adaptor has different ways of interacting with the ledger. For example, QBO is Faraday requests and Stripe is ruby objects.

[ryanwjackson]

From sync:

• consumer of library can provide custom serializer for operation
• CustomAttributeCollection and CustomAttribute, type is optional

[ryanwjackson]

After some more work on this, I see a couple solutions that are worth discussing:

Solution 1

customer = LedgerSync::Customer.new(
  name: 'Test Name',
  foo: 'bar',
  custom_attributes: {
    LedgerSync::ResourceAttribute::Custom.new(
      name: :foo,
      type: Type::String
    )
  }
)

customer.name # => 'Test Name'
customer.foo # => 'bar'
customer.custom_attributes # => [
  <LedgerSync::ResourceAttribute::Custom @name="foo">
]

class CustomLedgerSerializer < LedgerSync::NetSuite::Customer::LedgerSerializer
  attribute ledger_attribute: 'custom_foo_field_in_ledger',
            resource_attribute: :foo
end

op = LedgerSync::NetSuite::Customer::Create.new(
  adaptor: adaptor,
  ledger_deserializer: CustomLedgerSerializer,
  ledger_serializer: CustomLedgerSerializer,
  resource: customer
)

op.perform

Solution 2

class CustomCustomer < LedgerSync::Customer
  attribute :foo, type: Type::String
end

customer = CustomCustomer.new(
  name: 'Test Name',
  foo: 'bar'
)

customer.name # => 'Test Name'
customer.foo # => 'bar'
customer.custom_attributes # NotDefinedError

class CustomLedgerSerializer < LedgerSync::NetSuite::Customer::LedgerSerializer
  attribute ledger_attribute: 'custom_foo_field_in_ledger',
            resource_attribute: :foo
end

op = LedgerSync::NetSuite::Customer::Create.new(
  adaptor: adaptor,
  ledger_deserializer: CustomLedgerSerializer,
  ledger_serializer: CustomLedgerSerializer,
  resource: customer
)

op.perform

Other

Operation validation contracts are hard-coded into the operation. We can get make this more dynamic by allowing it to be passed in on operation initialization. This will at least give you the ability validate at the operation level if you want to.

[ryanwjackson]

What I like about Solution 2 is that we don't mix concepts of attributes. I find the LedgerSync::Customer.new(name: 'test', custom_attributes: [...]) to be confusing because name is a value of an attribute whereas custom_attributes is something "special"/functional.

[jozefvaclavik]

I like 2nd option. It's clean. If user needs to write custom serializer to handle the logic, defining custom resource should not be an issue.

[ryanwjackson]

As per live discussions, we will go with Solution 2.