search

jarrodconnolly / sequelize-slugify

Sequelize Slugify is a plugin for the Sequelize ORM that automatically creates and updates unique, URL safe slugs for your database models.
https://sequelize-slugify.nestedquotes.ca/
MIT License
56 stars 27 forks source link

Not working with DataTypes.VIRTUAL #53

Open lucassimines opened 3 years ago

lucassimines commented 3 years ago

What steps will reproduce the bug?

Page.init(
    {
      id: {
        type: DataTypes.UUID,
        defaultValue: UUIDV4,
        allowNull: false,
        primaryKey: true
      },
      title: {
        type: DataTypes.JSONB,
        allowNull: false
      },
      _title: {
        type: DataTypes.VIRTUAL,
        get() {
          const title: any = this.get('title');
          return title[langs[0]];
        }
      },
    },
    {
      sequelize,
      tableName: 'pages',
      modelName: 'Page'
    }
  );

  SequelizeSlugify.slugifyModel(Page, { source: ['_title'] });

How often does it reproduce? Is there a required condition?

What is the expected behavior?

The slug column is not getting _title Virtual column as the documentation says.

Screen Shot 2021-07-09 at 14 49 01

What do you see instead?

Additional information

lucassimines commented 3 years ago

I resolved the issue by removing the following lines:

// if nothing change (new appears as changed)
      if(!forceGenerateSlug && !changed) {
        return instance;
      }
jarrodconnolly commented 3 years ago

Thank you, that is helpful. I am not sure removing that is the best course of action as it will likely have other side effects.

I will have to look into why DataTypes.VIRTUAL is not generating the slug.

jarrodconnolly commented 3 years ago

It appears that fields of type DataTypes.VIRTUAL are not detected as changed, which makes sense as there is no way to know the previous version since it is not a stored field.

The documentation referenced appears to have been added without unit tests to back it up :(

I will continue to investigate, but in the meantime, it may be safer to manually call regenerateSlug() as in the unit test that I added. https://github.com/jarrodconnolly/sequelize-slugify/blob/main/jest-tests/slug-generation.test.js#L70

lucassimines commented 2 years ago

@jarrodconnolly any update about this?

agjs commented 1 year ago

@jarrodconnolly Hey :-). Any updates on this, and a side question, is the library still being maintained? Some of these issues have been here for a while, and I wonder if you maybe need a hand with this? I'm happy to help if you are willing to pair on it.

Cheers

jarrodconnolly commented 1 year ago

Hello, thank you for your comments.

This library is still maintained, I try to investigate and solve any issues as they come up. This issue was left open because at the time could only find a workaround as described and added a unit test to show the issue.

I am always open to pull requests or suggestions on how to solve issues.