Skip to content

Iss23 manage learning resources api#56

Open
rcmadden wants to merge 8 commits intomasterfrom
Iss23_manage_learning_resources_API
Open

Iss23 manage learning resources api#56
rcmadden wants to merge 8 commits intomasterfrom
Iss23_manage_learning_resources_API

Conversation

@rcmadden
Copy link
Contributor

http://screencast.com/t/DwWNjKjN9QP9

  • Create Learning Resources Model
  • Can create, view, modify and delete learning resource
  • Frisby test - Create, view and modify Learning Resource tests pass

@rcmadden
Copy link
Contributor Author

Ready for review

@rcmadden rcmadden changed the title WIP Iss23 manage learning resources api Iss23 manage learning resources api Jan 28, 2015
@jeffreytu
Copy link
Contributor

  • Update your local master and do a git merge master so you can get the updated changes to the integration tests, since they fail at the moment
  • Perhaps add some value checkers for the resource type in learningResource.js. See the example in master.
  • I think the authors property should be an array. so if you just change to "type": "array" should be good

@goldlilys
Copy link
Contributor

Besides the tests not passing as Jeff mentioned, your api test at learningResource.spec.js is missing the deleteRecord function and you should remove the 'fullstack' in the datasources.json but instead move it to a local one. When running tests, there's lint errors which means the latest fix to the lint errors from master weren't merged yet.

@jeffreytu
Copy link
Contributor

Can we rename the learningResource model to learning-resource? In my branch, learningResource is a method in Backbone. It might be confusing for some.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants