Merge pull request #738 from ae-utbm/fix-remove-from-picture

Fix button to remove a user from picture
This commit is contained in:
thomas girod 2024-07-26 14:48:28 +02:00 committed by GitHub
commit 2261782920
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 163 additions and 40 deletions

View File

@ -199,12 +199,6 @@
> form {
> p {
box-sizing: border-box;
> input {
width: 100%;
max-width: 100%;
box-sizing: border-box;
}
}
> .results_on_deck > div {
@ -219,12 +213,15 @@
right: 0;
}
}
> input {
width: 100%;
input {
min-width: 100%;
max-width: 100%;
box-sizing: border-box;
}
button {
font-weight: bold;
}
}
}
}

View File

@ -32,7 +32,6 @@
width: 100%;
}
// Django moment
> div.mini_profile_link {
position: relative;
@ -106,7 +105,6 @@
}
}
// Django moment
> a.mini_profile_link {
display: none;
}

View File

@ -1,17 +1,19 @@
from django.conf import settings
from ninja import Query
from ninja_extra import ControllerBase, api_controller, route
from ninja_extra.exceptions import PermissionDenied
from ninja_extra.permissions import IsAuthenticated
from pydantic import NonNegativeInt
from core.models import User
from sas.models import Picture
from sas.models import PeoplePictureRelation, Picture
from sas.schemas import PictureFilterSchema, PictureSchema
@api_controller("/sas")
class SasController(ControllerBase):
@api_controller("/sas/picture")
class PicturesController(ControllerBase):
@route.get(
"/picture",
"",
response=list[PictureSchema],
permissions=[IsAuthenticated],
url_name="pictures",
@ -22,11 +24,17 @@ class SasController(ControllerBase):
A user with an active subscription can see any picture, as long
as it has been moderated and not asked for removal.
An unsubscribed user can see the pictures he has been identified on
(only the moderated ones, too)
(only the moderated ones, too).
Notes:
Trying to fetch the pictures of another user with this route
while being unsubscribed will just result in an empty response.
Notes:
Unsubscribed users who are identified is not a rare case.
They can be UTT students, faluchards from other schools,
or even Richard Stallman (that ain't no joke,
cf. https://ae.utbm.fr/user/32663/pictures/)
"""
user: User = self.context.request.user
if not user.is_subscribed and filters.users_identified != {user.id}:
@ -45,3 +53,23 @@ class SasController(ControllerBase):
picture.compressed_url = picture.get_download_compressed_url()
picture.thumb_url = picture.get_download_thumb_url()
return pictures
@api_controller("/sas/relation", tags="User identification on SAS pictures")
class UsersIdentifiedController(ControllerBase):
@route.delete("/{relation_id}", permissions=[IsAuthenticated])
def delete_relation(self, relation_id: NonNegativeInt):
"""Untag a user from a SAS picture.
Root and SAS admins can delete any picture identification.
All other users can delete their own identification.
"""
relation = self.get_object_or_exception(PeoplePictureRelation, pk=relation_id)
user: User = self.context.request.user
if (
relation.user_id != user.id
and not user.is_root
and not user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
):
raise PermissionDenied
relation.delete()

View File

@ -1,10 +1,10 @@
from datetime import datetime
from ninja import FilterSchema, ModelSchema
from pydantic import Field
from ninja import FilterSchema, ModelSchema, Schema
from pydantic import Field, NonNegativeInt
from core.schemas import SimpleUserSchema
from sas.models import Picture
from sas.models import PeoplePictureRelation, Picture
class PictureFilterSchema(FilterSchema):
@ -23,3 +23,14 @@ class PictureSchema(ModelSchema):
full_size_url: str
compressed_url: str
thumb_url: str
class PictureCreateRelationSchema(Schema):
user_id: NonNegativeInt
picture_id: NonNegativeInt
class CreatedPictureRelationSchema(ModelSchema):
class Meta:
model = PeoplePictureRelation
fields = ["id", "user", "picture"]

View File

@ -4,11 +4,19 @@
<link rel="stylesheet" href="{{ scss('sas/picture.scss') }}">
{%- endblock -%}
{%- block additional_js -%}
<script src="{{ static('core/js/alpinejs.min.js') }}" defer></script>
{%- endblock -%}
{% block head %}
{{ super() }}
{% if picture.get_previous() %}
<link rel="preload" as="image" href="{{ url("sas:download_compressed", picture_id=picture.get_previous().id) }}">
<link
rel="preload"
as="image"
href="{{ url("sas:download_compressed", picture_id=picture.get_previous().id) }}"
>
{% endif %}
{% if picture.get_next() %}
<link rel="preload" as="image" href="{{ url("sas:download_compressed", picture_id=picture.get_next().id) }}">
@ -36,7 +44,8 @@
<div class="title">
<h3>{{ picture.get_display_name() }}</h3>
<h4>{{ picture.parent.children.filter(id__lte=picture.id).count() }} / {{ picture.parent.children.count() }}</h4>
<h4>{{ picture.parent.children.filter(id__lte=picture.id).count() }}
/ {{ picture.parent.children.count() }}</h4>
</div>
<br>
@ -100,7 +109,9 @@
<h5>{% trans %}Tools{% endtrans %}</h5>
<div>
<div>
<a class="text" href="{{ picture.get_download_url() }}">{% trans %}HD version{% endtrans %}</a>
<a class="text" href="{{ picture.get_download_url() }}">
{% trans %}HD version{% endtrans %}
</a>
<br>
<a class="text danger" href="?ask_removal">{% trans %}Ask for removal{% endtrans %}</a>
</div>
@ -139,20 +150,18 @@
{{ form.as_p() }}
<input type="submit" value="{% trans %}Go{% endtrans %}" />
</form>
<ul>
{% for r in picture.people.all() %}
<ul x-data="user_identification">
<template x-for="item in items" :key="item.id">
<li>
<a class="user" href="{{ r.user.get_absolute_url() }}">
{% if r.user.profile_pict %}
<div class="profile-pic" style="background-image: url('{{ r.user.profile_pict.get_download_url() }}');"></div>
{% endif %}
<span>{{ r.user.get_short_name() }}</span>
<a class="user" :href="item.user.url">
<img class="profile-pic" :src="item.user.picture" alt="image de profil"/>
<span x-text="item.user.name"></span>
</a>
{% if user == r.user or user.can_edit(picture) %}
<a class="delete" href="?remove_user={{ r.user.id }}">❌</a>
{% endif %}
<template x-if="can_be_removed(item)">
<a class="delete clickable" @click="remove(item)">❌</a>
</template>
</li>
{% endfor %}
</template>
</ul>
</div>
</div>
@ -162,6 +171,42 @@
{% block script %}
{{ super() }}
<script>
document.addEventListener("alpine:init", () => {
Alpine.data("user_identification", () => ({
items: [
{%- for r in picture.people.select_related("user", "user__profile_pict") -%}
{
id: {{ r.id }},
user: {
id: {{ r.user.id }},
name: "{{ r.user.get_short_name()|safe }}",
url: "{{ r.user.get_absolute_url() }}",
{% if r.user.profile_pict %}
picture: "{{ r.user.profile_pict.get_download_url() }}",
{% else %}
picture: "{{ static('core/img/unknown.jpg') }}",
{% endif %}
},
},
{%- endfor -%}
],
can_be_removed(item) {
{# If user is root or sas admin, he has the right, at "compile" time.
If not, he can delete only its own identification. #}
{% if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
return true;
{% else %}
return item.user.id === {{ user.id }};
{% endif %}
},
async remove(item) {
const res = await fetch(`/api/sas/relation/${item.id}`, {method: "DELETE"});
if (res.ok) {
this.items = this.items.filter((i) => i.id !== item.id)
}
},
}))
});
$(() => {
$(document).keydown((e) => {
switch (e.keyCode) {

View File

@ -1,10 +1,12 @@
from django.conf import settings
from django.db import transaction
from django.test import TestCase
from django.urls import reverse
from model_bakery import baker
from model_bakery.recipe import Recipe
from core.baker_recipes import old_subscriber_user, subscriber_user
from core.models import User
from core.models import RealGroup, User
from sas.models import Album, PeoplePictureRelation, Picture
@ -32,6 +34,8 @@ class TestSas(TestCase):
baker.make(PeoplePictureRelation, picture=pictures[4], user=cls.user_b)
baker.make(PeoplePictureRelation, picture=pictures[4], user=cls.user_c)
class TestPictureSearch(TestSas):
def test_anonymous_user_forbidden(self):
res = self.client.get(reverse("api:pictures"))
assert res.status_code == 403
@ -101,3 +105,49 @@ class TestSas(TestCase):
+ f"?users_identified={self.user_a.id}&users_identified={self.user_b.id}"
)
assert res.status_code == 403
class TestPictureRelation(TestSas):
def test_delete_relation_route_forbidden(self):
"""Test that unauthorized users are properly 403ed"""
# take a picture where user_a isn't identified
relation = PeoplePictureRelation.objects.exclude(user=self.user_a).first()
res = self.client.delete(f"/api/sas/relation/{relation.id}")
assert res.status_code == 403
for user in baker.make(User), self.user_a:
self.client.force_login(user)
res = self.client.delete(f"/api/sas/relation/{relation.id}")
assert res.status_code == 403
def test_delete_relation_with_authorized_users(self):
"""Test that deletion works as intended when called by an authorized user."""
relation: PeoplePictureRelation = self.user_a.pictures.first()
sas_admin_group = RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
sas_admin = baker.make(User, groups=[sas_admin_group])
root = baker.make(User, is_superuser=True)
for user in sas_admin, root, self.user_a:
with transaction.atomic():
self.client.force_login(user)
res = self.client.delete(f"/api/sas/relation/{relation.id}")
assert res.status_code == 200
assert not PeoplePictureRelation.objects.filter(pk=relation.id).exists()
transaction.set_rollback(True)
public = baker.make(User)
relation = public.pictures.create(picture=relation.picture)
self.client.force_login(public)
res = self.client.delete(f"/api/sas/relation/{relation.id}")
assert res.status_code == 200
assert not PeoplePictureRelation.objects.filter(pk=relation.id).exists()
def test_delete_twice(self):
"""Test a duplicate call on the delete route."""
self.client.force_login(baker.make(User, is_superuser=True))
relation = PeoplePictureRelation.objects.first()
res = self.client.delete(f"/api/sas/relation/{relation.id}")
assert res.status_code == 200
relation_count = PeoplePictureRelation.objects.count()
res = self.client.delete(f"/api/sas/relation/{relation.id}")
assert res.status_code == 404
assert PeoplePictureRelation.objects.count() == relation_count

View File

@ -143,12 +143,6 @@ class PictureView(CanViewMixin, DetailView, FormMixin):
self.object.rotate(270)
if "rotate_left" in request.GET:
self.object.rotate(90)
if "remove_user" in request.GET:
user = get_object_or_404(User, pk=int(request.GET["remove_user"]))
if user.id == request.user.id or request.user.is_in_group(
pk=settings.SITH_GROUP_SAS_ADMIN_ID
):
user.picture.filter(picture=self.object).delete()
if "ask_removal" in request.GET.keys():
self.object.is_moderated = False
self.object.asked_for_removal = True