Skip to content

Commit

Permalink
fix: skip undefined values in merge
Browse files Browse the repository at this point in the history
  • Loading branch information
adamcikado committed Nov 22, 2024
1 parent ade635f commit 10d546b
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 46 deletions.
98 changes: 52 additions & 46 deletions src/orm/base_model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1707,61 +1707,67 @@ class BaseModelImpl implements LucidRow {
merge(values: any, allowExtraProperties: boolean = false): this {
const Model = this.constructor as typeof BaseModel

if (!isObject(values)) {
return this
}

/**
* Merge values with the attributes
*/
if (isObject(values)) {
Object.keys(values).forEach((key) => {
const value = values[key]
Object.keys(values).forEach((key) => {
const value = values[key]

/**
* Set as column
*/
if (Model.$hasColumn(key)) {
;(this as any)[key] = value
return
}
if (value === undefined) {
return
}

/**
* Resolve the attribute name from the column names. Since people
* usually define the column names directly as well by
* accepting them directly from the API.
*/
const attributeName = Model.$keys.columnsToAttributes.get(key)
if (attributeName) {
;(this as any)[attributeName] = value
return
}
/**
* Set as column
*/
if (Model.$hasColumn(key)) {
;(this as any)[key] = value
return
}

/**
* If key is defined as a relation, then ignore it, since one
* must pass a qualified model to `this.$setRelated()`
*/
if (Model.$relationsDefinitions.has(key)) {
return
}
/**
* Resolve the attribute name from the column names. Since people
* usually define the column names directly as well by
* accepting them directly from the API.
*/
const attributeName = Model.$keys.columnsToAttributes.get(key)
if (attributeName) {
;(this as any)[attributeName] = value
return
}

/**
* If the property already exists on the model, then set it
* as it is vs defining it as an extra property
*/
if (this.hasOwnProperty(key)) {
;(this as any)[key] = value
return
}
/**
* If key is defined as a relation, then ignore it, since one
* must pass a qualified model to `this.$setRelated()`
*/
if (Model.$relationsDefinitions.has(key)) {
return
}

/**
* Raise error when not instructed to ignore non-existing properties.
*/
if (!allowExtraProperties) {
throw new Error(
`Cannot define "${key}" on "${Model.name}" model, since it is not defined as a model property`
)
}
/**
* If the property already exists on the model, then set it
* as it is vs defining it as an extra property
*/
if (this.hasOwnProperty(key)) {
;(this as any)[key] = value
return
}

this.$extras[key] = value
})
}
/**
* Raise error when not instructed to ignore non-existing properties.
*/
if (!allowExtraProperties) {
throw new Error(
`Cannot define "${key}" on "${Model.name}" model, since it is not defined as a model property`
)
}

this.$extras[key] = value
})

return this
}
Expand Down
31 changes: 31 additions & 0 deletions test/orm/base_model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3006,6 +3006,37 @@ test.group('BaseModel | fill/merge', (group) => {
assert.deepEqual(user.$attributes, { username: 'virk', age: 22 })
assert.equal(user.foo, 'bar')
})

test('ensure merge skips undefined values', async ({ fs, assert }) => {
const app = new AppFactory().create(fs.baseUrl, () => {})
await app.init()
const adapter = new FakeAdapter()

const BaseModel = getBaseModel(adapter)

class User extends BaseModel {
@column()
declare username: string

@column()
declare email: string
}

const email = 'virk@adonisjs.com'

const user = new User()
user.merge({
username: 'virk',
email,
})

user.merge({
username: 'nikk',
email: undefined,
})

assert.equal(user.email, email)
})
})

test.group('Base | apdater', (group) => {
Expand Down

0 comments on commit 10d546b

Please sign in to comment.