Skip to content

Grab id for exception message from request object#7

Closed
mecampbellsoup wants to merge 1 commit intotiagopog:masterfrom
mecampbellsoup:refactor-render-not-found
Closed

Grab id for exception message from request object#7
mecampbellsoup wants to merge 1 commit intotiagopog:masterfrom
mecampbellsoup:refactor-render-not-found

Conversation

@mecampbellsoup
Copy link
Copy Markdown
Contributor

No need for the regex as the desired id attribute is available as an instance variable on the request object.

Comment thread lib/jsonapi/utils.rb

def jsonapi_render_not_found(exception)
setup_request
id = exception.message.match(/=(\d+)/)[1]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tiagopog this regex was failing in our test suite because we are using UUIDs as primary keys, which aren't strings of strictly digits.

@mecampbellsoup
Copy link
Copy Markdown
Contributor Author

@tiagopog we should get automated builds set up for this gem, I think you can do it for free using Travis CI or Circle CI.

@tiagopog
Copy link
Copy Markdown
Owner

Sure, I actually started working on that. Up to the weekend I guess we're done.

@mecampbellsoup
Copy link
Copy Markdown
Contributor Author

👍

@tiagopog
Copy link
Copy Markdown
Owner

Well, in this case we would need to test when there is more than one identifier being passed in the request. I'm not sure right now if we could easily know which one was not found using only the request object. Through the exception message it's quite easy to "access" this information. But I'm totally open for suggestions, especially now considering the case of uuid's.

@tiagopog
Copy link
Copy Markdown
Owner

Hey, just let me know when you finish testing the suggested code against a request with multiple identifiers e.g. /users/:user_id/posts/:id – it needs to be capable to render a not found error pointing to the correct identifier (:user_id or :id).

@tiagopog
Copy link
Copy Markdown
Owner

I have some work to get done now, so I may also test it later :-)

@tiagopog tiagopog added the bug label Apr 13, 2016
@mecampbellsoup
Copy link
Copy Markdown
Contributor Author

Will do man, not going to be today, hopefully tomorrow or later this week :)

On April 13, 2016 at 11:38:02 AM, Tiago Guedes (notifications@github.com) wrote:

I have some work to get done now, so I may also test it later :-)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@tiagopog tiagopog closed this in #8 Apr 19, 2016
@mecampbellsoup mecampbellsoup deleted the refactor-render-not-found branch May 10, 2016 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants