Prevent displaying model verbose_name in permissions UI for custom permissions

[NXPY123]

Fixes #10982 Inconsistent use of model verbose_name in permissions UI

• Do the tests still pass?[^1]
• Does the code comply with the style guide?
  • Run make lint from the Wagtail root.
• For Python changes: Have you added tests to cover the new/fixed behaviour?
• For front-end changes: Did you test on all of Wagtail’s supported environments?[^2]
  • Please list the exact browser and operating system versions you tested:
  • Please list which assistive technologies [^3] you tested:
• For new features: Has the documentation been updated accordingly?

[squash-labs]

Manage this branch in Squash

Test this branch here: https://nxpy123bug10982-permissions-vi-l2u0d.squash.io

[lb-]

Thanks @NXPY123 - This appears to work, I am unsure about hard-coding the re-addition of the Can label though. It does not appear to impact translations (did a test) but I will defer to maybe @loicteixeira or @zerolab for a second opinion on this.

Either way, this will need unit tests, would you be up for that also?

Validation

Using Italian intentionally for reference. It appears that our permission names are only showing in English so maybe the patching of the label is OK.

# bakerydemo/breads/models.py
class Country(models.Model):
    # ...
    class Meta:
        verbose_name = "Country of Origin" # added this line
        verbose_name_plural = "Countries of Origin"

Before

After

[lb-]

See #11203 for some initial work on this, it also added some tests.

I have also prepared a PR for bakerydemo to make this kind of issue easier to spot in the future wagtail/bakerydemo#478

[NXPY123]

Yes, I'll look into the unit test. Hardcoding the Can was based on @laymonage's suggestion although maybe it can be more generalised?

[lb-]

Ok Thank you, happy to go with the suggestion then.

[NXPY123]

I've made a test based on the other PR. I'm not sure if this is a proper test because the problem occurs when the verbose_name is changed and I don't think the verbose_name changes here

[lb-]

Ok. If the tests from the other PR are not correct, please ignore.

[lb-]

Looking good, just needs some improvements to the tests.

[lb-]

Thank you @NXPY123 - this is working well and thanks for working through the testing scenarios.

I will merge the test app migrations into one and rebase/force push.

Really appreciate you working through this and taking on the feedback & questions (even if my questions were a duplicate of those already answered, sorry).

Thanks also to @FatGuyy for the initial PR on this.

[NXPY123]

Thank you so much for taking the time to review it! 🙌
The feedback was really helpful and helped me make the necessary changes. Glad I could be of help!

[laymonage]

Thank you for giving this a go, and sorry I'm late, but this seems to cause unintended side effects especially when the codename is not in the form of verb_modelname. Could we revisit the approach please?

[laymonage]

Does permission_action.rsplit(maxsplit=1)[0] do anything here?

• Line 1: splitting by _ from the left with maxsplit=1, results in either a list of one string or a list of two strings.
• Line 2: if the first string lowercased is the string "can", use the second string instead. Otherwise use the first one. Not a big fan of accessing the list with boolean values False/True here, but at least it won't crash in either case.
  • This means if the codename is e.g. perform_some_action, only perform is taken. If the codename is e.g. can_perform_some_action, then perform_some_action is taken.
  • Case in point: the simpletranslation contrib module has the submit_translation codename. This results in just submit (thus displaying "Can submit"), where previously this displays "Can submit translation".
• Line 3: at this point permission_action is a string (derived from the codename based on previous lines). Doing .rsplit(maxsplit=1)[0] would try to split by whitespaces, but the codename is very unlikely to have any whitespace, so in most cases this would just result in a list containing the same string. Then we take the first element, which is the string itself.

I think we should only use the codename for detecting the common permissions (main_permission_names), but use something derived from the name for displaying the label.

[NXPY123]

Oh sorry about that I didn't notice it. I was looking to to do .rsplit("_",maxsplit=1) so if it's can_deliver_pizza it will only take deliver.

[lb-]

@NXPY123 new PRs are up from @laymonage that have a different approach here - see #11666 & related items.

Thanks both, my apologies on missing this case myself in the reviews.

[NXPY123]

Thank you for giving this a go, and sorry I'm late, but this seems to cause unintended side effects especially when the codename is not in the form of verb_modelname. Could we revisit the approach please?

Yes, I'm really sorry about that. Will setting .rsplit("_",maxsplit=1) work or do I have to try another approach?