Fix DbLookup type for db

[rikutiira]

There's a small hiccup in the current DbLookup type which db uses.

Eg. if you have something like schema.db.model, the type of the model is DbCollection which is actually wrong because of what db.createCollection does:

      // Public API has a convenient array interface. It comes at the cost of
      // returning a copy of all records to avoid accidental mutations.
      Object.defineProperty(this, name, {
        get() {
          let recordsCopy = newCollection.all();

          [
            "insert",
            "find",
            "findBy",
            "where",
            "update",
            "remove",
            "firstOrCreate",
          ].forEach(function (method) {
            recordsCopy[method] = function () {
              return newCollection[method](...arguments);
            };
          });

          return recordsCopy;
        },
      });

There's no db.model.all() function as the DbCollection type suggests. This PR changes the DbLookup type to match the actual implementation.

[samselikoff]

I see. Still green on TS but it seems right – the public API is db.users.

(This API was designed a long time ago and I think is definitely worth revisiting after we ship v1.0 and consider changes that would make the library more modern + TS-friendly overall).

Looks like the test is failing, could you take a look? https://travis-ci.com/github/miragejs/miragejs/jobs/399644394#L534

Maybe you can just remove all() altogether. (It's really not meant to be public API at all.)

[rikutiira]

I see. Still green on TS but it seems right – the public API is db.users.

(This API was designed a long time ago and I think is definitely worth revisiting after we ship v1.0 and consider changes that would make the library more modern + TS-friendly overall).

Looks like the test is failing, could you take a look? https://travis-ci.com/github/miragejs/miragejs/jobs/399644394#L534

Maybe you can just remove all() altogether. (It's really not meant to be public API at all.)

It seems like the overview-test.ts file had duplicate route definition for the movies route:

  routes() {
    this.namespace = "api";

    this.get("/movies", (schema, request) => {
      return schema.db.movies.all();
    });

    this.get("/movies");
    this.get("/movies/:id");
    this.post("/movies");
    this.patch("/movies/:id");
    this.del("/movies/:id");
  },

The return schema.db.movies.all(); was wrong and shouldn't have even worked but the this.get("/movies"); route shorthand was overwriting the previous route definition so it was never called.

I removed the first movies route definition altogether.

[rikutiira]

@samselikoff any update on this?

[samselikoff]

Sorry about the delay! Merged <3