Allow to hook the initial build from outside (#4033)

* Allow to hook the initial build from outside

Add a new attribute to `trigger_build` to do not execute the task but
just return the immutable signature of it, so it can be chained into a
bigger task from outside when it's needed to do something else
before/after it's execution.

* Remove unused basic attribute on trigger build

* Split trigger_build to be able to prepare_build first

Added a ``ImportWizard.trigger_initial_build`` method to be override
from corporate site.

Also, instead of using ``trigger_build`` to create the Celery task and
call it immediately, split it to use ``prepare_build`` first to create
the task and use ``trigger_build`` to effectively triggers it.

* Remove now unused signal

* Fix Celery signature creation

* Fix testcases with basic on trigger_build

* Fix mock calls in test cases

* Fix test mock check

* Use async task to attach webhook

* Use proper context object

* Make it compatible with newer Django versions

* Py2 and Py3 compatible line

* Dismiss the sticky message and stay in the same page

* Use Context to render the template

* Lint errors fixed

* Call the task in a sync way properly

* Update common submodule to latest

* Define NonPersistentStorage for one-time notifications

NonPersistentNotification backed together with SiteNotification class
helps us to create one-time site notifications and attach it to a user
without the needs of a request/session object.

The message/notification is saved into the database as a
PersistentMessage and it's marked as read while it's retrived the
first time.

This system is useful for notifications that need to be attached from
a Celery task where we know the user but we don't have the request.

* Make string translatable

* Fix merge conflicts

* Recover accidentally deleted line

* Fixed linting errors

* Replace message_key with reason to be clearer

* Rename non persistent storage class

* Remove old templates

* Make SiteNotification more flexible

- render strings messages as Django Templates
- accept extra_context for the template
- do not crash if the reason is incorrect

* Adapt AttachWebhookNotification to the new features

* Refactor the task to attach a webhook

Instantiate only once the notification and adapt it depending the
context.

Also, if there are no connected services into our application do not
show a message to the user, but log it as a warning.

* Test cases for SiteNotification and NonPersistentStorage

* Remove unnecessary lines

* Show a persistent message for invalid project webhook

If we can setup a valid webhook, we show a persistent message with an
actionable link using our notifications abstraction.

At this point, the message is duplicated because we have a "fixed
template message" also which is planned to be removed soon.

* Improve copy

* Allow to hook the initial build from outside

Add a new attribute to `trigger_build` to do not execute the task but
just return the immutable signature of it, so it can be chained into a
bigger task from outside when it's needed to do something else
before/after it's execution.

* Remove unused basic attribute on trigger build

* Split trigger_build to be able to prepare_build first

Added a ``ImportWizard.trigger_initial_build`` method to be override
from corporate site.

Also, instead of using ``trigger_build`` to create the Celery task and
call it immediately, split it to use ``prepare_build`` first to create
the task and use ``trigger_build`` to effectively triggers it.

* Remove now unused signal

* Fix Celery signature creation

* Fix testcases with basic on trigger_build

* Fix mock calls in test cases

* Fix test mock check

* Use proper context object

* Make it compatible with newer Django versions

* Py2 and Py3 compatible line

* Dismiss the sticky message and stay in the same page

* Use Context to render the template

* Lint errors fixed

* Call the task in a sync way properly

* Update common submodule to latest

* Define NonPersistentStorage for one-time notifications

NonPersistentNotification backed together with SiteNotification class
helps us to create one-time site notifications and attach it to a user
without the needs of a request/session object.

The message/notification is saved into the database as a
PersistentMessage and it's marked as read while it's retrived the
first time.

This system is useful for notifications that need to be attached from
a Celery task where we know the user but we don't have the request.

* Make string translatable

* Use async task to attach webhook

* Fix merge conflicts

* Recover accidentally deleted line

* Fixed linting errors

* Replace message_key with reason to be clearer

* Rename non persistent storage class

* Remove old templates

* Make SiteNotification more flexible

- render strings messages as Django Templates
- accept extra_context for the template
- do not crash if the reason is incorrect

* Adapt AttachWebhookNotification to the new features

* Refactor the task to attach a webhook

Instantiate only once the notification and adapt it depending the
context.

Also, if there are no connected services into our application do not
show a message to the user, but log it as a warning.

* Test cases for SiteNotification and NonPersistentStorage

* Remove unnecessary lines

* Show a persistent message for invalid project webhook

If we can setup a valid webhook, we show a persistent message with an
actionable link using our notifications abstraction.

At this point, the message is duplicated because we have a "fixed
template message" also which is planned to be removed soon.

* Improve copy

* Remove fixed template notification about Project.has_valid_webhook

media/css/core.css

@@ -610,7 +610,9 @@ p.build-missing { font-size: .8em; color: #9d9a55; margin: 0 0 3px; }
 /* notification box */
 .notification { padding: 5px 0; color: #a55; }
 .notification-20,
-.notification-25 {
+.notification-25,
+.notification-101,
+.notification-102 {
   color: #5a5;
 }
 

readthedocs/core/utils/__init__.py

@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 """Common utilty functions."""
 
 from __future__ import absolute_import
@@ -16,7 +17,7 @@
 from future.backports.urllib.parse import urlparse
 from celery import group, chord
 
-from readthedocs.builds.constants import LATEST
+from readthedocs.builds.constants import LATEST, BUILD_STATE_TRIGGERED
 from readthedocs.doc_builder.constants import DOCKER_LIMITS
 
 
@@ -75,38 +76,47 @@ def cname_to_slug(host):
     return slug
 
 
-def trigger_build(project, version=None, record=True, force=False, basic=False):
+def prepare_build(
+        project, version=None, record=True, force=False, immutable=True):
     """
-    Trigger build for project and version.
+    Prepare a build in a Celery task for project and version.
 
-    If project has a ``build_queue``, execute task on this build queue. Queue
-    will be prefixed with ``build-`` to unify build queue names.
+    If project has a ``build_queue``, execute the task on this build queue. If
+    project has ``skip=True``, the build is not triggered.
+
+    :param project: project's documentation to be built
+    :param version: version of the project to be built. Default: ``latest``
+    :param record: whether or not record the build in a new Build object
+    :param force: build the HTML documentation even if the files haven't changed
+    :param immutable: whether or not create an immutable Celery signature
+    :returns: Celery signature of UpdateDocsTask to be executed
     """
     # Avoid circular import
     from readthedocs.projects.tasks import UpdateDocsTask
     from readthedocs.builds.models import Build
 
     if project.skip:
+        log.info(
+            'Build not triggered because Project.skip=True: project=%s',
+            project.slug,
+        )
         return None
 
     if not version:
         version = project.versions.get(slug=LATEST)
 
-    kwargs = dict(
-        pk=project.pk,
-        version_pk=version.pk,
-        record=record,
-        force=force,
-        basic=basic,
-    )
+    kwargs = {
+        'version_pk': version.pk,
+        'record': record,
+        'force': force,
+    }
 
-    build = None
     if record:
         build = Build.objects.create(
             project=project,
             version=version,
             type='html',
-            state='triggered',
+            state=BUILD_STATE_TRIGGERED,
             success=True,
         )
         kwargs['build_pk'] = build.pk
@@ -121,16 +131,44 @@ def trigger_build(project, version=None, record=True, force=False, basic=False):
         if project.container_time_limit:
             time_limit = int(project.container_time_limit)
     except ValueError:
-        pass
+        log.warning('Invalid time_limit for project: %s', project.slug)
+
     # Add 20% overhead to task, to ensure the build can timeout and the task
     # will cleanly finish.
     options['soft_time_limit'] = time_limit
     options['time_limit'] = int(time_limit * 1.2)
 
-    update_docs = UpdateDocsTask()
-    update_docs.apply_async(kwargs=kwargs, **options)
+    update_docs_task = UpdateDocsTask()
+
+    # Py 2.7 doesn't support ``**`` expand syntax twice. We create just one big
+    # kwargs (including the options) for this and expand it just once.
+    # return update_docs_task.si(project.pk, **kwargs, **options)
+    kwargs.update(options)
+
+    return update_docs_task.si(project.pk, **kwargs)
 
-    return build
+
+def trigger_build(project, version=None, record=True, force=False):
+    """
+    Trigger a Build.
+
+    Helper that calls ``prepare_build`` and just effectively trigger the Celery
+    task to be executed by a worker.
+
+    :param project: project's documentation to be built
+    :param version: version of the project to be built. Default: ``latest``
+    :param record: whether or not record the build in a new Build object
+    :param force: build the HTML documentation even if the files haven't changed
+    :returns: Celery AsyncResult promise
+    """
+    update_docs_task = prepare_build(
+        project,
+        version,
+        record,
+        force,
+        immutable=True,
+    )
+    return update_docs_task.apply_async()
 
 
 def send_email(recipient, subject, template, template_html, context=None,

readthedocs/notifications/__init__.py

@@ -12,11 +12,12 @@
 .. _`django-messages-extends`: https://github.com
                                /AliLozano/django-messages-extends/
 """
-from .notification import Notification
+from .notification import Notification, SiteNotification
 from .backends import send_notification
 
 __all__ = (
     'Notification',
+    'SiteNotification',
     'send_notification'
 )
 

readthedocs/notifications/backends.py

@@ -32,7 +32,11 @@ def send_notification(request, notification):
     backends = getattr(settings, 'NOTIFICATION_BACKENDS', [])
     for cls_name in backends:
         backend = import_string(cls_name)(request)
-        backend.send(notification)
+        # Do not send email notification if defined explicitly
+        if backend.name == EmailBackend.name and not notification.send_email:
+            pass
+        else:
+            backend.send(notification)
 
 
 class Backend(object):
@@ -96,8 +100,16 @@ def send(self, notification):
         req = HttpRequest()
         setattr(req, 'session', '')
         storage = cls(req)
+
+        # Use the method defined by the notification or map a simple level to a
+        # persistent one otherwise
+        if hasattr(notification, 'get_message_level'):
+            level = notification.get_message_level()
+        else:
+            level = LEVEL_MAPPING.get(notification.level, INFO_PERSISTENT)
+
         storage.add(
-            level=LEVEL_MAPPING.get(notification.level, INFO_PERSISTENT),
+            level=level,
             message=notification.render(
                 backend_name=self.name,
                 source_format=HTML,

readthedocs/notifications/constants.py

@@ -18,3 +18,20 @@
     REQUIREMENT: message_constants.WARNING_PERSISTENT,
     ERROR: message_constants.ERROR_PERSISTENT,
 }
+
+
+# Message levels to save the message into the database and mark as read
+# immediately after retrieved (one-time shown message)
+DEBUG_NON_PERSISTENT = 100
+INFO_NON_PERSISTENT = 101
+SUCCESS_NON_PERSISTENT = 102
+WARNING_NON_PERSISTENT = 103
+ERROR_NON_PERSISTENT = 104
+
+NON_PERSISTENT_MESSAGE_LEVELS = (
+    DEBUG_NON_PERSISTENT,
+    INFO_NON_PERSISTENT,
+    SUCCESS_NON_PERSISTENT,
+    WARNING_NON_PERSISTENT,
+    ERROR_NON_PERSISTENT,
+)

readthedocs/notifications/notification.py

@@ -1,16 +1,22 @@
+# -*- coding: utf-8 -*-
 """Support for templating of notifications."""
 
 from __future__ import absolute_import
 from builtins import object
+import logging
 from django.conf import settings
 from django.template import Template, Context
 from django.template.loader import render_to_string
 from django.db import models
+from django.http import HttpRequest
 
 from .backends import send_notification
 from . import constants
 
 
+log = logging.getLogger(__name__)
+
+
 class Notification(object):
 
     """
@@ -28,6 +34,7 @@ class Notification(object):
     level = constants.INFO
     subject = None
     user = None
+    send_email = True
 
     def __init__(self, context_object, request, user=None):
         self.object = context_object
@@ -45,8 +52,8 @@ def get_context_data(self):
             self.context_object_name: self.object,
             'request': self.request,
             'production_uri': '{scheme}://{host}'.format(
-                scheme='https', host=settings.PRODUCTION_DOMAIN
-            )
+                scheme='https', host=settings.PRODUCTION_DOMAIN,
+            ),
         }
 
     def get_template_names(self, backend_name, source_format=constants.HTML):
@@ -71,7 +78,7 @@ def render(self, backend_name, source_format=constants.HTML):
                 backend_name=backend_name,
                 source_format=source_format,
             ),
-            context=Context(self.get_context_data()),
+            context=self.get_context_data(),
         )
 
     def send(self):
@@ -84,3 +91,92 @@ def send(self):
         avoided.
         """
         send_notification(self.request, self)
+
+
+class SiteNotification(Notification):
+
+    """
+    Simple notification to show *only* on site messages.
+
+    ``success_message`` and ``failure_message`` can be a simple string or a
+    dictionary with different messages depending on the reason of the failure /
+    success. The message is selected by using ``reason`` to get the proper
+    value.
+
+    The notification is tied to the ``user`` and it could be sticky, persistent
+    or normal --this depends on the ``success_level`` and ``failure_level``.
+
+    .. note::
+
+        ``send_email`` is forced to False to not send accidental emails when
+        only a simple site notification is needed.
+    """
+
+    send_email = False
+
+    success_message = None
+    failure_message = None
+
+    success_level = constants.SUCCESS_NON_PERSISTENT
+    failure_level = constants.ERROR_NON_PERSISTENT
+
+    def __init__(
+            self, user, success, reason=None, context_object=None,
+            request=None, extra_context=None):
+        self.object = context_object
+
+        self.user = user or request.user
+        # Fake the request in case the notification is instantiated from a place
+        # without access to the request object (Celery task, for example)
+        self.request = request or HttpRequest()
+        self.request.user = user
+
+        self.success = success
+        self.reason = reason
+        self.extra_context = extra_context or {}
+        super(SiteNotification, self).__init__(context_object, request, user)
+
+    def get_context_data(self):
+        context = super(SiteNotification, self).get_context_data()
+        context.update(self.extra_context)
+        return context
+
+    def get_message_level(self):
+        if self.success:
+            return self.success_level
+        return self.failure_level
+
+    def get_message(self, success):
+        if success:
+            message = self.success_message
+        else:
+            message = self.failure_message
+
+        msg = ''  # default message in case of error
+        if isinstance(message, dict):
+            if self.reason:
+                if self.reason in message:
+                    msg = message.get(self.reason)
+                else:
+                    # log the error but not crash
+                    log.error(
+                        "Notification {} has no key '{}' for {} messages".format(
+                            self.__class__.__name__,
+                            self.reason,
+                            'success' if self.success else 'failure',
+                        ),
+                    )
+            else:
+                log.error(
+                    '{}.{}_message is a dictionary but no reason was provided'.format(
+                        self.__class__.__name__,
+                        'success' if self.success else 'failure',
+                    ),
+                )
+        else:
+            msg = message
+
+        return Template(msg).render(context=Context(self.get_context_data()))
+
+    def render(self, *args, **kwargs):  # pylint: disable=arguments-differ
+        return self.get_message(self.success)