SoftwareBrothers / adminjs-sequelizejs

Sequelizejs adapter for AdminBro
MIT License
27 stars 34 forks source link

SequelizeDatabaseError: column Provider.name does not exist #26

Closed MichaelLeeHobbs closed 2 years ago

MichaelLeeHobbs commented 4 years ago

Describe the bug Cannot sort of virtual columns. This is either a bug in sequlize or unsupported use case. I will be updating a similar issue is sequalize that appears to be related. https://github.com/sequelize/sequelize/issues/10983

Installed libraries and their versions

"dependencies": {
    "admin-bro": "^2.2.12",
    "admin-bro-expressjs": "^2.0.5",
    "admin-bro-sequelizejs": "^0.4.1",
    "aws-sdk": "^2.666.0",
    "cron": "^1.7.2",
    "cron-parser": "^2.13.0",
    "dotenv": "^8.2.0",
    "express": "^4.17.1",
    "express-fileupload": "^1.1.7-alpha.3",
    "express-formidable": "^1.2.0",
    "fs-extra": "^8.1.0",
    "json2csv": "^4.5.4",
    "mirth-api": "^0.0.5",
    "moment": "^2.24.0",
    "morgan": "^1.10.0",
    "node-fetch": "^2.6.0",
    "nodemailer": "^6.3.1",
    "pg": "^7.14.0",
    "sequelize": "^5.21.2",
    "swagger-jsdoc": "^4.0.0",
    "vm2": "^3.8.4"
  }

To Reproduce Steps to reproduce the behavior:

  1. Create a model with a virtual field, example
    
    /* jshint indent: 1 */

module.exports = (sequelize, DataTypes) => { let Provider = sequelize.define('Provider', { nationalProviderIdentifier: { type: DataTypes.INTEGER, allowNull: false, unique: true }, familyName: { type: DataTypes.STRING, allowNull: false }, givenName: { type: DataTypes.STRING, allowNull: false }, additionalName: { type: DataTypes.STRING, allowNull: false, defaultValue: '' }, name: { // type: DataTypes.VIRTUAL(DataTypes.STRING, ['givenName', 'familyName', 'additionalName']), type: DataTypes.VIRTUAL, get: function() { return ${this.getDataValue('givenName')} ${this.getDataValue('familyName')} ${this.getDataValue('additionalName')}.trim() } }, } )

return Provider

}

2. Open Admin Bro to the resource

**Expected behavior**
Throw an error to the effect of `Cannot sort on VIRTUAL Datatype "${sortBy}"! You can provide a different sort by field to avoid this issue, see: https://adminbro.com/ResourceOptions.html#sort`

Could sort by the primary key when the default sort by key is virtual but that could create an issue where it's not sorting as expect and the user has no clue why.

**Additional context**
Fix: https://github.com/SoftwareBrothers/admin-bro-sequelizejs/blob/master/src/resource.js

... const { Op, DataTypes } = require('sequelize') ... const { direction, sortBy } = sort if (this.SequelizeModel.fieldRawAttributesMap[sortBy].type instanceof DataTypes.VIRTUAL) { throw new Error(Cannot sort on VIRTUAL Datatype "${sortBy}" on resource "${this.SequelizeModel.name}"! You can provide a different sort by field to avoid this issue, see: https://adminbro.com/ResourceOptions.html#sort) } const sequelizeObjects = await this.SequelizeModel .findAll({ where: convertFilter(filter), limit, offset, order: [[sortBy, (direction || 'asc').toUpperCase()]], })

MichaelLeeHobbs commented 4 years ago

A related issue is Admin-Bro

api-controller.js

    const sortBy = decorated.options && decorated.options.sort && decorated.options.sort.sortBy || titlePropertyName
    const records = await resource.find(filter, {
      limit: 50,
      sort: {
        //sortBy: titlePropertyName,
        sortBy,
        direction: 'asc'
      }
    });
MichaelLeeHobbs commented 4 years ago

I'm taking a look at adding tests now. Hopefully, I will be able to add them to a pull request today.

MichaelLeeHobbs commented 4 years ago

The fix only partially works. If a user clicks on a virtual column field then the fix fails as in the backend throws the expected error and the frontend fails to load.

MichaelLeeHobbs commented 4 years ago

This is turning out the be a deeply complex issue.

module.exports = (sequelize, DataTypes) => {
    let Provider = sequelize.define('Provider',
        {
            nationalProviderIdentifier: {
                type: DataTypes.STRING,
                allowNull: false,
                unique: true
            },
            familyName: {
                type: DataTypes.STRING,
                allowNull: false
            },
            givenName: {
                type: DataTypes.STRING,
                allowNull: false
            },
            additionalName: {
                type: DataTypes.STRING,
                allowNull: false,
                defaultValue: ''
            },
            name: {
                // type: DataTypes.VIRTUAL(DataTypes.STRING, ['givenName', 'familyName', 'additionalName']),
                type: DataTypes.STRING,
                get: function () {
                    return `${this.getDataValue('familyName')}, ${this.getDataValue('givenName')} ${this.getDataValue('additionalName')}`.trim()
                },
                set: function (value) {

                }
            },

            // name: {
            //     // type: DataTypes.VIRTUAL(DataTypes.STRING, ['givenName', 'familyName', 'additionalName']),
            //     type: DataTypes.VIRTUAL,
            //     get: function () {
            //         return `${this.getDataValue('familyName')}, ${this.getDataValue('givenName')} ${this.getDataValue('additionalName')}`.trim()
            //     }
            // },
        }
    )

    Provider.adminBroConfig = {
        resource: Provider,
        options: {
            // sort: {sortBy: 'familyName'},
            editProperties: ['nationalProviderIdentifier', 'familyName', 'givenName', 'additionalName'],
            listProperties: ['nationalProviderIdentifier', 'name'],
            properties: {
                name: {isDisabled: true},
            }
        },
    }
    return Provider
}

The comment out code is the correct way to create a virtual column, however, this creates all sorts of issues that are not easily worked around. Instead, I created a real column "name" that is effectively virtual and resolves all the issues to the best of my knowledge.

At this point, I don't have a good fix beyond this ugly hack. I think to truly fix this it will take an overhaul of how admin-bro-sequelizejs/admin-bro sorts. I will take a look at this latter as there another issue. When sorting associate columns it does not sort as expected. For example, when sorting on name column above in the associate table it seems to sort by id and not alphabetical. I suspect this might be a sequlize issue but don't know at this point.

sp1r1t commented 2 years ago

Any updates on this? We are facing the same issue, as we want to show a nice string in a foreign key select instead of an id.

dziraf commented 2 years ago

I think your error comes from the fact that name field is used by default for sorting: https://github.com/SoftwareBrothers/adminjs/blob/a0d2a17d55af39e03cf3b3f5ab8cd5c09bcaaeb6/src/backend/services/sort-setter/sort-setter.ts#L40 https://github.com/SoftwareBrothers/adminjs/blob/926799a64bc9ca40b7e763dc2adf2b4c73f3f42f/src/backend/decorators/resource/resource-decorator.ts#L251

which is basically a title property. You can

a) specify sort config in your resource with a different sortBy that is not virtual: https://docs.adminjs.co/ResourceOptions.html#sort b) set isTitle: false for name and set isTitle: true to something else (https://docs.adminjs.co/PropertyOptions.html#isTitle)