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

Broken XML_ID uniqueness validation for tests #1247

Open
Mathis-Z opened this issue Feb 17, 2024 · 3 comments
Open

Broken XML_ID uniqueness validation for tests #1247

Mathis-Z opened this issue Feb 17, 2024 · 3 comments
Labels

Comments

@Mathis-Z
Copy link
Contributor

Mathis-Z commented Feb 17, 2024

There is a validation for tests to make sure there cannot be two tests with the same XML_ID for the same task. However this validation appears to be broken because I can just create tasks with tests that have the same XML_ID.
I guess Rails performs the validation of both tests before they are saved to the database and each validation check does not find the other test in the database so they just pass and get written to database successfully.
We should check if there are similar issues in other models.

EDIT: You can create a task like that but you cannot edit it afterwards because then the tests are in the database and the validation fails.

@Mathis-Z Mathis-Z added the bug label Feb 17, 2024
@MrSerth
Copy link
Member

MrSerth commented Feb 17, 2024

Indeed, I can confirm that this is a bug in the current implementation. We also noticed that when implementing contributions, but just added corresponding comments in the fs_task_contrib branch, yet without a ticket nor fixing it. From the tests I've made, I think this is an issue for several models, not just Test.

@kkoehn
Copy link
Collaborator

kkoehn commented Feb 19, 2024

It seems to be a bug in Rails. The corresponding issue (rails/#20676) was opened in 2015. There are multiple workarounds:

As far as I understand the issue, it only affects objects that are saved through nested_attributes. Since we are only using nested_attributes for ModelSolutions, Tests and TaskFiles and the TaskFiles need a specific validator anyway, we only need to find a solution for Tests and ModelSolutions.

I suggest we add the necessary database constraints and a validator that we can use for both models.

@Mathis-Z
Copy link
Contributor Author

Yes, that sounds good.

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

No branches or pull requests

3 participants