-
Notifications
You must be signed in to change notification settings - Fork 40
feat(requests): Add conversation locking functionality #518
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
feat(requests): Add conversation locking functionality #518
Conversation
d7c6467 to
b4823e1
Compare
b4823e1 to
f6dff91
Compare
| def update(self, identity, data=None, record=None, uow=None, **kwargs): | ||
| """Update the reviewers of a request.""" | ||
| if "reviewers" in data: | ||
| if data.get("reviewers", None): |
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.
This should not change because it adds a regression when we remove reviewers
| """Condition to choose generators set.""" | ||
| # If the event type is a log event, we don't need to check if the request is locked | ||
| # Because it's not a comment event and we don't want to block logs | ||
| if event_type and event_type == LogEventType: |
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.
I find this odd here and I know that it comes from the fact that the events_service.create assumes permission based on adding comments and not log events. Isnt it better to change the check of permission on the service method to explicitely check the event.type and apply correct permissions? @slint wdyt?
d4d0947 to
52716be
Compare
| self.require_permission(identity, "lock_request", request=request._record) | ||
|
|
||
| if request.data["is_locked"]: | ||
| raise RequestLockedError(description="The request is already locked.") |
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.
translations?
| self.require_permission(identity, "lock_request", request=request._record) | ||
|
|
||
| if not request.data["is_locked"]: | ||
| raise RequestLockedError(description="The request is already unlocked.") |
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.
translations?
d1d10b9 to
e033375
Compare
e033375 to
35e0200
Compare
closes: #498