Skip to content

Commit

Permalink
fix: updating with only update: false columns shouldn't trigger @Up…
Browse files Browse the repository at this point in the history
…dateDateColumn column updation

* test: add test case for 8394 WIP
* test(8393): add entity for test case WIP
* test(8393): remove unrelated text
* test(8393): fix compilation errors
* test(8393): use chai syntax
* test(8393); remove unnecessary logs and update test name
* test(8393): remove unnecessary logs and update test name
* fix(update): disallowing to pass empty object / only readonly props to update

Closes #8394

* fix(update): disallowing to pass object with only readonly props to update

Closes #8394

* refactor(update): correct code review requested changed

Closes #8394

* ci: trigger build
  • Loading branch information
rluvaton committed Dec 11, 2021
1 parent e52b26c commit 2834729
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/query-builder/UpdateQueryBuilder.ts
Expand Up @@ -441,11 +441,13 @@ export class UpdateQueryBuilder<Entity> extends QueryBuilder<Entity> implements
});
});

if (metadata.versionColumn && updatedColumns.indexOf(metadata.versionColumn) === -1)
updateColumnAndValues.push(this.escape(metadata.versionColumn.databaseName) + " = " + this.escape(metadata.versionColumn.databaseName) + " + 1");
if (metadata.updateDateColumn && updatedColumns.indexOf(metadata.updateDateColumn) === -1)
updateColumnAndValues.push(this.escape(metadata.updateDateColumn.databaseName) + " = CURRENT_TIMESTAMP"); // todo: fix issue with CURRENT_TIMESTAMP(6) being used, can "DEFAULT" be used?!

// Don't allow calling update only with columns that are `update: false`
if (updateColumnAndValues.length > 0 || Object.keys(valuesSet).length === 0) {
if (metadata.versionColumn && updatedColumns.indexOf(metadata.versionColumn) === -1)
updateColumnAndValues.push(this.escape(metadata.versionColumn.databaseName) + " = " + this.escape(metadata.versionColumn.databaseName) + " + 1");
if (metadata.updateDateColumn && updatedColumns.indexOf(metadata.updateDateColumn) === -1)
updateColumnAndValues.push(this.escape(metadata.updateDateColumn.databaseName) + " = CURRENT_TIMESTAMP"); // todo: fix issue with CURRENT_TIMESTAMP(6) being used, can "DEFAULT" be used?!
}
} else {
Object.keys(valuesSet).map(key => {
let value = valuesSet[key];
Expand Down
25 changes: 25 additions & 0 deletions test/github-issues/8393/entity/Post.ts
@@ -0,0 +1,25 @@
import {
Column,
Entity,
UpdateDateColumn,
PrimaryGeneratedColumn,
} from "../../../../src";

@Entity()
export class Post {
@PrimaryGeneratedColumn()
id: number;

// Extra, not required column just for the example
@Column()
title: string;

@UpdateDateColumn()
lastUpdated: Date;

@Column({
// Forcing this column to only be written on insert time
update: false
})
readOnlyColumn: number;
}
55 changes: 55 additions & 0 deletions test/github-issues/8393/issue-8393.ts
@@ -0,0 +1,55 @@
import "reflect-metadata";
import {
createTestingConnections,
closeTestingConnections,
reloadTestingDatabases
} from "../../utils/test-utils";
import { Connection, UpdateValuesMissingError } from "../../../src/";
import { expect } from "chai";
import { Post } from "./entity/Post";

describe("github issues > #8393 When trying to update `update: false` column with `@UpdateDateColumn` the update column is updated", () => {
let connections: Connection[];
before(
async () =>
(connections = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
schemaCreate: true,
dropSchema: true,
}))
);
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should not update the @UpdateDateColumn column when trying to update un-updatable column", () =>
Promise.all(
connections.map(async (connection) => {
const post = new Post();
post.title = "Control flow based type analysis";
post.readOnlyColumn = 1;

await connection.manager.save(post);

const updateResultPromise = connection.manager.update(Post, post.id, {
// Make a change to read only column
readOnlyColumn: 2,
});

await expect(updateResultPromise).to.be.rejectedWith(UpdateValuesMissingError);

const updatedPost = await connection.manager.findOne(
Post,
post.id
);

expect(updatedPost).to.be.an("object");

expect(post.readOnlyColumn).to.be.equal(
updatedPost!.readOnlyColumn
);

// Gonna be false
expect(post.lastUpdated.toString()).to.be.eql(updatedPost!.lastUpdated.toString());
})
));
});

0 comments on commit 2834729

Please sign in to comment.