Skip to content

Modify AND and OR permission operands to have the ability to use custom messages #6502

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

Closed
wants to merge 6 commits into from

Conversation

samitnuk
Copy link

@samitnuk samitnuk commented Mar 9, 2019

refs #6427

  • AND - If failed permission has a message, it will be returned.
  • OR - If both permissions will be failed:
    a) if both permissions have messages - will be returned one message combined from two messages;
    b) if only one permission (from both failed) has a message - this message will be returned (I suppose this is better then return the default one).
  • NOT - If failed permission has a message_inverted attribute, it will be returned. (@iproha94 thanks for this suggestion).

@samitnuk samitnuk force-pushed the issue/6427 branch 2 times, most recently from bcffbac to 1d77243 Compare December 7, 2019 10:19
@samitnuk
Copy link
Author

samitnuk commented Dec 7, 2019

@ozandogrultan Many thanks for your comments. I have updated this PR.

@iproha94
Copy link

@samitnuk wrote

NOT - untouched here since in any case, we cannot use the message from "inverted" permission.

I think for this case, we should add the inverted_message property to the custom permission class, so

class MyCustomPermission(permissions.BasePermission):
    message = _("you are not active")
    inverted_message = _("you are active")

and use it for rest_framework/permissions.py:

class NOT:
    def __init__(self, op1):
        self.op1 = op1
        self.message = getattr(self.op1, 'inverted_message', None)

but this is incorrect for double negative.

@samitnuk
Copy link
Author

Hi, @iproha94. Thank you for your review. I will check your comments and make updates asap.

@samitnuk samitnuk force-pushed the issue/6427 branch 2 times, most recently from ab99187 to 12e0009 Compare April 13, 2020 15:46
@samitnuk
Copy link
Author

@ozandogrultan, @auvipy, @iproha94 please review the updated PR. Many thanks.

@DrJfrost
Copy link

Is this being merged?

@tony-garcia
Copy link

Any plans to to get this merged??

@Riccardo-Maio
Copy link

I am running into this issue currently. Is this in the roadmap to get merged?

@stale
Copy link

stale bot commented Apr 18, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 18, 2022
@auvipy auvipy removed the stale label Jan 5, 2023
@MartinSchmidt123
Copy link

Same issue here.
I like the solution and would be happy to see this merged.

@auvipy auvipy closed this Mar 14, 2023
@auvipy auvipy reopened this Mar 14, 2023
@auvipy
Copy link
Member

auvipy commented Mar 14, 2023

let see how it works on current CI

@auvipy auvipy self-requested a review March 14, 2023 18:24
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

I think this enhancement also needs documentation updates

@stale
Copy link

stale bot commented Jun 11, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

Successfully merging this pull request may close these issues.

9 participants