Properties with mysql custom "columnName" don't get autoupdated

[darknos]

Description

#250 for 3.x

connect to #250

Checklist

• [+] New tests added or existing tests modified to cover all changes
• [+] Code conforms with the style
  guide

[slnode]

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[ssh24]

change this to var pName = (newSettings.mysql && newSettings.mysql.columnName) || self.client.escapeId(propName);

[darknos]

but in this case columnName will be not escaped and we have possible issue with columnName like from because SQL will be like CHANGE COLUMN from from ...

[darknos]

I want to merge this PR asap, but why do you want to replace documented and widely used self.columnEscaped to that.

... ten lines above we use var colName = expectedColNameForModel(propName, m); for the same result.

please, confirm the change and, please, explain the reason

[ssh24]

Right. But what I am not understanding is, if we specify the columnName explicitly to be something different than the property name, how would this work?

[ssh24]

Something like this?

var colName = newSettings.mysql && newSettings.mysql.columnName || self.client.escapeId(propName);
var pName = self.columnEscaped(model, colName);

[darknos]

var pName = self.client.escapeId(newSettings.mysql && newSettings.mysql.columnName || propName);

do the same as

var pName = self.columnEscaped(propName);

/**
 * Get the escaped column name for a given model property
 * @param {String} model The model name
 * @param {String} property The property name
 * @returns {String} The escaped column name
 */
SQLConnector.prototype.columnEscaped = function(model, property) {
  return this.escapeName(this.column(model, property));
};

/**
 * Get the column name for the given model property. The column name can be
 * customized at the model property definition level as `column` or
 * `columnName`. For example,
 *
 * ```json
 * "name": {
 *   "type": "string",
 *   "mysql": {
 *     "column": "NAME"
 *   }
 * }
 * ```
 *
 * @param {String} model The model name
 * @param {String} property The property name
 * @returns {String} The column name
 */
SQLConnector.prototype.column = function(model, property) {
  var prop = this.getPropertyDefinition(model, property);
  var columnName;
  if (prop && prop[this.name]) {
    columnName = prop[this.name].column || prop[this.name].columnName;
    if (columnName) {
      // Explicit column name, return as-is
      return columnName;
    }
  }
  columnName = property;
  if (typeof this.dbName === 'function') {
    columnName = this.dbName(columnName);
  }
  return columnName;
};

[ssh24]

Why do we need change this?

[darknos]

because we have a way how to find column name by property name but not vise versa.

I explained it here #250 (comment)

[ssh24]

@slnode ok to test

[ssh24]

@slnode test please

[ssh24]

@darknos Thank you for taking the initiative and submitting this patch.