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
API V3 Subproject Creation Bug fix #6275
Conversation
Changes look good. I'm leaving some suggestions to have more explicit tests. Also, I'm pushing some tests for the UI form, I was in the process of fixing this when I saw this PR p:
Oh, and looks like all validation errors from the subproject serializer are missing i18n, can you add it?
Co-Authored-By: Santos Gallegos <santos_g@outlook.com>
Co-Authored-By: Santos Gallegos <santos_g@outlook.com>
Sure thanks :)
Okay :) |
I was almost sure that I was going to make a mistake with this logic. I'm happy that you solved the issue. Thanks!
When working on this, I asked myself if it wouldn't be better to move the logic outside the Form and share it with the Serializer. After the bug that I introduced, I'd say it's clear that we have to do that instead of replicating the same logic. I want a function is_valid_as_subproject(project, subproject)
raising different customized exceptions that we can handle properly on the Form and Serializer.
I think we can have 2 methods that will check Should I update this PR with that or Its alright for now? But not sure how to raise different customized exceptions that we can handle properly on the Form and Serializer without any hack. like passing the validation error class |
Two methods are more explicit to me.
I think this refactor can be done in another PR. Do you want to work on that as well? |
@humitos Sure! |
fixes: #6271