Iss42 - learning resources module

[jeffreytu]

Addresses #42

Screencast: http://www.screencast.com/t/8qlm6jcOf

[jeffreytu]

The other current problem is if the user edits the current id, then goes to a different id and tries to hit the edit button, the form doesn't come up again. I think the form is still bound to the first id. Is there some sort of reset that needs to be called?

[jadedsurfer]

On line 84 of your test you cancel the changes so the expectations starting on line 105 will fail. You should move the beforeEach you have on line 73 to line 82 and duplicate it on line 97 so the tests don't have unintended interactions.

[jadedsurfer]

In the controller, in the showLearningResource method, you need to re-render the view with that record. Here's a quick and dirty way to do it.

this.fetchModel(learningResourceId, function(err, model){
      var view;

      if (err){
        view = this.renderError();
      } else {
        this.view.destroy();
        this.view = new View({model: model});
        view = this.renderView();
      }

[jeffreytu]

With the changes you're suggesting, I don't understand how the progression happens in the jasmine test. To me in the old cold, having the

        beforeEach(function(){
          view.$('#title').val('changed title');
          view.$('#desc').val('changed description');
          view.$('#authors').val('sis');
          view.$('#resourceType option:selected').val('link');
          view.render();
        });

right after describe('when the user clicks on the Edit button ', function(){ should run before each describe test in the block. Which means, describe('then clicks the cancel button', function(){ and describe('and then clicks on the Update button ', function(){ would get those new values. Since the cancel button is in it's own describe, I don't understand how that would cancel out the beforeEach I had at the beginning. Shouldn't the beforeEach would run again for the describe update?

Also I added in the changes you suggested, but still get the same matching errors of it seeing the initial values instead.

---

The edit button now works when switching id's, however I'm getting a new error on the controller.spec:

Chrome 40.0.2214 (Windows 7) Learning resource controller when calling showLearningResource with a valid resource type, renders the view FAILED
              display: none; /* Hides input box*/
            }
            ...
        Error: Expected element to have text 'fake render', but had '.edit, .b-cancel, .b-update, .b-delete, .div-space-edit{
            at cb (C:/Users/JSTADR~1/AppData/Local/Temp/25ac7c575ff07e6c139cf69ca01edbec55aab123.browserify:273:27 <- client\spa\js\learning-resource\spec\learning-resource.controller.spec.js:74:0)
            at null. (C:/Users/JSTADR~1/AppData/Local/Temp/25ac7c575ff07e6c139cf69ca01edbec55aab123.browserify:42:9 <- client\spa\js\learning-resource\learning-resource.controller.js:41:0)
            at Object.model.fetch.success (C:/Users/JSTADR~1/AppData/Local/Temp/25ac7c575ff07e6c139cf69ca01edbec55aab123.browserify:54:9 <- client\spa\js\learning-resource\learning-resource.controller.js:53:0)
            at success (C:/Users/JSTADR~1/AppData/Local/Temp/25ac7c575ff07e6c139cf69ca01edbec55aab123.browserify:251:17 <- client\spa\js\learning-resource\spec\learning-resource.controller.spec.js:52:0)

To me this sounds correct, since now the view is being re-rendered. Will it not be possible to have a 'fake render' anymore?

Another problem with this change. The current id is deleted from the database when going to a different id or refreshing the current id. Does it have something to do with the this.view.destroy(); you suggested? That code only sounds like the view is being emptied, not the model. Fixed this by removing this.view.destroy();. Edit between ids still work.

[goldlilys]

@jeffreytu I ran test:all for your branch and everything is passing. I think it is a windows issue that you're getting.

[jeffreytu]

I don't see any side effects on the front-side without the view.remove(). Would putting it just be for insurance then?

[goldlilys]

All is well when running gulp test, but when I do gulp test:unit:tdd, there's one error at

Chrome 40.0.2214 (Mac OS X 10.10.2) Learning resource controller when calling showLearningResource when switching to a different id does has a previous view to remove FAILED
    Expected spy remove to have been called.
        at Object.<anonymous> (/var/folders/g7/f6s075793fg7b9spwxnrqp_w0000gn/T/ef21f0803cbd5a2a50a708281503942bef4c4e1d.browserify:293:40 <- client/spa/js/learning-resource/spec/learning-resource.controller.spec.js:82:0)

Wow so many tests cases in the controller ... I think I need to add more for mine.

If you can, maybe simplify and create your editing interface as the main editing template for all of the different sections. Instead of adding a style to your local html, can you include it to /css/style.css ?

[jeffreytu]

Green button means ready for code review!

[goldlilys]

:+1: All tests pass including test unit. Had to make sure to install win-spawn module first though since that wasn't included as a dependency before.

[goldlilys]

When you have the time, can you also code review my branch for instructor?