Remove unnecessary write-time validating URLTextField #4322
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #4320 by @AetherUnbound
Description
As discussed in the PR linked by the issue and the issue itself, the
URLTextField
is wholly unnecessary. The usage ofURLField
on these model fields was already superfluous: the only featureURLField
adds overCharField
is write-time validation to check that strings "look like" URLs. However, the fields are literally never written to using Django ORM in application code, the data refresh process writes directly to the database in raw SQL inserts and never refers to Django's models at all.In the related PR, we added
URLTextField
to maintain the consistency with the original implementation. But really we can just remove it. There's no reason to keep this around. All code is a liability, and if we can remove it (if it is unused) we should. In this case, removing it is so trivial and of literally zero consequence for the application because the extra code and routinesURL[Text]Field
added were dead code. There is no reason not to remove it, and it is in fact prudent to do so.Testing Instructions
CI should pass. There are no new migrations. Changing
URLTextField
toTextField
does not result in any changes to the SQL content of that migration. You can test this out by checking out main and runningjust down -v && just api/up && just api/init
, then check out this branch and runjust logs web
and confirm that there is no issue with migrations. Additionally, make a request to the images and audio endpoints which will likewise confirm there is no meaningful change to the way the application reads these fields. Compare below to the SQL in the original PR (#4315):Checklist
Update index.md
).main
) or a parent feature branch.just catalog/generate-docs
for catalogPRs) or the media properties generator (
just catalog/generate-docs media-props
for the catalog or
just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin