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

Adding tests for permissions under main use cases WIP #12

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

chris-adam
Copy link

No description provided.

self.assertEqual(task_perms["encodeur"], [True, False, True, True, False, True, False, False, False])

# change_user(self.portal, "agent")
# FIXME agent does not have permission to treat the mail
Copy link
Author

Choose a reason for hiding this comment

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

@sgeulette Est-ce que tu as une idée de pourquoi l'utilisateur agent n'a pas le droit de "treat" le mail entrant ici ? J'ai déjà regardé avec Thomas et Benoit mais ça n'a rien donné. Tous les champs obligatoires sont bien remplis, un treating est bien assigné et "agent" est bien dans le tg. L'agent devrait avoir le droit "Modify portal Content" sur l'objet mais il n'a aucune permission...

Copy link
Author

Choose a reason for hiding this comment

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

Le lecteur n'a aussi jamais aucun droit sur quoi que ce soit, ce qui ne m'a pas l'air normal

Copy link
Author

Choose a reason for hiding this comment

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

@sgeulette up ?

Je n'ai pas non plus fait 3 fichiers comme tu me l'avais conseillé. ça n'est pas encore trop long mais on peut split en plusieurs fichiers facilement si nécessaire

Copy link
Member

Choose a reason for hiding this comment

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

Je revois ce point dès que c'est plus lisible ;-)

Copy link
Member

@sgeulette sgeulette left a comment

Choose a reason for hiding this comment

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

améliorer la lisibilité des tests

self.assertFalse(any(task_perms["dirg"]))
self.assertFalse(any(task_perms["agent"]))
self.assertFalse(any(task_perms["agent1"]))
self.assertEqual(task_perms["encodeur"], [True, False, True, True, False, True, False, False, False])
Copy link
Member

Choose a reason for hiding this comment

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

L'écriture des tests comme c'est fait là n'est pas assez claire à la lecture. Il faut avoir en mémoire toutes les permissions pour savoir ce qui est vrai ou faux.
Une possibilité serait de passer en argument à une fonction la liste des permissions (sous une forme d'alias pour faire plus court, ex: 'a_c_i' pour "Access contents information", etc) dans l'idée de faciliter à la lecture la correspondance entre permission et résultat.

self.pw.doActionFor(imail, "propose_to_manager")

imail_perms = self.get_perms(imail)
self.assertFalse(any(imail_perms["chef"]))
Copy link
Member

Choose a reason for hiding this comment

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

par défaut, on est dans un workflow sans n+1. Donc chef n'aura normalement jamais de droit.
Une fois que les tests seront bons, il serait intéressant de faire une copie de la méthode de test pour l'appliquer au workflow n+1

self.assertEqual(task_perms["encodeur"], [True, False, True, True, False, True, False, False, False])

# change_user(self.portal, "agent")
# FIXME agent does not have permission to treat the mail
Copy link
Member

Choose a reason for hiding this comment

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

Je revois ce point dès que c'est plus lisible ;-)

from zope.intid.interfaces import IIntIds


class TestPermissionsIncomingMail(TestPermissionsBase):
Copy link
Author

Choose a reason for hiding this comment

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

@sgeulette J'ai refactoré un peu les tests. Je n'ai fait ue incoming mail pour l'instant. Je ferai les autres si ça te convient comme ça (pour éviter de devoir changer plusieurs fois les emails et les outgoing mails)

Copy link
Member

@sgeulette sgeulette left a comment

Choose a reason for hiding this comment

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

C'est en effet beaucoup plus lisible.
Je vois du coup des choses à vérifier/revoir au niveau des settings.
Tu peux appliquer cette façon de faire au reste des fichiers.

)

# change_user(self.portal, "agent")
# FIXME agent does not have permission to treat the mail
Copy link
Author

Choose a reason for hiding this comment

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

Une idée de ceci ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants