Adjust how unique an admin action can be #233
Conversation
This is needed if for example a request was refused on grounds that changed.
@@ -1712,13 +1712,25 @@ def add_new_branch_request(session, pkg_name, clt_to, user): | |||
if clt_to.name == 'Fedora EPEL': | |||
_validate_pkg(session, clt_to.version, package.name) | |||
|
|||
action = model.AdminAction( | |||
action = model.AdminAction.search( |
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.
.search
returns a collection of actions? This should be actions = model.AdminAction.search(...
to make that clear.
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.
In this case it can only return 1 action as I am passing all the element that constitute the unique constraint, so it can only ever be a list of 1 item.
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.
It's still a list.
I saw action.pop()
and thought "wtf. An AdminAction
has a .pop()
method?"
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.
ok
Other than the cosmetic note above, 👍. Looks good! |
Adjust how unique an admin action can be
We cannot make unique an admin action based on its package, branch, user, action and status
otherwise we end up with a request closed, another open that cannot be closed with the same
status (unique constraints will fail).
What we can do otherwise is check if we have a request for that package on that branch and for this user and if so, we reset it.
This PR also adds a link to a request once it has been closed so that it can be sent to someone.