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.

Sign up for GitHub

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

fix: previousChanges should check instance is new record or not while specific attributes' values were undefined #276

Merged
merged 3 commits into from
Feb 21, 2022
Merged

fix: previousChanges should check instance is new record or not while specific attributes' values were undefined #276

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c48617
fix: previousChanges should check instance is new record or not while…
Feb 19, 2022
81be1c8
Merge branch 'master' into fix/previous_changes_is_new_record
cyjake Feb 21, 2022
e5f5870
fix: take attribute.cast null-like value to null
Feb 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/bone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,15 +371,15 @@ class Bone {
*/
previousChanges(name) {
if (name != null) {
if (this.#rawUnset.has(name) || this.#rawPrevious[name] === undefined || !this.hasAttribute(name)) return {};
if (this.#rawUnset.has(name) || (this.#rawPrevious[name] === undefined && this.isNewRecord) || !this.hasAttribute(name)) return {};
Copy link
Owner

@cyjake cyjake Feb 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this.isNewRecord covers this.#rawPrevious[name] === undefined already, can we give it a try?

btw, this line is a bit too much now, I believe a multiple line if block is necessary once the column width exceeds some threshold like 90 or 120

const value = this.attribute(name);
const valueWas = this.#rawPrevious[name] == null ? null : this.#rawPrevious[name];
if (util.isDeepStrictEqual(value, valueWas)) return {};
return { [name]: [ valueWas, value ] };
}
const result = {};
for (const attrKey of Object.keys(this.constructor.attributes)) {
if (this.#rawUnset.has(attrKey) || this.#rawPrevious[attrKey] === undefined) continue;
if (this.#rawUnset.has(attrKey) || (this.#rawPrevious[attrKey] === undefined && this.isNewRecord)) continue;
const value = this.attribute(attrKey);
const valueWas = this.#rawPrevious[attrKey] == null ? null : this.#rawPrevious[attrKey];
if (!util.isDeepStrictEqual(value, valueWas)) result[attrKey] = [ valueWas, value ];
Expand Down
6 changes: 5 additions & 1 deletion test/integration/suite/basics.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,12 @@ describe('=> Basic', () => {
assert.deepEqual(post.previousChanges('title'), {});
await post.save();
assert.deepEqual(post.previousChanges('title'), {});
assert.deepEqual(post.previousChanges('authorId'), { });
post.title = 'MHW';
post.authorId = 100;
await post.save();
assert.deepEqual(post.previousChanges('title'), { title: [ 'Untitled', 'MHW' ] });
assert.deepEqual(post.previousChanges('authorId'), { authorId: [ null, 100 ] });
// should return {} if key does not exist
assert.deepEqual(post.previousChanges('notExisted'), {});
});
Expand All @@ -208,10 +211,11 @@ describe('=> Basic', () => {
await post.save();
assert.deepEqual(post.previousChanges(), {});
post.title = 'MHW';
post.authorId = 100;
const prevUpdatedAt = post.updatedAt;
await sleep(10);
await post.save();
assert.deepEqual(post.previousChanges(), { title: [ 'Untitled', 'MHW' ], updatedAt: [ prevUpdatedAt, post.updatedAt ] });
assert.deepEqual(post.previousChanges(), { authorId: [ null, 100 ], title: [ 'Untitled', 'MHW' ], updatedAt: [ prevUpdatedAt, post.updatedAt ] });
});

it('Bone.changes(key): raw VS rawSaved', async function () {
Expand Down