Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

relationId issue #34

Closed
quinnvaughn opened this issue Mar 11, 2023 · 9 comments
Closed

relationId issue #34

quinnvaughn opened this issue Mar 11, 2023 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@quinnvaughn
Copy link

quinnvaughn commented Mar 11, 2023

I'm using a cuid as the id instead of a serial or uuid, so I call this method createId() from @paralleldrive/cuid2, and if I add that line, I am then unable to add a connection as just the id, I have to call a connect on the relation, for example

await db.state.create({
      ...state,
      id: createId(),
      country: { connect: { id: createdCountry.id } },
    })

as opposed to just setting countryId: createdCountry.id

However, if I remove the id field, I can do this (it does not work as id is null, but typescript doesn't freak out).

Is this normal, expected behavior? And if so, why?

@romeerez
Copy link
Owner

Please give more context, so I can try to reproduce it.

Share what your state and country table classes look like.

@quinnvaughn
Copy link
Author

Well it's not specific to either table. It's any table any time you custom add the id as the primary key.

Here's an address as an example.

export class AddressTable extends BaseTable {
  table = 'address'
  columns = this.setColumns((t) => ({
    id: t.varchar(36).primaryKey(),
    street: t.text().max(1000),
    postalCode: t.varchar(12),
    countryId: t.varchar(36).foreignKey('country', 'id').index(),
    stateId: t.varchar(36).foreignKey('state', 'id').index(),
    cityId: t.varchar(36).foreignKey('city', 'id').index(),
    ...t.timestamps(),
  }))

  relations = {
    country: this.belongsTo(() => CountryTable, {
      foreignKey: 'countryId',
      primaryKey: 'id',
    }),

    state: this.belongsTo(() => StateTable, {
      foreignKey: 'stateId',
      primaryKey: 'id',
    }),

    city: this.belongsTo(() => CityTable, {
      foreignKey: 'cityId',
      primaryKey: 'id',
    }),
  }
}

So id is a varchar of 36 as that's the maximum possible length of a cuid. Once I add an id: createId() the countryId, stateId and cityId will disappear as options and then I have to use the form like so

return await db.address.create({
        id: createId(),
        street: data.street,
        postalCode: data.postalCode,
        city: {
          connect: {
            id: city.id,
          },
        },
        state: {
          connect: {
            id: city.state.id,
          },
        },
        country: {
          connect: {
            id: city.state.countryId,
          },
        },
      })

So it would throw an error if I tried to do countryId: city.state.countryId instead of the prisma like connect, but it becomes an option if I remove the id: createId() so I'm just wondering if that's intentional or a bug?

@romeerez
Copy link
Owner

romeerez commented Mar 11, 2023

I reproduced it, it's a bug, thank you for finding it! I'll do my best to fix it asap.

The problem arises when id is set manually when creating, so the cases when id is a serial or uuid and generated automatically by the database are fine.

@romeerez romeerez added the bug Something isn't working label Mar 11, 2023
@romeerez romeerez assigned quinnvaughn and romeerez and unassigned quinnvaughn Mar 11, 2023
@romeerez
Copy link
Owner

It's fixed, update all orchid-related libraries to the latest version to get the fix.

@quinnvaughn
Copy link
Author

In regards to this, are there any future plans to add hooks to the tables? This way I could just add the createId() to a beforeCreate hook or something.

@romeerez
Copy link
Owner

Yes, I was thinking of supporting a JS-side callback in .default() method, so later it will be like this:

export class AddressTable extends BaseTable {
  table = 'address'
  columns = this.setColumns((t) => ({
    id: t.varchar(36).primaryKey().default(() => createId()),
  }))
}

And it's possible to add a custom column type with predefined methods (in the BaseTable definition), so define once id: t.varchar(36).default(() => createId()), and later to use it like:

export class AddressTable extends BaseTable {
  table = 'address'
  columns = this.setColumns((t) => ({
    id: t.id().primaryKey(),
  }))
}

And this override is currently possible in the ORM, but is yet to be handled in some way in the migrations.

Anything useful goes to the todo list, I'll be grateful if you continue trying different things with it and find more room for improvement.

@romeerez
Copy link
Owner

Now it's possible to set the id in a default callback:

export class AddressTable extends BaseTable {
  table = 'address'
  columns = this.setColumns((t) => ({
    id: t.varchar(36).primaryKey().default(createId),
  }))
}

And all create* methods (except createManyFrom) will use a new id for each new record.

@quinnvaughn
Copy link
Author

So just for my reference, this is just in the table definition, the migrations aren't aware of this yet, correct?

@romeerez
Copy link
Owner

Yes, so in a migration, it will be t.varchar(36).primaryKey() without a default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants