Skip to content
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

[8664] auto searchprofile names #5928

Merged
merged 1 commit into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions changelog/8664.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
### Added

- add a new field `number` to SearchProfile which indexes the search profiles per
user
- add auto-naming to SearchProfiles for unnamed search profiles

### Changed

- make `name` field on SearchProfile optional as users can only change it after
creation and it makes accommodating the auto-naming scheme easier
3 changes: 2 additions & 1 deletion meinberlin/apps/kiezradar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@


class SearchProfileAdmin(admin.ModelAdmin):
list_display = ("id", "name", "user")
list_display = ("id", "name", "user", "number")
readonly_fields = ("number",)
list_filter = (
"name",
"status",
Expand Down
31 changes: 31 additions & 0 deletions meinberlin/apps/kiezradar/migrations/0006_searchprofile_number.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 4.2.11 on 2025-01-09 15:25

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("meinberlin_kiezradar", "0005_populate_project_status"),
]

operations = [
migrations.AddField(
model_name="searchprofile",
name="number",
field=models.PositiveSmallIntegerField(null=True),
),
migrations.AddIndex(
model_name="searchprofile",
index=models.Index(models.F("number"), name="searchprofile_number_idx"),
),
migrations.AlterModelOptions(
name="searchprofile",
options={"ordering": ["number"]},
),
migrations.AlterField(
model_name="searchprofile",
name="name",
field=models.CharField(max_length=255, null=True),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Generated by Django 4.2.11 on 2025-01-09 15:26

from django.db import migrations, models


def add_number_to_search_profiles(apps, schema_editor):
SearchProfile = apps.get_model("meinberlin_kiezradar", "SearchProfile")
for sp in SearchProfile.objects.all():
if sp.number is None:
latest = None
try:
latest = (
SearchProfile.objects.filter(number__isnull=False)
.filter(user=sp.user)
.latest("number")
)
except SearchProfile.DoesNotExist:
pass
goapunk marked this conversation as resolved.
Show resolved Hide resolved
sp.number = latest.number + 1 if latest else 1
sp.save()


class Migration(migrations.Migration):
dependencies = [
("meinberlin_kiezradar", "0006_searchprofile_number"),
]

operations = [
migrations.RunPython(add_number_to_search_profiles, migrations.RunPython.noop),
migrations.AlterField(
model_name="searchprofile",
name="number",
field=models.PositiveSmallIntegerField(),
),
migrations.AddConstraint(
model_name="searchprofile",
constraint=models.UniqueConstraint(
models.F("user"), models.F("number"), name="unique-search-profile"
),
),
]
20 changes: 18 additions & 2 deletions meinberlin/apps/kiezradar/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class SearchProfile(models.Model):
on_delete=models.CASCADE,
related_name="search_profiles",
)
name = models.CharField(max_length=255)
name = models.CharField(max_length=255, null=True)
number = models.PositiveSmallIntegerField()
description = models.TextField(blank=True, null=True)
disabled = models.BooleanField(default=False)
notification = models.BooleanField(default=False)
Expand Down Expand Up @@ -100,8 +101,23 @@ class SearchProfile(models.Model):
blank=True,
)

def save(self, update_fields=None, *args, **kwargs):
"""Custom save() to add the next unused number per user on creation"""
if self.number is None:
latest = None
try:
latest = SearchProfile.objects.filter(user=self.user).latest("number")
except SearchProfile.DoesNotExist:
pass
m4ra marked this conversation as resolved.
Show resolved Hide resolved
self.number = latest.number + 1 if latest else 1
m4ra marked this conversation as resolved.
Show resolved Hide resolved
super().save(update_fields=update_fields, *args, **kwargs)

class Meta:
ordering = ["name"]
ordering = ["number"]
constraints = [
models.UniqueConstraint("user", "number", name="unique-search-profile")
]
Copy link
Contributor

Choose a reason for hiding this comment

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

we also need an index on the number field.

indexes = [models.Index("number", name="searchprofile_number_idx")]

def __str__(self):
return f"kiezradar search profile - {self.name}, disabled {self.disabled}"
8 changes: 6 additions & 2 deletions meinberlin/apps/kiezradar/serializers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.conf import settings
from django.utils.module_loading import import_string
from django.utils.translation import gettext as _
from rest_framework import serializers

from adhocracy4.administrative_districts.models import AdministrativeDistrict
Expand Down Expand Up @@ -45,6 +46,7 @@ class Meta:
"id",
"user",
"name",
"number",
"description",
"disabled",
"notification",
Expand All @@ -57,7 +59,7 @@ class Meta:
"topics",
]

read_only_fields = ["user"]
read_only_fields = ["user", "number"]

def create(self, validated_data):
# Pop many-to-many and one-to-many fields from validated_data
Expand Down Expand Up @@ -112,6 +114,8 @@ def to_representation(self, instance):
{"id": topic.id, "code": topic.code, "name": topics_enum(topic.code).label}
for topic in instance.topics.all()
]

representation["query_text"] = instance.query.text if instance.query else ""

