🗃️(teams) remove slug field
After some reflexion, the use of a slug field raises to many problems without being really needed. One problem is the slug is made from the group name, but we don't have unicity on this, so a user might be blocked without any clue. We also want to allow group names to be reused (which is already allowed except for the automatic slug). The unique ID that will be shared with Service Providers will be the PK/UUID.
This commit is contained in:
@@ -26,6 +26,10 @@ and this project adheres to
|
|||||||
- 🔇(backend) remove Sentry duplicated warning/errors
|
- 🔇(backend) remove Sentry duplicated warning/errors
|
||||||
- 👷(ci) add sharding e2e tests #467
|
- 👷(ci) add sharding e2e tests #467
|
||||||
|
|
||||||
|
### Removed
|
||||||
|
|
||||||
|
- 🗃️(teams) remove `slug` field
|
||||||
|
|
||||||
## [1.4.1] - 2024-10-23
|
## [1.4.1] - 2024-10-23
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
@@ -115,7 +115,6 @@ class TeamAdmin(admin.ModelAdmin):
|
|||||||
inlines = (TeamAccessInline, TeamWebhookInline)
|
inlines = (TeamAccessInline, TeamWebhookInline)
|
||||||
list_display = (
|
list_display = (
|
||||||
"name",
|
"name",
|
||||||
"slug",
|
|
||||||
"created_at",
|
"created_at",
|
||||||
"updated_at",
|
"updated_at",
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -172,7 +172,6 @@ class TeamSerializer(serializers.ModelSerializer):
|
|||||||
"""Serialize teams."""
|
"""Serialize teams."""
|
||||||
|
|
||||||
abilities = serializers.SerializerMethodField(read_only=True)
|
abilities = serializers.SerializerMethodField(read_only=True)
|
||||||
slug = serializers.SerializerMethodField()
|
|
||||||
|
|
||||||
class Meta:
|
class Meta:
|
||||||
model = models.Team
|
model = models.Team
|
||||||
@@ -181,7 +180,6 @@ class TeamSerializer(serializers.ModelSerializer):
|
|||||||
"name",
|
"name",
|
||||||
"accesses",
|
"accesses",
|
||||||
"abilities",
|
"abilities",
|
||||||
"slug",
|
|
||||||
"created_at",
|
"created_at",
|
||||||
"updated_at",
|
"updated_at",
|
||||||
]
|
]
|
||||||
@@ -189,7 +187,6 @@ class TeamSerializer(serializers.ModelSerializer):
|
|||||||
"id",
|
"id",
|
||||||
"accesses",
|
"accesses",
|
||||||
"abilities",
|
"abilities",
|
||||||
"slug",
|
|
||||||
"created_at",
|
"created_at",
|
||||||
"updated_at",
|
"updated_at",
|
||||||
]
|
]
|
||||||
@@ -209,10 +206,6 @@ class TeamSerializer(serializers.ModelSerializer):
|
|||||||
return team.get_abilities(request.user)
|
return team.get_abilities(request.user)
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
def get_slug(self, instance):
|
|
||||||
"""Return slug from the team's name."""
|
|
||||||
return instance.get_slug()
|
|
||||||
|
|
||||||
|
|
||||||
class InvitationSerializer(serializers.ModelSerializer):
|
class InvitationSerializer(serializers.ModelSerializer):
|
||||||
"""Serialize invitations."""
|
"""Serialize invitations."""
|
||||||
|
|||||||
18
src/backend/core/migrations/0003_team_slug_nullable.py
Normal file
18
src/backend/core/migrations/0003_team_slug_nullable.py
Normal file
@@ -0,0 +1,18 @@
|
|||||||
|
# Generated by Django 5.1.2 on 2024-11-04 14:25
|
||||||
|
|
||||||
|
from django.db import migrations, models
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
|
||||||
|
dependencies = [
|
||||||
|
('core', '0002_add_organization_and_more'),
|
||||||
|
]
|
||||||
|
|
||||||
|
operations = [
|
||||||
|
migrations.AlterField(
|
||||||
|
model_name='team',
|
||||||
|
name='slug',
|
||||||
|
field=models.SlugField(max_length=100, null=True),
|
||||||
|
),
|
||||||
|
]
|
||||||
@@ -22,7 +22,6 @@ from django.db import models, transaction
|
|||||||
from django.template.loader import render_to_string
|
from django.template.loader import render_to_string
|
||||||
from django.utils import timezone
|
from django.utils import timezone
|
||||||
from django.utils.functional import lazy
|
from django.utils.functional import lazy
|
||||||
from django.utils.text import slugify
|
|
||||||
from django.utils.translation import gettext_lazy as _
|
from django.utils.translation import gettext_lazy as _
|
||||||
from django.utils.translation import override
|
from django.utils.translation import override
|
||||||
|
|
||||||
@@ -486,7 +485,6 @@ class Team(BaseModel):
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
name = models.CharField(max_length=100)
|
name = models.CharField(max_length=100)
|
||||||
slug = models.SlugField(max_length=100, unique=True, null=False, editable=False)
|
|
||||||
|
|
||||||
users = models.ManyToManyField(
|
users = models.ManyToManyField(
|
||||||
User,
|
User,
|
||||||
@@ -511,15 +509,6 @@ class Team(BaseModel):
|
|||||||
def __str__(self):
|
def __str__(self):
|
||||||
return self.name
|
return self.name
|
||||||
|
|
||||||
def save(self, *args, **kwargs):
|
|
||||||
"""Override save function to compute the slug."""
|
|
||||||
self.slug = self.get_slug()
|
|
||||||
return super().save(*args, **kwargs)
|
|
||||||
|
|
||||||
def get_slug(self):
|
|
||||||
"""Compute slug value from name."""
|
|
||||||
return slugify(self.name)
|
|
||||||
|
|
||||||
def get_abilities(self, user):
|
def get_abilities(self, user):
|
||||||
"""
|
"""
|
||||||
Compute and return abilities for a given user on the team.
|
Compute and return abilities for a given user on the team.
|
||||||
|
|||||||
@@ -5,12 +5,11 @@ Tests for Teams API endpoint in People's core app: create
|
|||||||
import pytest
|
import pytest
|
||||||
from rest_framework.status import (
|
from rest_framework.status import (
|
||||||
HTTP_201_CREATED,
|
HTTP_201_CREATED,
|
||||||
HTTP_400_BAD_REQUEST,
|
|
||||||
HTTP_401_UNAUTHORIZED,
|
HTTP_401_UNAUTHORIZED,
|
||||||
)
|
)
|
||||||
from rest_framework.test import APIClient
|
from rest_framework.test import APIClient
|
||||||
|
|
||||||
from core.factories import OrganizationFactory, TeamFactory, UserFactory
|
from core.factories import OrganizationFactory, UserFactory
|
||||||
from core.models import Team
|
from core.models import Team
|
||||||
|
|
||||||
pytestmark = pytest.mark.django_db
|
pytestmark = pytest.mark.django_db
|
||||||
@@ -55,82 +54,6 @@ def test_api_teams_create_authenticated():
|
|||||||
assert team.accesses.filter(role="owner", user=user).exists()
|
assert team.accesses.filter(role="owner", user=user).exists()
|
||||||
|
|
||||||
|
|
||||||
def test_api_teams_create_authenticated_slugify_name():
|
|
||||||
"""
|
|
||||||
Creating teams should automatically generate a slug.
|
|
||||||
"""
|
|
||||||
organization = OrganizationFactory(with_registration_id=True)
|
|
||||||
user = UserFactory(organization=organization)
|
|
||||||
client = APIClient()
|
|
||||||
client.force_login(user)
|
|
||||||
|
|
||||||
response = client.post(
|
|
||||||
"/api/v1.0/teams/",
|
|
||||||
{"name": "my team"},
|
|
||||||
)
|
|
||||||
|
|
||||||
assert response.status_code == HTTP_201_CREATED
|
|
||||||
team = Team.objects.get()
|
|
||||||
assert team.name == "my team"
|
|
||||||
assert team.slug == "my-team"
|
|
||||||
assert team.organization == organization
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize(
|
|
||||||
"param",
|
|
||||||
[
|
|
||||||
("my team", "my-team"),
|
|
||||||
("my team", "my-team"),
|
|
||||||
("MY TEAM TOO", "my-team-too"),
|
|
||||||
("mon équipe", "mon-equipe"),
|
|
||||||
("front devs & UX", "front-devs-ux"),
|
|
||||||
],
|
|
||||||
)
|
|
||||||
def test_api_teams_create_authenticated_expected_slug(param):
|
|
||||||
"""
|
|
||||||
Creating teams should automatically create unaccented, no unicode, lower-case slug.
|
|
||||||
"""
|
|
||||||
organization = OrganizationFactory(with_registration_id=True)
|
|
||||||
user = UserFactory(organization=organization)
|
|
||||||
|
|
||||||
client = APIClient()
|
|
||||||
client.force_login(user)
|
|
||||||
|
|
||||||
response = client.post(
|
|
||||||
"/api/v1.0/teams/",
|
|
||||||
{
|
|
||||||
"name": param[0],
|
|
||||||
},
|
|
||||||
)
|
|
||||||
|
|
||||||
assert response.status_code == HTTP_201_CREATED
|
|
||||||
team = Team.objects.get()
|
|
||||||
assert team.name == param[0]
|
|
||||||
assert team.slug == param[1]
|
|
||||||
assert team.organization == organization
|
|
||||||
|
|
||||||
|
|
||||||
def test_api_teams_create_authenticated_unique_slugs():
|
|
||||||
"""
|
|
||||||
Creating teams should raise an error if already existing slug.
|
|
||||||
"""
|
|
||||||
TeamFactory(name="existing team")
|
|
||||||
user = UserFactory()
|
|
||||||
|
|
||||||
client = APIClient()
|
|
||||||
client.force_login(user)
|
|
||||||
|
|
||||||
response = client.post(
|
|
||||||
"/api/v1.0/teams/",
|
|
||||||
{
|
|
||||||
"name": "èxisting team",
|
|
||||||
},
|
|
||||||
)
|
|
||||||
|
|
||||||
assert response.status_code == HTTP_400_BAD_REQUEST
|
|
||||||
assert response.json()["slug"] == ["Team with this Slug already exists."]
|
|
||||||
|
|
||||||
|
|
||||||
def test_api_teams_create_cannot_override_organization():
|
def test_api_teams_create_cannot_override_organization():
|
||||||
"""
|
"""
|
||||||
Authenticated users should be able to create teams and not
|
Authenticated users should be able to create teams and not
|
||||||
|
|||||||
@@ -69,7 +69,6 @@ def test_api_teams_retrieve_authenticated_related():
|
|||||||
assert response.json() == {
|
assert response.json() == {
|
||||||
"id": str(team.id),
|
"id": str(team.id),
|
||||||
"name": team.name,
|
"name": team.name,
|
||||||
"slug": team.slug,
|
|
||||||
"abilities": team.get_abilities(user),
|
"abilities": team.get_abilities(user),
|
||||||
"created_at": team.created_at.isoformat().replace("+00:00", "Z"),
|
"created_at": team.created_at.isoformat().replace("+00:00", "Z"),
|
||||||
"updated_at": team.updated_at.isoformat().replace("+00:00", "Z"),
|
"updated_at": team.updated_at.isoformat().replace("+00:00", "Z"),
|
||||||
|
|||||||
@@ -7,7 +7,6 @@ import random
|
|||||||
import pytest
|
import pytest
|
||||||
from rest_framework.status import (
|
from rest_framework.status import (
|
||||||
HTTP_200_OK,
|
HTTP_200_OK,
|
||||||
HTTP_400_BAD_REQUEST,
|
|
||||||
HTTP_401_UNAUTHORIZED,
|
HTTP_401_UNAUTHORIZED,
|
||||||
HTTP_403_FORBIDDEN,
|
HTTP_403_FORBIDDEN,
|
||||||
HTTP_404_NOT_FOUND,
|
HTTP_404_NOT_FOUND,
|
||||||
@@ -125,7 +124,7 @@ def test_api_teams_update_authenticated_administrators():
|
|||||||
elif key == "updated_at":
|
elif key == "updated_at":
|
||||||
assert value > initial_values[key]
|
assert value > initial_values[key]
|
||||||
else:
|
else:
|
||||||
# name, slug and abilities successfully modified
|
# name and abilities successfully modified
|
||||||
assert value == new_values[key]
|
assert value == new_values[key]
|
||||||
|
|
||||||
|
|
||||||
@@ -158,7 +157,7 @@ def test_api_teams_update_authenticated_owners():
|
|||||||
elif key == "updated_at":
|
elif key == "updated_at":
|
||||||
assert value > old_team_values[key]
|
assert value > old_team_values[key]
|
||||||
else:
|
else:
|
||||||
# name, slug and abilities successfully modified
|
# name and abilities successfully modified
|
||||||
assert value == new_team_values[key]
|
assert value == new_team_values[key]
|
||||||
|
|
||||||
|
|
||||||
@@ -189,31 +188,3 @@ def test_api_teams_update_administrator_or_owner_of_another():
|
|||||||
team.refresh_from_db()
|
team.refresh_from_db()
|
||||||
team_values = serializers.TeamSerializer(instance=team).data
|
team_values = serializers.TeamSerializer(instance=team).data
|
||||||
assert team_values == old_team_values
|
assert team_values == old_team_values
|
||||||
|
|
||||||
|
|
||||||
def test_api_teams_update_existing_slug_should_return_error():
|
|
||||||
"""
|
|
||||||
Updating a team's name to an existing slug should return a bad request,
|
|
||||||
instead of creating a duplicate.
|
|
||||||
"""
|
|
||||||
user = factories.UserFactory()
|
|
||||||
|
|
||||||
client = APIClient()
|
|
||||||
client.force_login(user)
|
|
||||||
|
|
||||||
factories.TeamFactory(name="Existing team", users=[(user, "administrator")])
|
|
||||||
my_team = factories.TeamFactory(name="New team", users=[(user, "administrator")])
|
|
||||||
|
|
||||||
updated_values = serializers.TeamSerializer(instance=my_team).data
|
|
||||||
# Update my team's name for existing team. Creates a duplicate slug
|
|
||||||
updated_values["name"] = "existing team"
|
|
||||||
response = client.put(
|
|
||||||
f"/api/v1.0/teams/{my_team.id!s}/",
|
|
||||||
updated_values,
|
|
||||||
format="json",
|
|
||||||
)
|
|
||||||
assert response.status_code == HTTP_400_BAD_REQUEST
|
|
||||||
assert response.json()["slug"] == ["Team with this Slug already exists."]
|
|
||||||
# Both teams names and slugs should be unchanged
|
|
||||||
assert my_team.name == "New team"
|
|
||||||
assert my_team.slug == "new-team"
|
|
||||||
|
|||||||
@@ -47,12 +47,6 @@ def test_models_teams_name_max_length():
|
|||||||
factories.TeamFactory(name="a " * 51)
|
factories.TeamFactory(name="a " * 51)
|
||||||
|
|
||||||
|
|
||||||
def test_models_teams_slug_empty():
|
|
||||||
"""Slug field should not be empty."""
|
|
||||||
with pytest.raises(ValidationError, match="This field cannot be blank."):
|
|
||||||
models.Team.objects.create(slug="")
|
|
||||||
|
|
||||||
|
|
||||||
# get_abilities
|
# get_abilities
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -174,8 +174,6 @@ def create_demo(stdout):
|
|||||||
queue.push(
|
queue.push(
|
||||||
models.Team(
|
models.Team(
|
||||||
name=f"Team {i:d}",
|
name=f"Team {i:d}",
|
||||||
# slug should be automatic but bulk_create doesn't use save
|
|
||||||
slug=f"team-{i:d}",
|
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
queue.flush()
|
queue.flush()
|
||||||
|
|||||||
@@ -93,27 +93,6 @@ test.describe('Teams Create', () => {
|
|||||||
await expect(page).toHaveURL(/\/teams\//);
|
await expect(page).toHaveURL(/\/teams\//);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('checks error when duplicate team', async ({ page, browserName }) => {
|
|
||||||
const panel = page.getByLabel('Teams panel').first();
|
|
||||||
|
|
||||||
await panel.getByRole('link', { name: 'Add a team' }).click();
|
|
||||||
|
|
||||||
const teamName = `My duplicate team ${browserName}-${Math.floor(Math.random() * 1000)}`;
|
|
||||||
await page.getByText('Team name').fill(teamName);
|
|
||||||
await page.getByRole('button', { name: 'Create the team' }).click();
|
|
||||||
|
|
||||||
await panel.getByRole('link', { name: 'Add a team' }).click();
|
|
||||||
|
|
||||||
await page.getByText('Team name').fill(teamName);
|
|
||||||
await page.getByRole('button', { name: 'Create the team' }).click();
|
|
||||||
|
|
||||||
await expect(
|
|
||||||
page.getByText(
|
|
||||||
'This name is already used for another group. Please enter another one.',
|
|
||||||
),
|
|
||||||
).toBeVisible();
|
|
||||||
});
|
|
||||||
|
|
||||||
test('checks 404 on teams/[id] page', async ({ page }) => {
|
test('checks 404 on teams/[id] page', async ({ page }) => {
|
||||||
await page.goto('/teams/some-unknown-team');
|
await page.goto('/teams/some-unknown-team');
|
||||||
await expect(
|
await expect(
|
||||||
|
|||||||
Reference in New Issue
Block a user