- 
                Notifications
    You must be signed in to change notification settings 
- Fork 172
chore: extract evaluate_record_deletion into utils #3201
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
base: master
Are you sure you want to change the base?
Conversation
ed7cb5f    to
    0cab183      
    Compare
  
    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.
LGTM, minor comments on the function args and their use.
0cab183    to
    1b21417      
    Compare
  
    1b21417    to
    6b4621c      
    Compare
  
    6b4621c    to
    e193a1e      
    Compare
  
    |  | ||
| def evaluate_record_deletion(record: RDMRecord): | ||
| """Evaluate whether a given record can be deleted by an identity.""" | ||
| rec_del = RDMRecordDeletionPolicy().evaluate(g.identity, record._record) | 
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.
same here for g.identity
| return list(existing_requests)[0] | ||
|  | ||
|  | ||
| def evaluate_record_deletion(record: RDMRecord): | 
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.
Is this the same changes as in the other PR (it might include the same commit)?
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.
yes sorry the other PR includes this PR and #3200 as I presume they could be merged first. sorry for the confusion
❤️ Thank you for your contribution!
Description
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Frontend
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: