Amélioration système de ban#1268
Conversation
Com, Subscriptions, Posters, Others
HTMX, Alpine, Invoice Calls, Products, Bugfixes, Other
Sith theme, `Selling.date` index, galaxy simplification, OG tags, dependencies update, bugfixes and others
Refactors, better `PageRev` handling, better user invisibilisation and fixes
imperosol
left a comment
There was a problem hiding this comment.
Pour données de dev, tu peux les rajouter dans core/management/commands/populate_more.py. T'es pas obligé de créer 50 bans non plus, dans la mesure où on en a moins que ça sur le vrai site.
Il faudrait ajouter quelques tests.
Et, le plus important, je sais pas trop comment ça passe niveau RGPD. Faire sortir des données sensibles du système, ça doit être quelque chose de justifié et de très contrôlé (c'est pour ça qu'on ne permet de générer une liste des cotisants). Donc je suis pas sûr que légalement, on a le droit de faire ça.
Dans le cas où on a le droit, il faudrait mettre un message d'avertissement au moment d'exporter le pdf, pour faire savoir de manière explicite à l'utilisateur qu'il manipule des données extrêmement sensibles, en lui donnant une liste des choses qu'il pourra faire et qu'il ne pourra pas faire avec ces données.
Si le but est de prévenir les orgas d'une soirée que quelqu'un n'a pas le droit de rentrer, juste la photo et le nom suffisent, donc je pense pas que ça soit une bonne idée de permettre d'avoir le motif du ban.
| LANGUAGES = [ | ||
| ("fr", _("French")), | ||
| ("en", _("English")), | ||
| ] |
There was a problem hiding this comment.
Je saisis pas trop l'intérêt de générer en français ou en anglais.
Le document contient relativement peu de texte et est dédié aux orgas des soirées, donc des francophones.
|
D'un point du vue RGPD il faudrait les mettre à jour, mais c'est OK si la liste est détruite derrière, d’où un PDF pour le papier. |
Co-authored-by: thomas girod <56346771+imperosol@users.noreply.github.com>
Co-authored-by: thomas girod <56346771+imperosol@users.noreply.github.com>
…ndling by providing barman message
|
Cette Pr vas etre changer vers une amélioration plus global du système de ban |
|
Je sais pas comment on check les cotisations en local donc hésitez pas à me le dire ❤️ |
C'est à dire ? |
update uv dependencies
Ouvre plutôt des PRs différentes. Refactor les bans, et ajouter la fonctionnalité de la génération du PDF, c'est deux trucs différents. Comme ça, ça sera plus simple à review, et ça bloquera pas tout s'il y a des choses à modifier. |
- Add separate URL and view logic for admin notifications when a banned user attempts a purchase without a barman present - Update admin notification and ban page rendering to handle cases where barman is absent - Refactor and extend tests for banned user scenarios, including admin notification URL checks and edge cases - Clean up imports and minor code comments for clarity - Update dependencies in lockfile
|
Mise a jour du fonctionnement. Pour les barman :
Pour la personne ban :
Pour les admin (root) :
Test :
Dite moi si ce behavor est important je peux toujours changer le fonctionnement pour juste retourner une 302 avec le message d'erreur. |
… admin UI to use them - Introduce /api/counter/<id>/seller/add and /api/counter/<id>/seller/remove endpoints for bulk seller management with detailed error handling - Update admin UI for banned users with counter permissions to use new API endpoints via AJAX, providing real-time feedback - Remove legacy admin_remove_user_from_counter view and related URL - Refactor admin views and URL patterns for clarity and consistency - Add comprehensive tests for new API endpoints - Update translations for new UI messages
…nter click test coverage - Add test_seller_api.py with parameterized tests for bulk add/remove seller API endpoints, covering edge cases and error codes - Refactor test_ban.py to use parameterized tests for PDF generation, reducing duplication and increasing coverage - Enhance test_counter.py to verify ban logic takes precedence over can_buy and improve session authentication checks - Update ban_user_has_counter_permission.jinja to use consistent double quotes in JS and template tags - Refactor click.py to prioritize ban checks before can_buy and session validation
|
Update :
|
|
Honnêtement, quelle proportion du code a été générée par IA ? |
|
Globalement tout ce qui est sur le site à était fait à la main avec une pass d'IA dessus pour corriger/best practices. Les test ont était générer par IA avec description des inputs et outputs attendues puis vérification et adaptation à la main. En gros c'est surtout les test que j'avais fait avant avec a chaque fois une def (J'avais pas le temps de tous les écrire et je connaissais pas la paramétrisation), mais la paramétrisation à était faite à la main avec un matching des test case avec l'aide de l'IA (Je lui donne les nouveaux et les anciens et il doivent êtres identiques, double checking). La partie pdf pour les math, et que l'alignement soit bien. Il y a aussi eu un peu pour le front pour avoir une structure de base. |
imperosol
left a comment
There was a problem hiding this comment.
Je laisse une petite partie des commentaires que j'ai envie de laisser, parce que 1700 lignes de code dont le style est pas raccord avec le reste de la base de code (et pas raccord avec lui-même par moments), c'est vraiment la mort à review.
| <div class="row gap"> | ||
| <div> | ||
| <a href="{{ url("core:user_profile", ban_user.id) }}"> | ||
| <img src="{{ ban_user.profile_pict.get_download_url() if ban_user.profile_pict else static("core/img/unknown.jpg") }}" alt="{% trans %}Profile photo{% endtrans %}" style="max-width:120px;max-height:120px;border-radius:8px;"> |
There was a problem hiding this comment.
Evite les styles inline dans le html
There was a problem hiding this comment.
Même pour les photo ? On est sur une page admin et c'est un composant custom qui ne sert qu'une fois
| admin_group_ids = [settings.SITH_GROUP_ROOT_ID] | ||
| notif_type = "BANNED_COUNTER_ATTEMPT" | ||
| counter = self.get_object() | ||
| if barman is not None and hasattr(barman, "id") and barman.id is not None: |
There was a problem hiding this comment.
Dans ta fonction, barman est type-hint comme User. Si à ce stade tu as besoin de faire autant de null-check, c'est que quelque chose est déjà foireux en amont.
There was a problem hiding this comment.
C'est pendant les test je me suis rendu compte que les notif pouvaient partir sans barman associé, c'est donc pour ce cas précis, en temps normal cela n'arrive pas mais je préfère le couvrir pour éviter une erreur possible dans un scénario X
|
C'était pas la bonne cible de merge, j'ai changé vers taiste :) |
|
rebase ? 👀 |
|
PR morte |
Hello !
Petite PR pour un système de partage des bans.
Il nous faut un moyen de partager les bans avec l'orga des soirées et autres. J'ai donc mis en place un système avec un export pdf de la liste de ban en cours pour une certaine date.
Ce système permet de :
C'est uniquement accessible aux root et ça cache le message de raison.
Voila une query SQL pour ban 50 personnes :