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

Move output setting to device table #974

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

jasperkamerling
Copy link
Contributor

The database doesn't match the JPA model that was created. This may cause issues when deleting ssld's or devices.
The real solution would be to refactor the database but this is a first step to have the jpa model match the database.

Schema

Signed-off-by: Jasper Kamerling <[email protected]>
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Comment on lines +103 to +111
/**
* The output settings are only used in the ssld table but in the database it is linked to the
* Device Table.
*/
@LazyCollection(LazyCollectionOption.FALSE)
@ElementCollection()
@CollectionTable(name = "device_output_setting", joinColumns = @JoinColumn(name = "device_id"))
protected List<DeviceOutputSetting> outputSettings = new ArrayList<>();

Copy link
Member

@smvdheijden smvdheijden Jan 30, 2023

Choose a reason for hiding this comment

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

I prefer not to have an SSLD specific field in the generic device Entitiy. It would probably be better to change the relationship in the DB and change the foreign key to point to the ssld id (which should effectively be the same as the device id due to the inheritance relation between them)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True that would be better but that would be a big change in the data model.
This just fixes the application issues with the data model.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think a big change in the data model is needed. Setting ON DELETE CASCADE will probably also work?

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