Skip to content
5 changes: 3 additions & 2 deletions tests/browser/test_admin_project_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ def test_audit_log_created_on_staff_user_access(
# Login as staff user
self.perform_login(driver, "admin", TEST_PASSWORD, wait)

# Navigate to owner's project
# Navigate to owner's project; the pk URL redirects to the canonical
# full_id URL, which serves the page and creates the single audit log
driver.get(f"{self.live_server_url}/projects/{project.pk}/")

# Verify audit log created
Expand All @@ -298,7 +299,7 @@ def test_audit_log_created_on_staff_user_access(

log = logs.first()
assert log.action == ProjectAccessLog.Action.VIEW
assert log.view_name == "ProjectDetailView"
assert log.view_name == "ProjectDetailByFullIdView"

def test_regular_user_denied_access_not_logged(self, driver, owner, project, wait):
"""Test that regular user denied access is NOT logged.
Expand Down
6 changes: 2 additions & 4 deletions wafer_space/notifications/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,8 @@ def mark_notification_read(request, notification_id):
with contextlib.suppress(Exception):
# For ProjectFile, redirect to the project detail page
if hasattr(notification.content_object, "project"):
return redirect(
"projects:detail",
pk=notification.content_object.project.pk,
)
project = notification.content_object.project
return redirect(project.get_absolute_url())
Comment on lines 74 to +78

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline wafer_space/notifications/views.py \
  --match mark_notification_read --view expanded
rg -n -C3 'GenericForeignKey|content_object|related.*project' wafer_space

Repository: wafer-space/platform.wafer.space

Length of output: 14052


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,140p' wafer_space/notifications/views.py
printf '\n---\n'
rg -n "def get_absolute_url|class ProjectFile|class Project" wafer_space/projects -n -C 3

Repository: wafer-space/platform.wafer.space

Length of output: 18446


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '359,430p' wafer_space/projects/models.py
printf '\n---\n'
sed -n '762,860p' wafer_space/projects/models.py
printf '\n---\n'
python3 - <<'PY'
from pathlib import Path
text = Path("wafer_space/projects/models.py").read_text()
for needle in ["class ProjectFile", "project = models.ForeignKey", "get_absolute_url(self)"]:
    print(f"## {needle}")
    idx = text.find(needle)
    print(idx)
PY

Repository: wafer-space/platform.wafer.space

Length of output: 6365


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 3 "content_object\s*=|Notification\(|notify|ProjectFile" wafer_space/notifications wafer_space/projects wafer_space/shuttles

Repository: wafer-space/platform.wafer.space

Length of output: 50389


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 2 "content_object\s*=|Notification\.objects\.create|NotificationFactory|notification.*ProjectFile|ProjectFileFactory" wafer_space/notifications wafer_space/projects --glob '!**/migrations/**' --glob '!**/docs/**'

Repository: wafer-space/platform.wafer.space

Length of output: 25746


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,180p' wafer_space/notifications/services.py
printf '\n---\n'
sed -n '1,220p' wafer_space/notifications/tests/test_views.py

Repository: wafer-space/platform.wafer.space

Length of output: 4374


Remove the blanket exception suppression in the notification redirect

contextlib.suppress(Exception) here hides bugs in hasattr(...) and project.get_absolute_url() and always falls back to the list on unexpected failures. Catch only the specific relation/URL exceptions you expect, or drop the wrapper entirely.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wafer_space/notifications/views.py` around lines 74 - 78, Remove the blanket
contextlib.suppress(Exception) wrapper around the notification redirect logic.
In the notification view, either let unexpected failures from hasattr, the
project relation, or project.get_absolute_url propagate, or catch only the
specific expected relation/URL exceptions while preserving the redirect for
valid ProjectFile notifications and the existing fallback behavior.

Source: Coding guidelines


# Default: redirect back to notifications list
return redirect("notifications:list")
Expand Down
4 changes: 3 additions & 1 deletion wafer_space/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,9 @@ def clean_project_id(self):
msg = "Project ID is required"
raise ValidationError(msg)

if not project_id.isalnum():
# ASCII check matters: .upper() leaves "àbcd" as "ÀBCD", which passes
# .isalnum() but has no place in the ASCII-only canonical project URL.
if not (project_id.isascii() and project_id.isalnum()):
msg = "Project ID must be alphanumeric (A-Z, 0-9)"
raise ValidationError(msg)

Expand Down
28 changes: 26 additions & 2 deletions wafer_space/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.core.validators import FileExtensionValidator
from django.db import models
from django.db import transaction
from django.urls import reverse
from django.utils import timezone
from django.utils.formats import date_format
from django.utils.functional import cached_property
Expand Down Expand Up @@ -50,11 +51,18 @@ class CheckExecutionContext:


def validate_project_id(value: str) -> None:
"""Validate project ID is 4 alphanumeric uppercase characters."""
"""Validate project ID is 4 alphanumeric uppercase ASCII characters.

Uses an explicit ASCII check rather than ``str.isalnum()`` alone, which
also accepts non-ASCII alphanumerics (e.g. "ÀBCD", which is unchanged by
``str.upper()`` and so passes the uppercase check too). Those cannot be
reversed into the ASCII-only canonical project URL, so accepting one here
would store a project whose every link raises NoReverseMatch.
"""
if len(value) != PROJECT_ID_LENGTH:
msg = "Project ID must be exactly 4 characters"
raise ValidationError(msg)
if not value.isalnum():
if not (value.isascii() and value.isalnum()):
msg = "Project ID must be alphanumeric (A-Z, 0-9)"
raise ValidationError(msg)
# Check that any letters present are uppercase (digits are okay)
Expand Down Expand Up @@ -349,6 +357,22 @@ def save(self, *args, **kwargs):
"proprietary_terms_url": self.proprietary_terms_url,
}

def get_absolute_url(self) -> str:
"""Get URL for the project's detail view.

Prefers the canonical manufacturing-ID URL, falling back to the pk
URL for projects not yet on a shuttle (which have no full_id).

Returns:
str: URL for project detail.

"""
if self.full_id:
return reverse(
"projects:detail_by_full_id", kwargs={"full_id": self.full_id}
)
return reverse("projects:detail", kwargs={"pk": self.pk})

def clean(self):
"""Validate model, including core field immutability.

Expand Down
15 changes: 15 additions & 0 deletions wafer_space/projects/tests/test_shuttle_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,21 @@ def test_full_id_without_both_returns_empty(self):
project = Project.objects.create(user=self.user, name="Test Project")
assert project.full_id == ""

def test_get_absolute_url_uses_full_id_when_on_a_shuttle(self):
"""The canonical URL is the manufacturing-ID one once assigned."""
project = Project.objects.create(
user=self.user, name="Test Project", shuttle=self.shuttle, project_id="ABCD"
)
assert project.get_absolute_url() == "/projects/G891ABCD/"

def test_get_absolute_url_falls_back_to_pk_without_full_id(self):
"""Projects with no shuttle have no full_id, so the pk URL stands in."""
project = Project.objects.create(
user=self.user, name="Test Project", project_id="ABCD"
)
assert project.full_id == ""
assert project.get_absolute_url() == f"/projects/{project.pk}/"

def test_shuttle_run_display_with_shuttle(self):
"""Test shuttle_run_display property returns formatted string."""
project = Project.objects.create(
Expand Down
12 changes: 12 additions & 0 deletions wafer_space/projects/tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ def test_invalid_project_id_special_characters(self):
validate_project_id("AB-D")
assert "alphanumeric" in str(exc_info.value)

def test_invalid_project_id_non_ascii_alphanumeric(self):
"""Test that non-ASCII alphanumerics are invalid.

"ÀBCD" satisfies str.isalnum() and is left unchanged by str.upper(),
so it passed both the alphanumeric and uppercase checks. It has no
place in the ASCII-only canonical project URL: storing one made every
page that links the project raise NoReverseMatch.
"""
with pytest.raises(ValidationError) as exc_info:
validate_project_id("ÀBCD")
assert "alphanumeric" in str(exc_info.value)

def test_invalid_project_id_spaces(self):
"""Test that spaces are invalid."""
with pytest.raises(ValidationError) as exc_info:
Expand Down
Loading
Loading