Skip to content

Commit

Permalink
Fix slug generation (#207)
Browse files Browse the repository at this point in the history
* Fix slug generation

Slugs are intended to be human-readable and unique for an artifact.
However when a user submitted multiple versions they often got
duplicated slugs. This was because our check for an existing version was
based on the artifacts date, not the version date.

We attempted to fix in #133, but the issue persisted. I just happened to
notice this filter code seemed wrong.

* Fix black formatting
  • Loading branch information
Mark-Powers authored Nov 11, 2024
1 parent f5c69cb commit 672ff18
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 10 deletions.
2 changes: 1 addition & 1 deletion trovi/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ def generate_slug(instance: "ArtifactVersion", created: bool = False, **_):
with transaction.atomic():
if instance.artifact:
versions_today_query = instance.artifact.versions.filter(
artifact__created_at__date=instance.created_at.date(),
created_at__date=instance.created_at.date(),
).select_for_update()
versions_today = versions_today_query.exclude(
slug__exact=""
Expand Down
6 changes: 4 additions & 2 deletions trovi/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from pathlib import Path

# Build paths inside the project like this: BASE_DIR / 'subdir'.
import sys
from urllib.parse import urlparse

BASE_DIR = Path(__file__).resolve().parent.parent
Expand Down Expand Up @@ -124,7 +125,7 @@
# Plugins
"rest_framework",
"drf_spectacular",
'corsheaders',
"corsheaders",
# Trovi
"trovi.apps.TroviConfig",
"trovi.api.apps.ApiConfig",
Expand Down Expand Up @@ -191,8 +192,9 @@

# Database
# https://docs.djangoproject.com/en/3.2/ref/settings/#databases
TESTING = "test" in sys.argv or "test_coverage" in sys.argv

if os.getenv("DB_ENGINE"):
if not TESTING and os.getenv("DB_ENGINE"):
DATABASES = {
"default": {
"ENGINE": os.getenv("DB_ENGINE"),
Expand Down
1 change: 1 addition & 0 deletions util/sql_format.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Pretty-Print SQL queries for console output."""

from logging import Formatter

from django.conf import settings
Expand Down
19 changes: 12 additions & 7 deletions util/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Not meant to be robust, and mostly unvalidated, but helpful to avoid
having to reference dicts with strings repeatedly in tests.
"""

import datetime
import logging
import os
Expand Down Expand Up @@ -160,6 +161,8 @@ def fake_tag() -> str:
is_reproducible=True,
repro_access_hours=3,
)
artifact_don_quixote.created_at -= datetime.timedelta(days=2)

version_don_quixote_1 = ArtifactVersion(
artifact=artifact_don_quixote,
contents_urn=f"urn:trovi:contents:chameleon:{uuid4()}",
Expand Down Expand Up @@ -270,9 +273,9 @@ def generate_fake_artifact() -> list[models.Model]:
ArtifactEvent(
artifact_version=random.choice(artifact_versions),
event_type=random.choice(ArtifactEvent.EventType.values),
event_origin=None
if fake.boolean(chance_of_getting_true=90)
else fake_user_urn(),
event_origin=(
None if fake.boolean(chance_of_getting_true=90) else fake_user_urn()
),
)
for _ in range(random.randint(0, 40))
]
Expand All @@ -282,10 +285,12 @@ def _verified() -> dict[str, Union[bool, Optional[datetime.datetime]]]:
verified = fake.boolean(chance_of_getting_true=20)
return {
"verified": verified,
"verified_at": None
if not verified
else fake.date_time_between(
datetime.date.min, datetime.date.max, timezone.utc
"verified_at": (
None
if not verified
else fake.date_time_between(
datetime.date.min, datetime.date.max, timezone.utc
)
),
}

Expand Down

0 comments on commit 672ff18

Please sign in to comment.