-
Notifications
You must be signed in to change notification settings - Fork 8
views: checks requests tab templates #29
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
views: checks requests tab templates #29
Conversation
daecbac
to
310426b
Compare
819c390
to
f97823a
Compare
f97823a
to
38925db
Compare
{# | ||
Rule descriptions can contain HTML to link to a page with more details about the rule. | ||
This field is sanitized in the backend with SanitizedHTML. | ||
#} | ||
<div class="pt-5 text-muted">{{ rule_result.rule_description | safe }}</div> |
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.
The backend sanitization and the React conterpart in the form are implemented in zenodo/zenodo-rdm#1135
{%if rule_result.success %} | ||
{% set ns.rule_severity_level = "success" %} | ||
{% elif rule_result.level == "info" %} | ||
{% set ns.rule_severity_level = "warning" %} | ||
{% elif rule_result.level == "error" %} | ||
{% set ns.rule_severity_level = "error" %} | ||
{% else %} | ||
{% set ns.rule_severity_level = "unknown" %} |
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.
why not use it directly?
{%if rule_result.success %} | |
{% set ns.rule_severity_level = "success" %} | |
{% elif rule_result.level == "info" %} | |
{% set ns.rule_severity_level = "warning" %} | |
{% elif rule_result.level == "error" %} | |
{% set ns.rule_severity_level = "error" %} | |
{% else %} | |
{% set ns.rule_severity_level = "unknown" %} | |
{% set ns.rule_severity_level = rule_result.level %} |
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 not super obvious, this is why I'm explaining it in the comment above:
This code depends on 2 fields of each `rule_results` : `success` and `level`. If the boolean `success` field is true, it means that the check was successful. Otherwise, it means that the check failed, and we display the severity `level` of the check.
invenio_checks/templates/semantic-ui/invenio_checks/requests/details.html
Outdated
Show resolved
Hide resolved
invenio_checks/templates/semantic-ui/invenio_checks/requests/details.html
Outdated
Show resolved
Hide resolved
invenio_checks/templates/semantic-ui/invenio_checks/requests/overall_severity_level.html
Outdated
Show resolved
Hide resolved
|
||
{% set ns = namespace(overall_severity_level="success") %} | ||
|
||
{% for error in checks.result.errors %} |
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 part should be computed on the backend in the CheckRun
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'm running out of time, so I added a FIXME for now :(
invenio_checks/templates/semantic-ui/invenio_checks/requests/details.html
Outdated
Show resolved
Hide resolved
invenio_checks/templates/semantic-ui/invenio_checks/requests/title.html
Outdated
Show resolved
Hide resolved
a1de56e
to
d2df185
Compare
Just auto-squashed before merging. |
d2df185
to
7375da6
Compare
❤️ Thank you for your contribution!
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: