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

Todo example - getById() with String #26

Closed
tylorr opened this issue Dec 11, 2013 · 5 comments
Closed

Todo example - getById() with String #26

tylorr opened this issue Dec 11, 2013 · 5 comments

Comments

@tylorr
Copy link

tylorr commented Dec 11, 2013

I was playing around with the todo example and I found a little bug. When I try to access this url: /todos/0 I get an error message saying Todo not found. (And I know there is a todo with id 0).

I discovered that the TodoStore.Get() method receives the id as a string then passes it along to the TodoStore.getById() method which compares the passed in id to the ids in the todos list using the === operator. Since the ids stored in the todos list are numbers and do not match the type of the passed in string, the comparison fails.

The get method is never used in the todos example, so it probably never showed up.

Some possible solutions are to convert the string to a number before passing it to getById or use the == operator so that it will not check type as well.

@tylorr
Copy link
Author

tylorr commented Dec 11, 2013

Further investigation reveals that the get method should probably be receiving the id as a number, like update and remove does.

@daffl
Copy link
Member

daffl commented Dec 11, 2013

Thanks for catching that! I changed the === in the example to ==. I'm wondering why it doesn't get it as an actual number though (especially if update and remove get it as a number).

@daffl
Copy link
Member

daffl commented Apr 16, 2014

Upon a little further investigation I think the conversion needs to be done on the service level (for example with feathers-hooks) since in many cases the id isn't an actual number. Closing this issue but still open for other suggestions of course.

@daffl daffl closed this as completed Apr 16, 2014
@Glavin001
Copy link
Contributor

the conversion needs to be done on the service level (for example with feathers-hooks) since in many cases the id isn't an actual number.

Agreed. feathers-hooks to the rescue!

daffl pushed a commit that referenced this issue Aug 21, 2018
daffl pushed a commit that referenced this issue Aug 21, 2018
daffl added a commit that referenced this issue Aug 21, 2018
daffl added a commit that referenced this issue Aug 22, 2018
daffl pushed a commit that referenced this issue Aug 25, 2018
daffl added a commit that referenced this issue Aug 28, 2018
* chore(package): update chai to version 4.0.2

Closes #24

* Use latest sinon-chai
daffl added a commit that referenced this issue Aug 29, 2018
* chore(package): update chai to version 4.0.2

Closes #26

* Use latest sinon-chai
daffl pushed a commit that referenced this issue Aug 29, 2018
daffl added a commit that referenced this issue Aug 29, 2018
* chore(package): update chai to version 4.0.2

Closes #24

* Use latest sinon-chai
daffl added a commit that referenced this issue Aug 29, 2018
* chore(package): update chai to version 4.0.2

Closes #26

* Use latest sinon-chai
daffl pushed a commit that referenced this issue Aug 29, 2018
@lock
Copy link

lock bot commented Feb 8, 2019

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue with a link to this issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants