component: run checks on publish and update runs table #40
Conversation
invenio_checks/components.py
Outdated
| latest_check = ( | ||
| CheckRun.query.filter( | ||
| CheckRun.config_id == check_config.id, | ||
| CheckRun.record_id == record.id, | ||
| CheckRun.is_draft.is_(True), | ||
| CheckRun.is_draft.is_(record.is_draft), |
There was a problem hiding this comment.
See above, but this becomes especially confusing now. Is record a record or draft? We shouldn't rely on calling record.is_draft, it should be obvious/readable from the context we're in.
invenio_checks/components.py
Outdated
| def publish(self, identity, data=None, record=None, errors=None, **kwargs): | ||
| """Run checks on publish.""" | ||
| if errors is None: | ||
| errors = [] | ||
|
|
||
| self._run_checks(identity, data, record, errors) |
There was a problem hiding this comment.
I'm not a big fan of the fact that we have one function that "does it all". Can you have a go at splitting _run_checks into something we can also start reusing e.g. in the requests "Checks" tab and/or the Request overrides we have in Zenodo for running checks on inclusion requests?
It might be that we actually don't want these to be in this component, but on an API or Service layer of checks.
invenio_checks/components.py
Outdated
| CheckRun.query.filter_by( | ||
| config_id=check_config.id, record_id=record.id, is_draft=True | ||
| ).delete() |
There was a problem hiding this comment.
| CheckRun.query.filter_by( | |
| config_id=check_config.id, record_id=record.id, is_draft=True | |
| ).delete() | |
| CheckRun.query.filter_by(record_id=record.id, is_draft=True).delete() |
If we refactor this big function, we could just run the suggested version in the publish(...) handler. Even better we could also run an UDPATE to convert is_draft=True check runs to is_draft=False beforehand.
invenio_checks/components.py
Outdated
| def publish(self, identity, draft=None, record=None): | ||
| """Run checks on publish.""" | ||
| CheckRunAPI.delete_check_run(record_uuid=record.id, is_draft=False) | ||
| CheckRunAPI.run_checks(identity, is_draft=False, record=record) |
There was a problem hiding this comment.
Need to test it again, it is not updating correctly now
|
There is some issue with new draft save not showing errors |
❤️ Thank you for your contribution!
Description
Depends on #33 to be merged first
Partially closes inveniosoftware/invenio-rdm-records#2051
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: