entrouvert
/
chrono
14
1
Fork 0
Code Issues Pull Requests 4 Releases Wiki Activity

agendas: handle admin role in export file (#89990) #253

Merged
vdeniaud merged 1 commits from wip/89990-import-export-gerer-un-role-d-ad into main 2024-04-29 09:45:34 +02:00
Conversation 0 Commits 1 Files Changed 2 +39
vdeniaud commented 2024-04-24 16:11:35 +02:00
Owner
Copy Link
No description provided.
vdeniaud added 1 commit 2024-04-24 16:11:35 +02:00
gitea/chrono/pipeline/head This commit looks good Details
47136b1391 agendas: handle admin role in export file (#89990)
vdeniaud changed title from WIP: agendas: handle admin role in export file (#89990) to agendas: handle admin role in export file (#89990) 2024-04-24 16:23:42 +02:00
pmarillonnet reviewed 2024-04-25 14:49:49 +02:00
pmarillonnet left a comment
Owner
Copy Link

Deux interrogations de ma part (plus par manque de familiarité avec le code de chrono de ma part que de réelle remise en question de la PR), dis-moi ce que tu en penses.

Edit: et on dirait que mon brouillon de révision est passé à la trappe, damn.

Deux interrogations de ma part (plus par manque de familiarité avec le code de chrono de ma part que de réelle remise en question de la PR), dis-moi ce que tu en penses. Edit: et on dirait que mon brouillon de révision est passé à la trappe, damn.
pmarillonnet reviewed 2024-04-25 14:52:44 +02:00
chrono/agendas/models.py Outdated
@ -566,2 +566,4 @@
if permissions.get(permission):
data[permission + '_role'] = Group.objects.get(name=permissions[permission])
if permissions.get('admin'):
data['edit_role'] = Group.objects.get(name=permissions['admin'])
pmarillonnet commented 2024-04-25 14:52:44 +02:00
Owner
Copy Link

Ok je comprends l’idée mais pourquoi :
· on ne cherche pas à attraper ici les Group.DoesNotExist ? J’imagine qu’il y a des cas où le rôle déclaré dans le fichier d’export n’est pas connu de la cible (?)
· on ne laisse pas inchangée la valeur de data['edit_role'] si déjà renseignée ici ? N’y-a-t-il pas des cas où cette valeur existe déjà et où il faut pas l’écraser ?

Ok je comprends l’idée mais pourquoi : · on ne cherche pas à attraper ici les Group.DoesNotExist ? J’imagine qu’il y a des cas où le rôle déclaré dans le fichier d’export n’est pas connu de la cible (?) · on ne laisse pas inchangée la valeur de `data['edit_role']` si déjà renseignée ici ? N’y-a-t-il pas des cas où cette valeur existe déjà et où il faut pas l’écraser ?
vdeniaud commented 2024-04-25 15:29:09 +02:00
Author
Owner
Copy Link

Le DoesNotExist est attrapé ailleurs, j'ai ajouté un test.

Si il y a un rôle admin dans l'export, le comportement attendu est bien d'écraser le role d'édition (en recette on aura le rôle admin qui remplace le rôle d'édition, et le nouveau rôle d'édition qui ne correspond à rien)

Le DoesNotExist est attrapé ailleurs, j'ai ajouté un test. Si il y a un rôle admin dans l'export, le comportement attendu est bien d'écraser le role d'édition (en recette on aura le rôle admin qui remplace le rôle d'édition, et le nouveau rôle d'édition qui ne correspond à rien)
pmarillonnet commented 2024-04-25 15:36:24 +02:00
Owner
Copy Link

Ah oui ok pigé, j’avais pas saisi cette subtilité, merci.

Ah oui ok pigé, j’avais pas saisi cette subtilité, merci.
vdeniaud force-pushed wip/89990-import-export-gerer-un-role-d-ad from 47136b1391 to dc871e4edf 2024-04-25 15:27:44 +02:00 Compare
pmarillonnet approved these changes 2024-04-25 15:36:39 +02:00
vdeniaud merged commit 733cdfc9a9 into main 2024-04-29 09:45:34 +02:00
vdeniaud deleted branch wip/89990-import-export-gerer-un-role-d-ad 2024-04-29 09:45:34 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
pmarillonnet
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/chrono#253
No description provided.
Delete Branch "wip/89990-import-export-gerer-un-role-d-ad"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?