Update commit logic to use the new format.

[martinmr]

This change changes the format of the data passed to the commit endpoint
to include the list of preds.

Also updates the port to the port of the test cluster and removes a test
that is not compatible with the change.

---

This change is 

[martinmr]

This is failing because the version that the script installs does not have the changes yet. We need to wait until those changes are in the latest official release to be able to merge these changes.

[paulftw]

Reviewed 2 of 5 files at r1.
Reviewable status: 2 of 5 files reviewed, 4 unresolved discussions (waiting on @martinmr and @paulftw)

---

lib/clientStub.js, line 78 at r1 (raw file):

    };
    DgraphClientStub.prototype.commit = function (ctx) {
        var reqMap = {};

Is this a generated file or duplicated code? See comment below about the reqMap.

---

src/clientStub.ts, line 107 at r1 (raw file):

        const preds = "preds";

        if (ctx.keys == null) {

idiomatic javascript for constructing this reqMap is:

const reqMap = {
  keys: ctx.keys || [],
  preds: ctx.preds || [],
}

no need for any extra consts and if-branches. Please rewrite.

---

src/clientStub.ts, line 119 at r1 (raw file):

        }

        const body = JSON.stringify(reqMap);

I think it's OK to inline this expression as body isn't used elsewhere.

Also a good idea to check maybe callAPI will do JSON.stringify when an object is passed instead of a string.

---

tests/helper.ts, line 3 at r1 (raw file):

import * as dgraph from "../src";

export const SERVER_ADDR = "http://localhost:8180"; // tslint:disable-line no-http-string

I don't see a corresponding change of the server config in this PR. Is this change specific to your machine?

[martinmr]

Reviewable status: 2 of 5 files reviewed, 3 unresolved discussions (waiting on @paulftw)

---

lib/clientStub.js, line 78 at r1 (raw file):

Previously, paulftw (Paul Korzhyk) wrote…

Is this a generated file or duplicated code? See comment below about the reqMap.

Yes, this is generated code

---

src/clientStub.ts, line 107 at r1 (raw file):

Previously, paulftw (Paul Korzhyk) wrote…

idiomatic javascript for constructing this reqMap is:

const reqMap = {
  keys: ctx.keys || [],
  preds: ctx.preds || [],
}

no need for any extra consts and if-branches. Please rewrite.

The linter was complaining (ctx.keys is not a real boolean so it shouldn't be used that way) but I managed to do something similar.

---

src/clientStub.ts, line 119 at r1 (raw file):

Previously, paulftw (Paul Korzhyk) wrote…

I think it's OK to inline this expression as body isn't used elsewhere.

Also a good idea to check maybe callAPI will do JSON.stringify when an object is passed instead of a string.

I tried but the compiler complains saying it needs a property name. I left it as is.
Passing reqMap directly to callAPI didn't work.

---

tests/helper.ts, line 3 at r1 (raw file):

Previously, paulftw (Paul Korzhyk) wrote…

I don't see a corresponding change of the server config in this PR. Is this change specific to your machine?

We usually run the tests in a dgraph cluster with an offset (by convention 100) so that the tests don't run in a prod or non-testing cluster by default. That's why I made the change. It looks like the build script for this repo is doing the same.

[paulftw]

This has fallen under the rug. Closing the PR without merging.

I think this commit logic has been implemented in another commit, since the tests are green.