Restful behavior not fully implemented in can.fixture.store.findOne #803

Closed
Alex-Krautmann opened this Issue Mar 19, 2014 · 6 comments

Comments

Projects
None yet
5 participants
@Alex-Krautmann

Currently, findOne on a store just indexes the store for the needed item. This should probably check for the existence of the item first, and either return it or send an error response.

VOTE HERE: http://bithub.com/event/32869

@justinbmeyer justinbmeyer added the Bug label Mar 19, 2014

@justinbmeyer justinbmeyer added this to the 2.1.0 milestone Mar 19, 2014

@markfeimer

This comment has been minimized.

Show comment
Hide comment
@markfeimer

markfeimer Mar 21, 2014

Could you please post some example code?

Could you please post some example code?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Mar 26, 2014

Contributor

@Alex-Krautmann If you have 30 min to an hour this week, I can work with you to setup a dev environment and fix this bug. Ping me on skype: justinbmeyer, or shoot me an email: justin AT bitovi. Thanks!

Contributor

justinbmeyer commented Mar 26, 2014

@Alex-Krautmann If you have 30 min to an hour this week, I can work with you to setup a dev environment and fix this bug. Ping me on skype: justinbmeyer, or shoot me an email: justin AT bitovi. Thanks!

@Alex-Krautmann

This comment has been minimized.

Show comment
Hide comment
@Alex-Krautmann

Alex-Krautmann Mar 27, 2014

Finally got some time to look at it. You can find a pretty self explanatory repo here:
https://github.com/Alex-Krautmann/fixture-store-example

Finally got some time to look at it. You can find a pretty self explanatory repo here:
https://github.com/Alex-Krautmann/fixture-store-example

@neektza neektza self-assigned this Apr 28, 2014

@ekryski

This comment has been minimized.

Show comment
Hide comment
@ekryski

ekryski Apr 30, 2014

Contributor

@Alex-Krautmann I started looking into this. It looks like findOne when using can.fixture.store does return the item if you are requesting with a valid id. It returns undefined otherwise. See code.

I would assume you are looking for a mock 404 to be returned if the resource isn't found? The only thing we will need to be conscious of, is still allowing a developer to create custom mock responses. For example, you might want to override the findOne call to return a 401 unauthorized.

Thoughts? @justinbmeyer @daffl @ccummings

Contributor

ekryski commented Apr 30, 2014

@Alex-Krautmann I started looking into this. It looks like findOne when using can.fixture.store does return the item if you are requesting with a valid id. It returns undefined otherwise. See code.

I would assume you are looking for a mock 404 to be returned if the resource isn't found? The only thing we will need to be conscious of, is still allowing a developer to create custom mock responses. For example, you might want to override the findOne call to return a 401 unauthorized.

Thoughts? @justinbmeyer @daffl @ccummings

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Apr 30, 2014

Contributor

Yes, he's looking for a 404. A developer can manually hookup their own fixture service and bypass the store all together if they need to show a 401. Or they could use store's hooks internally:

var thingsStore = can.store( ... );

can.fixture("/things", thingsStore.findAll);
can.fixture("/things/{id}", function(request, response){
  var item = thingsStore.find(request);
  if(!item){
    response(401,"error", ...)
  } else {
   ...
  }
})
Contributor

justinbmeyer commented Apr 30, 2014

Yes, he's looking for a 404. A developer can manually hookup their own fixture service and bypass the store all together if they need to show a 401. Or they could use store's hooks internally:

var thingsStore = can.store( ... );

can.fixture("/things", thingsStore.findAll);
can.fixture("/things/{id}", function(request, response){
  var item = thingsStore.find(request);
  if(!item){
    response(401,"error", ...)
  } else {
   ...
  }
})
@ekryski

This comment has been minimized.

Show comment
Hide comment
@ekryski

ekryski Apr 30, 2014

Contributor

There's a proposed kick at it. Let know what you think.

I think we could actually have some nice default errors for the other methods as well (ie. UPDATE, DELETE).

Contributor

ekryski commented Apr 30, 2014

There's a proposed kick at it. Let know what you think.

I think we could actually have some nice default errors for the other methods as well (ie. UPDATE, DELETE).

neektza added a commit that referenced this issue Apr 30, 2014

Merge branch 'fixture-findone-#803' of github.com:ekryski/canjs into …
…store_findone_fix

Conflicts:
	util/fixture/fixture.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment