Improve error message from TracAdmin remove when permission doesn't exist

[Ryan J Ollos]

When attempting to assign an undefined permission a helpful error is echoed.

$ trac-admin ../tracdev permission add anonymous NOT_A_PERM
TracError: NOT_A_PERM is not a valid action.

The error message seen when attempting to remove an undefined permission is:

$ trac-admin ../tracdev permission remove anonymous NOT_A_PERM
Error: Cannot remove permission NOT_A_PERM for user anonymous. The user has not been granted the permission.

It would probably be more helpful to check if the permission exists before trying to remove it from a user, in order to echo a more helpful error message in the case that the user makes a typo or a Component isn't loaded.

[walty <walty8@…>]

pls let me know if unit test is needed..

[Ryan J Ollos]

Unit test would be good. You can add a test in tags/trac-1.0.7/trac/admin/tests/console.py#L307​. Expected output goes in tags/trac-1.0.7/trac/admin/tests/console-tests.txt#L96​.

[walty]

added unit test

[walty]

While I try to add the unit test to the patch, I got one minor issue. The original test suites did not get passed (9 cases failed), I tried to revert all my patches, and set up a clean test environment, and it's still the same.

Later I found that it seems to be related with [13853], which removes BATCH_TICKET_MODIFY by default, so the result file for permission list could not match with the actual output.

I finally manually added back the permission in order to test my patch (but the corresponding change is not included in the patch file).

[Ryan J Ollos]

Thank you for finding the cause of the test failures! I had noticed those a few times, but never investigated. Fixed on 1.0-stable in [14204], merged in [14205].

[Ryan J Ollos]

Patch looks good. I added a few additional minor changes in log:rjollos.git:t11938.

[walty]

Thanks, I checked the code, and it did make more sense with elif :)

[Ryan J Ollos]

Committed to trunk in [14210:14211].