Deprecate Domain expires_on in favor of expires_at

[duduribeiro]

We deprecated expires_on attribute in our documentation in favor of
expires_at. This commit introduces the new field and deprecates the old
one. It also updates to use the new fixtures.

This is not a breaking change, since both old methods still exists.

[weppos]

Please do not make it a breaking changes. We can easily have expires_on being a getter/setter that reads and writes expires_at, and prints out a warning.

No need to break a client and make it a major bump (especially as we just went through another major bump a few weeks ago).

[weppos]

@duduribeiro my review is not required here, it's not business critical and we've plenty of team members familiar with Ruby. Once the changes I suggested are implemented, feel free to tag someone else as reviewer.

As a general rule, the less breaking changes we introduce, the easier will be to merge and ship changes. Otherwise this PR would have had to sit here for long time until we shipped another major review (shipping 2 majors in a month would be a bit unreasonable).

[jacegu]

Removing myself as reviewer until requested changes are implemented.

[jacegu]

@duduribeiro the approach you took of adding the deprecation warnings in the attribute getter and setter triggers the warning building an domain from the response because we are still returning expires_on in the payload:

irb(main):007:0> client.domains.domains(2835)
[DEPRECATION] Domain#expires_on= is deprecated. Please use `expires_at=` instead.
[DEPRECATION] Domain#expires_on= is deprecated. Please use `expires_at=` instead.
[DEPRECATION] Domain#expires_on= is deprecated. Please use `expires_at=` instead.
[DEPRECATION] Domain#expires_on= is deprecated. Please use `expires_at=` instead.
[DEPRECATION] Domain#expires_on= is deprecated. Please use `expires_at=` instead.
=> #<Dnsimple::PaginatedResponse:0x00007fda9ad260c8 @http_response=#<HTTParty::Response:0x168f0 parsed_response={"data"=>[{"id"=>232454, "account_id"=>2835, "registrant_id"=>nil, "name"=>"domain.com", "unicode_name"=>"domain.com", "state"=>"hosted", ...

Is that intended?

[jacegu]

@duduribeiro after looking into this with a fresh 🧠 this morning here are my conclusions:

1. We need to keep both expires_on and expires_on=. However I would not change their behaviour. That is, I would simply redefine them to add the warning in you introduced on the first place in 0892766. The getter should still return @expires_on. The setter should still set @expires_on. They should NOT touch @expires_at.

2. We need to overwrite the initializer in Dnsimple::Struct::Domain so we set @expires_on based on the value of expires_at and so we never call expires_on= (first implementation that comes to mind):

   def initialize(attributes = {})
     attributes.delete("expires_on")
     super
     @expires_on = Date.parse(expires_at).to_s if expires_at
   end

With that ☝️ we no longer depend on the value returned from the server and, essentially, we have the same behaviour that we have today, with the extra deprecation warnings.

[jacegu]

👍

[dallasread]

Removing myself as reviewer as there are already 2, though I will keep an eye on the conversation.

[san983]

I removed the requires-major-release label since this can be addressed as a minor 👍