-
Notifications
You must be signed in to change notification settings - Fork 102
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
fix(permission) add the json params to the checked params #2712
Conversation
2417f10
to
abf568c
Compare
Codecov ReportAll modified lines are covered by tests ✅
... and 8 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il y a eu avoir des changements dans le composant dataset car historiquement les infos était passé par query string et non par POST (si je me souviens bien – et ça expliquerai cette regression).
Globalement, il vaut mieux qu’une route mélange les paramètres query string et les paramètres POST. En effet, les données par POST sont typées contrairement aux données par query string, ce qui peut amener à pas mal de confusion (comme interpréter comme vrai la chaîne de texte false
).
En revanche il ne pose pas de soucis d’avoir certains paramètres par query string, et d’autres par POST, mais il faut faire attention à bien les transmettre de la manière attendu.
@@ -103,6 +103,8 @@ def get_datasets(): | |||
:returns: `list<TDatasets>` | |||
""" | |||
params = MultiDict(request.args) | |||
if request.is_json: | |||
params.update(MultiDict(request.json)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
params.update(MultiDict(request.json)) | |
params.update(request.json) |
update
accepte un mapping donc un dict
f692130
to
8a8f1b8
Compare
query = TDatasets.query.filter_by_creatable(params.pop("create")) | ||
create = params.pop("create").split(".") | ||
if len(create) > 1: | ||
query = TDatasets.query.filter_by_creatable(create[0], object_code=create[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Peut être nommé les paramètres ?
query = TDatasets.query.filter_by_creatable(create[0], object_code=create[1]) | |
query = TDatasets.query.filter_by_creatable(module_code=create[0], object_code=create[1]) |
Je rajouterais bien aussi dans la docstring le fait qu'on peut passer le module_code + le object_code dans "create". ça fait une mini doc sur l'API pour qu'on s'en souvienne
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pas faux, on risque d'oublier pourquoi ya ca sinon
8a8f1b8
to
eb25cff
Compare
Fix le problème de permissions dans Occtax, Occhab et Import sur la liste des JDD (les permissions C du module n'étaient pas prise en compte, mais seulement celles du R sur Métadonnées)