Refactor with Sync 1.0 changes

[superkhau]

Connect to strongloop-internal/scrum-loopback#214

[bajtos]

Can you please make the commit message of 7a545e7021fdc69d0cc96cd803f6722b1ac3414e more explanatory? Change tracking was already enabled.

[superkhau]

@bajtos Running grunt alone shows this error:

Running "karma:unit" (karma) task
INFO [karma]: Karma v0.12.31 server started at http://localhost:8080/
INFO [launcher]: Starting browser PhantomJS
WARN [watcher]: Pattern ".../loopback-example-offline-sync/client/ngapp/test/mock/**/*.js" does not match any file.
INFO [PhantomJS 1.9.8 (Mac OS X)]: Connected on socket z-UTHdFtwZ2ZLg01gBEa with id 33976604
PhantomJS 1.9.8 (Mac OS X) ERROR
  TypeError: 'undefined' is not a function (evaluating 'Number.isFinite(parseInt(v, 10))')
  at .../loopback-example-offline-sync/client/lbclient/browser.bundle.js:16224

Warning: Task "karma:unit" failed. Use --force to continue.

Aborted due to warnings.

Do you know what this means?

[superkhau]

Also getting:

Running "mochaTest:server" (mochaTest) task

  Todo
    New todo empty data
      POST /api/Todos
        ✓ should be allowed
        ✓ should have statusCode 200
        1) should respond with a new todo # this is failing
  ...
  1 failing

  1) Todo New todo empty data POST /api/Todos should respond with a new todo:
     AssertionError: null == true
      at Context.<anonymous> (server/test/todo.test.js:23:9)

For some reason, the test is returning an actual mongo id (might be something to do with the "before save" operation hook).

[bajtos]

Number.isFinite

See https://github.com/strongloop/loopback-boot/pull/124

Todo New todo empty data POST /api/Todos should respond with a new todo

No idea.

this.res.body.id returns an actual MongoDB id instead of the t-math id. Any idea why this is happening?

I think that's because all Todo ids are initialized with a GUID value - see "defaultFn": "guid" in model config. This is intended and required for sync to work correctly.

[superkhau]

See https://github.com/strongloop/loopback-boot/pull/124

I see, will bump loopback-boot version when that gets merged.

No idea.

Going to skip the test then.

I think that's because all Todo ids are initialized with a GUID value - see "defaultFn": "guid" in model config. This is intended and required for sync to work correctly.

I see. So it's possible this initialization is causing that one test to fail then. Anyways, I'm skipping the test, so its not blocking what we're trying to do anyways.

[bajtos]

Going to skip the test then.

No no, don't skip the test, fix it instead. As you can see, it's checking the format of the id property:

assert(this.res.body.id.match(/t-\d+/));

Since you are no longer generating ids as t-XYZA, you should rework the test to check something else that makes sense. For example that id has a GUID format.

Or simply remove it('should respond with a new todo') completely, since the other tests are already testing that a new Todo instance can be created.

[superkhau]

No no, don't skip the test...

Updated the test to check for proper GUID format.

[superkhau]

@bajtos Please code review. If you see anything obvious that needs fixing, please add it to this PR yourself as I will be away for 2 weeks. If there is no hurry to merge this, then I can finish it up when I'm back. As for the easy-to-read GUID feature, I created a separate issue at https://github.com/strongloop/loopback-example-offline-sync/issues/62 and will work on it after I'm back.

[chandadharap]

Assigning back to @bajtos , Simon on PTO

[bajtos]

Reassigning back to Simon, offline sync is not urgent now.

[ritch]

@superkhau @bajtos is this good to go?

[superkhau]

@ritch Almost. I need to refactor the configs based on @bajtos's comments and then it should be good to go.

[superkhau]

@bajtos Code review again please.

[bajtos]

Nice! I left one more comment, feel free to ignore it. The rest LGTM.