Conversation
Juwang110
left a comment
There was a problem hiding this comment.
Overall logic looks good, just need to refactor for a non-nullable assignee
| public async up(queryRunner: QueryRunner): Promise<void> { | ||
| await queryRunner.query(` | ||
| ALTER TABLE orders | ||
| ADD COLUMN assignee_id INT, |
There was a problem hiding this comment.
I think that asignees should not be able to be nullable
There was a problem hiding this comment.
since there isn't an order creation workflow yet, i think it might be best to keep it nullable for now and change it later once that's implemented. the existing orders currently have null assignees so it might break things
There was a problem hiding this comment.
@Yurika-Kan what do you think? I don't think it will break anything since the null assignees are new here but will defer to your judgement
| export class AddAssigneeToOrders1773009000618 implements MigrationInterface { | ||
| public async up(queryRunner: QueryRunner): Promise<void> { | ||
| await queryRunner.query(` | ||
| ALTER TABLE orders |
There was a problem hiding this comment.
Also here we can edit the dummy data to add ids for the asignee for each order
| }) | ||
| shippingCost!: number | null; | ||
|
|
||
| @ManyToOne(() => User, { nullable: true }) |
|
|
||
| @ManyToOne(() => User, { nullable: true }) | ||
| @JoinColumn({ name: 'assignee_id', referencedColumnName: 'id' }) | ||
| assignee!: User | null; |
| nullable: true, | ||
| }) | ||
| shippingCost!: number | null; | ||
|
|
There was a problem hiding this comment.
I see the relation but we should add an explicity assignee_id column definition to this entity since we add a table column
| id: number; | ||
| firstName: string; | ||
| lastName: string; | ||
| } | null; |
| {order.assignee?.lastName.charAt(0).toUpperCase()} | ||
| </Box> | ||
| ) : ( | ||
| <Box>No Assignees</Box> |
ℹ️ Issue
Closes 135
📝 Description
✔️ Verification
UI testing