checks: passes checks to draft edit & update to return latest run #3060
Conversation
0einstein0
changed the title
| CheckRun.query.filter( | ||
| CheckRun.config_id.in_(check_config_ids), CheckRun.record_id == record_uuid | ||
| ) | ||
| .order_by(CheckRun.is_draft.desc(), CheckRun.revision_id.desc()) |
There was a problem hiding this comment.
I have the same feeling here, as we had in the Checks component. We should be very explicit about which check runs we're fetching, i.e. for the record or draft. Either that, or we fetch them all and sort/split them out here in code (if e.g. we plan to be able to display them both). Doing "tricks" with sorting based on is_draft and revision_id can be confusing and faulty.
| @@ -209,6 +209,7 @@ def user_dashboard_request_view(request, **kwargs): | |||
| permissions=topic["permissions"], | |||
| is_preview=is_draft, # preview only when draft | |||
| is_draft=is_draft, | |||
| has_draft=record._record.has_draft, | |||
There was a problem hiding this comment.
Wouldn't is_draft actually work the same? We might also need is_published though (see zenodo/zenodo-rdm#1188 (comment) )
| errors = [] | ||
| record_uuid = current_rdm_records_service.draft_cls.pid.resolve( | ||
| pid_value, registered_only=False | ||
| ).id | ||
| request = draft.data.get("parent", {}).get("review") | ||
| if request and record_uuid: | ||
| checks = resolve_checks(record_uuid, request) or [] | ||
| errors = [ | ||
| err | ||
| for check in checks | ||
| for err in (check.result.get("errors") if check.result else []) | ||
| ] |
There was a problem hiding this comment.
This might not be needed anymore, since draft coming from read_draft(...) already has checks errors in it.
That means we can just pass errors=record.errors below?
|
|
||
| # Early exit if not draft submission nor record inclusion | ||
| request_type = request["type"] | ||
| is_draft_submission = request_type == CommunitySubmission.type_id |
There was a problem hiding this comment.
| is_draft_submission = request_type == CommunitySubmission.type_id | |
| is_draft_review = request_type == CommunitySubmission.type_id |
for naming consistency
| is_draft_submission = request_type == CommunitySubmission.type_id | ||
| is_record_inclusion = request_type == CommunityInclusion.type_id | ||
|
|
||
| if not is_draft_submission and not is_record_inclusion: |
There was a problem hiding this comment.
when would this be possible?
| return None | ||
|
|
||
| # Resolve the target community from the request if the community was not passed as an argument | ||
| if not community: |
There was a problem hiding this comment.
in which cases community will not be passed as an argument?
| communities.append(community.id) | ||
|
|
||
| # Early exit if no check config found for the communities | ||
| from invenio_checks.models import CheckConfig, CheckRun |
There was a problem hiding this comment.
why local import is needed?
There was a problem hiding this comment.
invenio-checks is not a dependency of invenio-app-rdm by default, so this is only accessed if you have set CHECKS_ENABLED = True. We can add the dependency now, since we're getting more production usage.
w:heart: Thank you for your contribution!
Description
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: