Skip to content

#4036 Updated build list to include an alert state #5222

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 20, 2019
Merged

#4036 Updated build list to include an alert state #5222

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ba946b3
Updated build list to include an alert state
saadmk11 Feb 3, 2019
8609e94
Docstring issue fixed.
saadmk11 Feb 5, 2019
3736b94
Template filter name changed.
saadmk11 Feb 12, 2019
284cecf
Logic Moved to Models
saadmk11 Feb 13, 2019
4463c3f
Lint issue Fixed.
saadmk11 Feb 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions media/css/core.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,14 @@ p.build-missing { font-size: .8em; color: #9d9a55; margin: 0 0 3px; }
#footer label { color: #BCC1C3; font-weight: normal; }
#footer input[type="text"], #footer input[type="email"] { padding: 4px; font-size: 12px; line-height: 16px; margin-bottom: 5px }

/* Warning Icon for Build List triggered */
.module-item.col-span a span.icon-warning:before {
font-family: FontAwesome;
font-size: .9em;
padding-right: .3em;
font-weight: normal;
content: "\f071";
}

/* utils */

Expand Down
8 changes: 8 additions & 0 deletions readthedocs/core/templatetags/core_tags.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

"""Template tags for core app."""

import datetime
import hashlib
from urllib.parse import urlencode

from django import template
from django.conf import settings
from django.utils import timezone
from django.utils.encoding import force_bytes, force_text
from django.utils.safestring import mark_safe

Expand Down Expand Up @@ -112,3 +114,9 @@ def key(d, key_name):
@register.simple_tag
def readthedocs_version():
return __version__


@register.simple_tag
def get_past_time(mins):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about naming this as minutes_ago?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused about what to call it. Thanks for the suggestion. I will change it as soon as possible.

"""Subtracts provided minutes from current time."""
return timezone.now() - datetime.timedelta(minutes=int(mins))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tend to avoid putting logic in templates like this. It should probably be a property defined on the models. A property method like Build.is_stale would be useful outside of templates. We might eventually have to surface this in our API when we make this an API driven page, but for now, the build list page is tied to the database models directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! For easier maintenance, we keep logic out of templates and put them on models instead, so this logic can be moved to the Build model. The rest of the PR looks , but I'm sure will change a bit with the move of this logic.

@agjohnson Thanks for your review. I have committed the changes you asked for. Please have a look and let me know if any further changes are required.

6 changes: 5 additions & 1 deletion readthedocs/templates/core/build_list_detailed.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
{% load i18n %}
{% load static %}
{% load core_tags %}

{% get_past_time 5 as 5_mins_ago %}

{% for build in build_qs %}
<li class="module-item col-span">
<div id="build-{{ build.id }}">
<a href="{{ build.get_absolute_url }}"><span id="build-state">{% if build.state != 'finished' %}{{ build.get_state_display }} {% else %} {% if build.success %}{% trans "Passed" %}{% else %}{% trans "Failed" %}{% endif %}{% endif %}</span>
<a href="{{ build.get_absolute_url }}">{% if build.state == 'triggered' and build.date < 5_mins_ago %}
<span class="icon-warning" title="{% trans 'This build is still waiting to be built' %}"></span>{% endif %}<span id="build-state">{% if build.state != 'finished' %}{{ build.get_state_display }} {% else %} {% if build.success %}{% trans "Passed" %}{% else %}{% trans "Failed" %}{% endif %}{% endif %}</span>
<img src="{% static 'core/img/loader.gif' %}" class="build-loading hide">
<span class="quiet">{% if build.version %}{% blocktrans with build.version.slug as slug and build.type as type %}version {{ slug }} ({{ type }}){% endblocktrans %}{% endif %}</span><span class="quiet right">{% blocktrans with build.date|timesince as date %}{{ date }} ago{% endblocktrans %}</span>
</a>
Expand Down