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

Brian-lab #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

brials
Copy link

@brials brials commented Mar 1, 2017

No description provided.

model/rwby.js Outdated
let rwby = new Rwby(_rwby.name, _rwby.weapon, _rwby.symbol);
return storage.createItem('rwby', rwby);
} catch (err) {
return Promise.reject(err);
Copy link

Choose a reason for hiding this comment

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

Please have more detailed errors using createError like such:
catch (err) {
return Promise.reject(createError(400, err.message));

This will help you meet the testing requirements for this assignment as well.

model/rwby.js Outdated

Rwby.fetchRwby = function(id){
debug('Fetch RWBY');
storage.fetchItem('rwby', id);
Copy link

Choose a reason for hiding this comment

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

You're missing 'return' for your fetchItem functionality. So try:
return storage.fetchItem('rwby', id);

Copy link

Choose a reason for hiding this comment

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

This should fix your get tests.

.catch( err => next(err));
});

rwbyRouter.put('/api/rwby/:id', function(req, res, next){
Copy link

Choose a reason for hiding this comment

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

You're missing jsonParser middleware here: Try the following:
rwbyRouter.put('/api/rwby/:id', jsonParser, function(req, res, next){

Copy link

Choose a reason for hiding this comment

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

I'm sure you understand why we need this. If not, we can talk about it tomorrow, but in the meantime, this should also fix your post test.

}
});
it(' should return a rwby', done => {
let updateRwby = {name :'new name', content : 'new content'};
Copy link

Choose a reason for hiding this comment

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

I'm assuming this was the part you copied from the lab, remember to have the appropriate parameters as such:
let updateRwby = { name: 'new name', weapon: 'new weapon', symbol: 'new symbol' };

});
it(' should return a rwby', done => {
let updateRwby = {name :'new name', content : 'new content'};
request.put(`${url}/api/rwby?id=${this.tempRwby.id}`)
Copy link

Choose a reason for hiding this comment

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

Also, keep in mind that if you decide to use req.params.id, you need to format your endpoints differently as well:
request.put(${url}/api/rwby/${this.tempRwby.id})
For future reference:
req.params.id = /${id}
req.query.id = ?id=${id}

Copy link

Choose a reason for hiding this comment

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

This should fix your put test.

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

Successfully merging this pull request may close these issues.

2 participants