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.

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

Replacing PUT with PATCH #39

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Replacing PUT with PATCH #39

wants to merge 5 commits into from

Conversation

ZhangTerrence
Copy link
Contributor

Replaced PUT with PATCH and so upsert dtos now support partial updates. In the case where a collection is being updated, all the existing relations in that collection are disassociated and then the only the ids given in the new collection are added back.

@@ -21,7 +21,7 @@ public class CostOptionDTO {
private LocalDate validTo;
private String option;
private String currency;
private int amount;
private Integer amount;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need Integer? Integer creates and Integer object, which uses more memory. Why no use the primitive int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

int is a value type so its default initialized to 0 instead of null

src/main/java/com/sarapis/orservice/entity/Program.java Outdated Show resolved Hide resolved
@@ -90,23 +90,28 @@ public class Location {

@PreRemove
Copy link
Member

Choose a reason for hiding this comment

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

What is this @PreRemove doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's setting the opposite side of the bidirectional relationship to null when the entity is delete. I think this is necessary since the relation is bidirectional.

@miguel-merlin
Copy link
Member

@ZhangTerrence Added some comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants