Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix jsonapi.rb update breakages #128

Open
zwolf opened this issue Feb 10, 2020 · 2 comments
Open

Fix jsonapi.rb update breakages #128

zwolf opened this issue Feb 10, 2020 · 2 comments

Comments

@zwolf
Copy link
Member

zwolf commented Feb 10, 2020

jsonapi.rb has updated and changed the way that its error handling and deserialization work.

  • The error handler no longer includes ActiveSupport::Concern, and therefore doesn't have rescue_from for the ErrorExtender to work with. The base.class_eval { rescue_from(...) } syntax seems to have replaced it for some reason. ErrorExtender needs to be updated to work somehow. See stas/jsonapi.rb@ace0e24#diff-469289c4fae6d78ec04d5ecd3df83beeL10

  • The deserializer went from type checking for ActionController::Parameters (fine) to checking document.respond_to?(:permit!), which is not fine because the goal was to NOT use strong params and instead to use a whitelist of params to the deserialize method. See stas/jsonapi.rb@ace0e24#diff-e86fe4b4d2ba087be81a0a031f138fd8L19

These changes aren't great for us, frankly, but I see how they move the library to being less tightly coupled to Rails, which is a net positive.

@nciemniak
Copy link
Contributor

The deserializer went from type checking for ActionController::Parameters (fine) to checking document.respond_to?(:permit!), which is not fine because the goal was to NOT use strong params and instead to use a whitelist of params to the deserialize method. See stas/jsonapi.rb@ace0e24#diff-e86fe4b4d2ba087be81a0a031f138fd8L19

Interestingly enough, the params that we are passing to jsonapi_deserialize in the TranscriptionController do respond to the permit method (i.e. params.respond_to?(:permit!) evaluates to true), which means they pass through the serializer the same way as they did before the update. So... we should be fine on this front.

@zwolf
Copy link
Member Author

zwolf commented Mar 10, 2020

I'm not seeing the error I saw previously in the broken travis builds, so maybe something changed in an update. They're still failing (the ErrorExtender one and another related to coveralls) but yeah, this seems fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants