inveniosoftware / invenio-communities

Invenio communities module.
https://invenio-communities.readthedocs.io
MIT License
5 stars 73 forks source link

fix is verified #1174

Closed utnapischtim closed 1 month ago

utnapischtim commented 1 month ago

NOTES

utnapischtim commented 1 month ago

should i add the other two commits about fixing the ValidationError handling? i am asking because it would need to update the tests too and not to make work which is not wanted i would like to ask first.

zzacharo commented 1 month ago

should i add the other two commits about fixing the ValidationError handling? i am asking because it would need to update the tests too and not to make work which is not wanted i would like to ask first.

Can you elaborate on the current bug that the ValidationError exception fixes? We never caught validation errors properly?

utnapischtim commented 1 month ago

Can you elaborate on the current bug that the ValidationError exception fixes? We never caught validation errors properly?

it is from the usability point of view but to fix it properly it produces more work then i thought . the point for me was that if i would remove the owner without stating the group as the only owner i get a validation error back. the visualization could be improved. my simple improvement was to catch the ValidationError and give back only the string. this would show only the message to the user which in my point of view improves the usability. the line "ValidationError" is mostly to technical for users.

but i think we should put a little bit more thoughts into the error handling here and therefore i will remove those two commits so this PR is only on fixing the is verified bug.