♻️(back) move check constraint on filename in save method

The check constraint on the filename can be moved in the save method. A
check constraint will execute a sql query and in this case it is not
needed, we have all the element needed to do this check in the python
code and not in the database. This will reduce the number queries on
every save.
This commit is contained in:
Manuel Raynaud
2025-06-23 11:40:34 +02:00
parent 66d43c946c
commit 2e8077f7e8
5 changed files with 47 additions and 14 deletions
@@ -0,0 +1,21 @@
# Generated by Django 5.2.3 on 2025-06-23 09:22
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('core', '0007_item_hard_deleted_at'),
]
operations = [
migrations.AlterModelOptions(
name='item',
options={'ordering': ('created_at',), 'verbose_name': 'Item', 'verbose_name_plural': 'Items'},
),
migrations.RemoveConstraint(
model_name='item',
name='check_filename_set_for_files',
),
]
+22 -10
View File
@@ -556,16 +556,7 @@ class Item(TreeModel, BaseModel):
| models.Q(deleted_at=models.F("ancestors_deleted_at"))
),
name="check_deleted_at_matches_ancestors_deleted_at_when_set",
),
models.CheckConstraint(
condition=models.Q(
models.Q(type=ItemTypeChoices.FILE, filename__isnull=False)
| models.Q(
~models.Q(type=ItemTypeChoices.FILE), filename__isnull=True
)
),
name="check_filename_set_for_files",
),
)
]
indexes = [
GistIndex(fields=["path"]),
@@ -576,6 +567,27 @@ class Item(TreeModel, BaseModel):
def save(self, *args, **kwargs):
"""Set the upload state to pending if it's the first save and it's a file"""
# Validate filename requirements based on item type
if self.type == ItemTypeChoices.FILE:
if self.filename is None:
raise ValidationError(
{
"filename": ValidationError(
_("Filename is required for files."),
code="item_filename_required_for_files",
)
}
)
elif self.filename is not None:
raise ValidationError(
{
"filename": ValidationError(
_("Filename is only allowed for files."),
code="item_filename_only_allowed_for_files",
)
}
)
if self.created_at is None and self.type == ItemTypeChoices.FILE:
self.upload_state = ItemUploadStateChoices.PENDING
@@ -500,7 +500,7 @@ def test_authentication_session_tokens(
status=200,
)
with django_assert_num_queries(30):
with django_assert_num_queries(27):
user = klass.authenticate(
request,
code="test-code",
@@ -143,7 +143,7 @@ def test_models_invitationd_new_user_filter_expired_invitations():
).exists()
@pytest.mark.parametrize("num_invitations, num_queries", [(0, 27), (1, 31), (20, 31)])
@pytest.mark.parametrize("num_invitations, num_queries", [(0, 24), (1, 28), (20, 28)])
def test_models_invitations_new_userd_user_creation_constant_num_queries(
django_assert_num_queries, num_invitations, num_queries
):
+2 -2
View File
@@ -699,7 +699,7 @@ def test_models_items_creating_file_without_filename_should_fail():
factories.ItemFactory(type=models.ItemTypeChoices.FILE, filename=None)
assert exc_info.value.message_dict == {
"__all__": ["Constraint “check_filename_set_for_files” is violated."]
"filename": ["Filename is required for files."]
}
@@ -717,7 +717,7 @@ def test_models_items_creating_non_file_with_filename_should_fail(item_type):
factories.ItemFactory(type=item_type, filename="file.txt")
assert exc_info.value.message_dict == {
"__all__": ["Constraint “check_filename_set_for_files” is violated."]
"filename": ["Filename is only allowed for files."]
}