search

lessonly / scim_rails

SCIM Adapter for Rails.
MIT License
68 stars 76 forks source link

Update for use with Rails 5.2 #9

Closed armilam closed 5 years ago

armilam commented 5 years ago

Why?

This gem doesn't currently have a need to prevent anyone from using Rails up to 5.2.x. In fact, we are upgrading our core app beyond Rails 5.0, so supporting higher versions of Rails will be important to us.

What?

This PR allows Rails versions up to but not including 6. It's possible this could support Rails 6, but that hasn't been tested, so we're going to keep the restriction at 5.x.x for now.

This commit also fixes a reference to the test Rakefile, which is in a different folder than originally specified.

Credit goes to @henrypoydar for this work. I've just updated it a little.

Testing Notes

This doesn't change any functionality, so there isn't really anything to test here. Once we upgrade our app to Rails 5.1, we will need to make sure then that SCIM still works.

armilam commented 5 years ago

Fixes #6

This is an update to #7

armilam commented 5 years ago

Whoops, I definitely broke something. I'm looking into that.

armilam commented 5 years ago

An update: Rails 5.3 (as of https://github.com/rails/rails/pull/35549 - a month ago!) the parsing of the response object's Content-Type has changed.

Before this change a Content-Type of application/scim+json, application/json would make it to the client. After this change, the parser removes everything after the space.

That PR and the tests for that file don't refer to the possibility of there being a comma-separated value for a response's Content-Type, but I think that is technically correct. It doesn't make sense for a response to have multiple content types (except when specifying the type of each part of a multipart response). So I think that the application/scim+json, application/json is technically wrong and should be just application/scim+json.

I am going to make that update.