-
Notifications
You must be signed in to change notification settings - Fork 2
Fix project name length validation #43
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
base: main
Are you sure you want to change the base?
Conversation
Fixes #27 The solution was generated using github copilot workspace. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/accso/SecureCheckPlus/issues/27?shareId=XXXX-XXXX-XXXX-XXXX).
|
The solution generated by copilot was pretty much entirely wrong, so I reworked it and now it should be properly working. |
|
|
||
| projectId = serializers.CharField(source="project_id", read_only=True) | ||
| projectName = serializers.CharField(source="project_name", allow_blank=True, validators=[MaxLengthValidator(255)]) | ||
| projectName = serializers.CharField(source="project_name", allow_blank=True, max_length=255) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this length relate to the maximum length of 25 characters declared in backend/analyzer/models.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I haven't seen the constraint in models.py. I think that the check in project_serializer.py is then redundant and can be removed without losing validation. Is that correct? Or should we lave it, but reduce it to 25?
|
I’m not quite sure, if the constraint in the model backend automatically propagates to the frontend. My guess would be that it does not. In any case the current length in the data model is too small. There’s actually another issue for the increase of the lengths. Note that we have make sure to use the model update mechanism of Django for this so that existing instances will be correctly patched model-wise.
See #26
|
|
This should be merged after 26-extended-field-lengths to avoid issues. |
Fixes #27
The solution was generated using github copilot workspace.
For more details, open the Copilot Workspace session.