search

JsonApiClient / json_api_client

Build client libraries compliant with specification defined by jsonapi.org
MIT License
362 stars 186 forks source link

fix: Multi-word custom endpoint not respecting route format #400

Closed sebasjimenez10 closed 6 months ago

sebasjimenez10 commented 2 years ago

This PR fixes an issue I experienced when working with multi-word custom endpoints. If you have a multi-word custom endpoint and you also have the self.route_format = :dasherized_route config in your base class, upon calling that custom_endpoint JsonApiClient doesn't respect the configuration selected and will use the underscored version of the endpoint name.

For instance:

Given,

class Pet < TestResource
  self.site = "http://example.com/"
  self.route_format = :dasherized_route

  custom_endpoint :related_pets, on: :member, request_method: :get
  custom_endpoint :vip_pets, on: :collection, request_method: :get
end

When calling Pet.vip_pets or pet = Pet.new(...); pet.related_pets the resulting endpoint being called will be either: http://example.com/pets/vip_pets or http://example.com/pets/:id/related_pets

instead of http://example.com/pets/vip-pets or http://example.com/pets/:id/related-pets

Same issue applies for camelized_routes.

This PR fixes this by checking the route_format configuration attribute and making sure it is converted to the appropriate endpoint name, using ActiveSupport helper methods.

The methods defined in the instance of the class will still be underscored, so that we can still do Pet.vip_vets.

Happy to address any concerns or comments, thanks!

sebasjimenez10 commented 2 years ago

Hey @gaorlov 👋 giving this one a bump for you to take a look 🙏

esbanarango commented 2 years ago

What's the current blocker for merging this one? 🙏

gaorlov commented 2 years ago

Hello! Thanks for taking the time to contribute! I'll take a closer look tomorrow. I'm the meantime, can you please add an entry to the changelog?

Thanks!

Greg

sebasjimenez10 commented 2 years ago

@gaorlov updated the changelog! 🙏

sebasjimenez10 commented 2 years ago

@gaorlov small bump! 🙂

gaorlov commented 2 years ago

@sebasjimenez10 sorry about the delay. For reasons i don't yet understand the automated testing has stopped working. I'm working with the owners of the JsonAPiClient org to resolve this issue and will let you know when we can start merging again.

I appreciate your patience!

Greg

sebasjimenez10 commented 2 years ago

@gaorlov thank you so much for looking into it. No worries on the wait, I'm glad to contribute 🙏🏻

sebasjimenez10 commented 2 years ago

Hey @gaorlov, just dropping by and giving this one a bump! 🙏

sebasjimenez10 commented 1 year ago

Hey @gaorlov, hope you're doing good! Just wanted to bump this one!

esbanarango commented 1 year ago

🙏

sebasjimenez10 commented 1 year ago

@gaorlov done! Just rebased on top of master (main). Tests are looking good. Please let me know if there is anything else I need to do!

sebasjimenez10 commented 1 year ago

@gaorlov small bump!

sebasjimenez10 commented 12 months ago

@gaorlov tiny bump on this one! 🙏

gaorlov commented 6 months ago

thank you for you contributions and patience! 1.23.0 is now live with your changes