Skip to content
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

Implementation of new notification system #254

Merged
merged 11 commits into from
Jan 4, 2024
14 changes: 11 additions & 3 deletions readthedocsext/theme/templates/builds/build_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
</div>
</div>

{# TODO: remove this `build.output block completely #}
{% if build.output %}
{# If we have build output, this is an old build #}
<p>
Expand Down Expand Up @@ -187,11 +188,18 @@ <h3>{% trans "Build Errors" %}</h3>
{% comment %}
Error list
{% endcomment %}
<div class="ui attached inverted red segment" data-bind="visible: error" style="display: none;">
{% if build.notifications.exists %}
<div class="ui attached inverted red segment">
<i class="fa-solid fa-exclamation circular icon"></i>
<span data-bind="text: error"></span>
{# Show error notifications as "build-error" #}
humitos marked this conversation as resolved.
Show resolved Hide resolved
{% for notification in build.notifications.all %}
{% if notification.get_message.type == "error" %}
<span>{{ notification.get_message.get_rendered_body|safe }}</span>
humitos marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% endfor %}
humitos marked this conversation as resolved.
Show resolved Hide resolved
<span>{% trans "For more information on this error, see our documentation." %}
</div>
</div>
{% endif %}

<div class="ui inverted bottom attached segment transition slide">

Expand Down
18 changes: 18 additions & 0 deletions src/js/build/detail.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ export class BuildDetailView {
/** @observable {Boolean} There was doc output in the build */
this.can_view_docs = ko.observable(false);

this.poll_api_counts = 0;

// Consolidate all of the observable updates that depend on build state
this.state.subscribe((state) => {
this.update_state(state);
Expand Down Expand Up @@ -413,6 +415,9 @@ export class BuildDetailView {
this.state(data.state);
this.state_display(data.state_display);

this.poll_api_counts = this.poll_api_counts + 1;


// This is a mock command used to preview the command output.
// TODO probably do this in the application instead
this.add_command({
Expand All @@ -438,6 +443,19 @@ export class BuildDetailView {
// this update happens at the very end of API updates instead.
if (this.is_finished()) {
this.is_polling(false);

console.log(this.poll_api_counts);

humitos marked this conversation as resolved.
Show resolved Hide resolved

// HACK: this is a small hack to avoid implementing the new notification system
// on ext-theme at this point. This will come in a future PR.
// The notifications are rendered properly via Django template in a static way.
// So, we refresh the page once the build has finished to make Django render the notifications.
// We use a check of 1 API poll for those builds that are already finished when opened.
// The new dashboard will implement the new notification system in a nicer way using APIv3.
if (this.poll_api_counts !== 1) {
location.reload();
}
} else {
setTimeout(() => {
this.poll_api();
Expand Down