if not instance.name:
representation["name"] = _("Searchprofile %d") % instance.number
m4ra marked this conversation as resolved.
Show resolved Hide resolved
return representation
94 changes: 94 additions & 0 deletions tests/kiezradar/test_api_kiezradar.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def test_create_search_profile(client, user, setup_data):
data = response.json()

assert data["name"] == payload["name"]
assert data["number"] == 1
assert data["description"] == payload["description"]
assert data["disabled"] == payload["disabled"]
assert data["query"] == payload["query"]
Expand All @@ -77,6 +78,67 @@ def test_create_search_profile(client, user, setup_data):
)


@pytest.mark.django_db
def test_create_and_update_search_profile_without_name(client, user, setup_data):
"""Test creating and updating a SearchProfile without name via the API."""
client.login(email=user.email, password="password")

payload = {
"description": "A description for the filters profile.",
"disabled": False,
"status": setup_data["project_status"],
"query": setup_data["query"],
"districts": setup_data["districts"],
"topics": setup_data["topics"],
"organisation": setup_data["organisations"],
"project_types": setup_data["project_types"],
}

url = reverse("searchprofiles-list")
response = client.post(url, payload, format="json")

assert response.status_code == 201
data = response.json()

assert data["name"] == "Searchprofile 1"
assert data["number"] == 1
assert data["description"] == payload["description"]
assert data["disabled"] == payload["disabled"]
assert data["query"] == payload["query"]

# Check if the object was created in the database
search_profile = SearchProfile.objects.get(id=data["id"])
assert search_profile.name is None
assert search_profile.description == payload["description"]
assert search_profile.disabled == payload["disabled"]
assert search_profile.query.id == payload["query"]
assert (
list(search_profile.districts.values_list("id", flat=True))
== payload["districts"]
)
assert list(search_profile.status.values_list("id", flat=True)) == payload["status"]
assert list(search_profile.topics.values_list("id", flat=True)) == payload["topics"]
assert (
list(search_profile.project_types.values_list("id", flat=True))
== payload["project_types"]
)

payload = {
"name": "Test Search Profile",
}
url = reverse("searchprofiles-detail", kwargs={"pk": search_profile.id})
response = client.patch(url, data=payload, content_type="application/json")
data = response.json()

assert response.status_code == 200
assert data["name"] == payload["name"]
assert data["number"] == 1

# Check if the object was updated in the database
search_profile = SearchProfile.objects.get(id=data["id"])
assert search_profile.name == payload["name"]


@pytest.mark.django_db
def test_update_search_profile(client, user, setup_data, search_profile_factory):
"""Test updating a SearchProfile via the API."""
Expand Down Expand Up @@ -139,3 +201,35 @@ def test_update_search_profile(client, user, setup_data, search_profile_factory)
# Check if the object was updated in the database
search_profile = SearchProfile.objects.get(id=data["id"])
assert search_profile.query.text == payload["query_text"]


@pytest.mark.django_db
def test_creating_multiple_search_profiles_assign_correct_number(
client, user, setup_data
):
"""Test creating and updating a SearchProfile without name via the API."""
client.login(email=user.email, password="password")

payload = {
"description": "A description for the filters profile.",
"disabled": False,
"status": setup_data["project_status"],
"query": setup_data["query"],
"districts": setup_data["districts"],
"topics": setup_data["topics"],
"organisation": setup_data["organisations"],
"project_types": setup_data["project_types"],
}

url = reverse("searchprofiles-list")
for i in range(1, 6):
response = client.post(url, payload, format="json")
assert response.status_code == 201
data = response.json()
assert data["name"] == "Searchprofile " + str(i)
assert data["number"] == i

assert SearchProfile.objects.count() == 5
for i, search_profile in enumerate(SearchProfile.objects.all()):
assert search_profile.name is None
assert search_profile.number == i + 1
21 changes: 21 additions & 0 deletions tests/kiezradar/test_search_profile.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

from adhocracy4.projects.models import Topic
from meinberlin.apps.kiezradar.models import SearchProfile


@pytest.mark.django_db
Expand All @@ -17,6 +18,7 @@ def test_create_search_profile(
assert search_profile.districts.all().count() == 0
assert search_profile.project_types.all().count() == 0
assert search_profile.organisations.all().count() == 0
assert search_profile.number == 1

topic1 = Topic.objects.first()
topic2 = Topic.objects.last()
Expand Down Expand Up @@ -53,3 +55,22 @@ def test_create_search_profile(
search_profile.query = query
search_profile.save()
assert search_profile.query == query


@pytest.mark.django_db
def test_search_profile_save_adds_number(
user,
search_profile_factory,
):
user_2 = search_profile_factory().user
assert user is not user_2
for i in range(5):
search_profile_factory(user=user)
for i in range(2):
search_profile_factory(user=user_2)

assert SearchProfile.objects.count() == 8
for i, search_profile in enumerate(SearchProfile.objects.filter(user=user)):
assert search_profile.number == i + 1
for i, search_profile in enumerate(SearchProfile.objects.filter(user=user_2)):
assert search_profile.number == i + 1