diff --git a/docs/manual/extending/coderef/index.rst b/docs/manual/extending/coderef/index.rst index 6677bad8335f802021a31316ea29be5bf9d086ff..9fac17d510ddf922be7fd3c3ae0c48706aa66160 100644 --- a/docs/manual/extending/coderef/index.rst +++ b/docs/manual/extending/coderef/index.rst @@ -126,8 +126,10 @@ Review Requests and Reviews .. autosummary:: :toctree: python + reviewboard.reviews.actions reviewboard.reviews.chunk_generators reviewboard.reviews.context + reviewboard.reviews.default_actions reviewboard.reviews.errors reviewboard.reviews.fields reviewboard.reviews.forms diff --git a/docs/manual/extending/extensions/hooks/action-hook.rst b/docs/manual/extending/extensions/hooks/action-hook.rst new file mode 100644 index 0000000000000000000000000000000000000000..0460a88a3c2ff6283719abd749ea6670f2014288 --- /dev/null +++ b/docs/manual/extending/extensions/hooks/action-hook.rst @@ -0,0 +1,189 @@ +.. _action-hook: + +========== +ActionHook +========== + +Extensions can make use of a variety of action hooks in order to inject +clickable actions into various parts of the UI. + +The :py:mod:`reviewboard.extensions.hooks` module contains the following hooks: + +.. autosummary:: + + ~reviewboard.extensions.hooks.ActionHook + ~reviewboard.extensions.hooks.BaseReviewRequestActionHook + ~reviewboard.extensions.hooks.ReviewRequestActionHook + ~reviewboard.extensions.hooks.DiffViewerActionHook + ~reviewboard.extensions.hooks.HeaderActionHook + +When instantiating any of these hooks, we can pass in a list of dictionaries +that define the actions that we'd like to insert. These dictionaries must have +the following keys: + +``id`` (optional): + The ID of the action. + +``label``: + The label for the action. + +``url``: + The URL to invoke when the action is clicked. + + If we want to invoke a JavaScript action, then this should be ``#``, and + there should be a selector on the ``id`` field to attach the handler (as + opposed to a ``javascript:`` URL, which doesn't work on all browsers). + +``image`` (optional): + The path to the image used for the icon. + +``image_width`` (optional): + The width of the image. + +``image_height`` (optional): + The height of the image. + +.. versionadded:: 2.6 + + The :py:class:`~.hooks.BaseReviewRequestActionHook` class was added. Also, + instead of passing in a list of dictionaries, we instead recommend passing + in a list of :py:class:`~.actions.BaseReviewRequestAction` instances. + +.. seealso:: The :py:mod:`reviewboard.reviews.actions` module. + +There are also two hooks that can provide dropdown menus: + +.. autosummary:: + + ~reviewboard.extensions.hooks.ReviewRequestDropdownActionHook + ~reviewboard.extensions.hooks.HeaderDropdownActionHook + +These work like the basic action hooks, except instead of a ``url`` field, they +contain an ``items`` field which is another list of +:py:class:`~.ActionHook`-style dictionaries. + +.. versionadded:: 2.6 + + Up to two levels of action nesting are now possible. Also, instead of + passing in a list of dictionaries, we instead recommend passing in a list of + :py:class:`~.actions.BaseReviewRequestMenuAction` instances. + +.. seealso:: The :py:mod:`reviewboard.reviews.actions` module. + + +Modifying the Default Actions +============================= + +.. versionadded:: 2.6 + +The :py:mod:`reviewboard.reviews.actions` module provides two useful methods +for working with default review request actions: + +.. autosummary:: + + ~reviewboard.reviews.actions.register_actions + ~reviewboard.reviews.actions.unregister_actions + +.. seealso:: The :py:mod:`reviewboard.reviews.default_actions` module. + + +Example +======= + +.. code-block:: python + + from reviewboard.extensions.base import Extension + from reviewboard.extensions.hooks import (BaseReviewRequestActionHook, + HeaderDropdownActionHook, + ReviewRequestActionHook) + from reviewboard.reviews.actions import (BaseReviewRequestAction, + BaseReviewRequestMenuAction, + register_actions, + unregister_actions) + + + class NewCloseAction(BaseReviewRequestAction): + action_id = 'new-close-action' + label = 'New Close Action!' + + + class SampleMenuAction(BaseReviewRequestMenuAction): + action_id = 'sample-menu-action' + label = 'Sample Menu' + + + class FirstItemAction(BaseReviewRequestAction): + action_id = 'first-item-action' + label = 'First Item' + + + class SampleSubmenuAction(BaseReviewRequestMenuAction): + action_id = 'sample-submenu-action' + label = 'Sample Submenu' + + + class SubItemAction(BaseReviewRequestAction): + action_id = 'sub-item-action' + label = 'Sub Item' + + + class LastItemAction(BaseReviewRequestAction): + action_id = 'last-item-action' + label = 'Last Item' + + + class SampleExtension(Extension): + def initialize(self): + # Register a new action in the Close menu. + register_actions([NewCloseAction()], 'close-review-request-action') + + # Register a new review request action that only appears if the user + # is on a review request page. + ReviewRequestActionHook(self, actions=[ + { + 'id': 'foo-item-action', + 'label': 'Foo Item', + 'url': '#', + }, + ]) + + # Register a new dropdown menu action (with two levels of nesting) + # that appears if the user is on a review request page, a file + # attachment page, or a diff viewer page. + BaseReviewRequestActionHook(self, actions=[ + SampleMenuAction([ + FirstItemAction(), + SampleSubmenuAction([ + SubItemAction(), + ]), + LastItemAction(), + ]), + ]) + + # Add a dropdown in the header that links to other pages. + HeaderDropdownActionHook(self, actions=[ + { + 'label': 'Sample Header Dropdown', + 'items': [ + { + 'label': 'Item 1', + 'url': '#', + }, + { + 'label': 'Item 2', + 'url': '#', + }, + ], + }, + ]) + + def shutdown(self): + super(SampleExtension, self).shutdown() + + # Restore everything back to the original state by unregistering all + # of the custom review request actions that were registered. + unregister_actions([ + NewCloseAction.action_id, + 'foo-item-action', + SampleMenuAction.action_id, + ]) diff --git a/docs/manual/extending/extensions/hooks/action-hooks.rst b/docs/manual/extending/extensions/hooks/action-hooks.rst deleted file mode 100644 index 6633dce6d701e199c7356e71cd50056706564a8d..0000000000000000000000000000000000000000 --- a/docs/manual/extending/extensions/hooks/action-hooks.rst +++ /dev/null @@ -1,102 +0,0 @@ -.. _action-hooks: - -============ -Action Hooks -============ - -There are a variety of action hooks, which allow injecting clickable actions -into various parts of the UI. - -:py:mod:`reviewboard.extensions.hooks` contains the following hooks: - -+-------------------------------------+-----------------------------------+ -| Class | Location | -+=====================================+===================================+ -| :py:class:`ReviewRequestActionHook` | The bar at the top of a review | -| | request (containing "Close", | -| | "Update", etc.) | -+-------------------------------------+-----------------------------------+ -| :py:class:`DiffViewerActionHook` | Like the ReviewRequestActionHook, | -| | but limited to the diff viewer | -| | page. | -+-------------------------------------+-----------------------------------+ -| :py:class:`HeaderActionHook` | An action in the page header. | -+-------------------------------------+-----------------------------------+ - -When instantiating any of these, you can pass a list of dictionaries defining -the actions you'd like to insert. These dictionaries have the following fields: - -* - **id**: The ID of the action (optional) - -* - **label**: The label for the action. - -* - **url**: The URI to invoke when the action is clicked. If you want to - invoke a javascript action, this should be '#', and you should use a - selector on the **id** field to attach the handler (as opposed to a - javascript: URL, which doesn't work on all browsers). - -* - **image**: The path to the image used for the icon (optional). - -* - **image_width**: The width of the image (optional). - -* - **image_height**: The height of the image (optional). - -There are also two hooks to provide drop-down menus in the action bars: - -+---------------------------------------------+-------------------------+ -| Class | Location | -+=============================================+=========================+ -| :py:class:`ReviewRequestDropdownActionHook` | The bar at the top of a | -| | review request. | -+---------------------------------------------+-------------------------+ -| :py:class:`HeaderDropdownActionHook` | The page header. | -+---------------------------------------------+-------------------------+ - -These work like the basic ActionHooks, except instead of a **url** field, they -contain an **items** field which is another list of dictionaries. Only one -level of nesting is possible. - - -Example -======= - -.. code-block:: python - - from reviewboard.extensions.base import Extension - from reviewboard.extensions.hooks import (HeaderDropdownActionHook, - ReviewRequestActionHook) - - - class SampleExtension(Extension): - def initialize(self): - # Single entry on review requests, consumed from JavaScript. - ReviewRequestActionHook(self, actions=[ - { - 'id': 'sample-item', - 'label': 'Review Request Item', - 'url': '#', - }, - ]) - - # A drop-down in the header that links to other pages. - HeaderDropdownActionHook(self, actions=[ - { - 'label': 'Header Dropdown', - 'items': [ - { - 'label': 'Item 1', - 'url': '...', - }, - { - 'label': 'Item 2', - 'url': '...', - }, - ], - }, - ]) diff --git a/docs/manual/extending/extensions/hooks/index.rst b/docs/manual/extending/extensions/hooks/index.rst index 06b7884b5c2b999895ad49d297d25eb39ff2da6b..dc2177d895aa401e7ee02dc1d91b77b500d61b0e 100644 --- a/docs/manual/extending/extensions/hooks/index.rst +++ b/docs/manual/extending/extensions/hooks/index.rst @@ -19,7 +19,7 @@ The following hooks are available for use by extensions. auth-backend-hook account-pages-hook account-page-forms-hook - action-hooks + action-hook admin-widget-hook comment-detail-display-hook dashboard-sidebar-items-hook diff --git a/reviewboard/extensions/hooks.py b/reviewboard/extensions/hooks.py index 8ac1f68e8b1ef8d48aa75fdb88e3ff8559d09d10..31b9d665ec665869a3d3502f0cc31f5fa1d11618 100644 --- a/reviewboard/extensions/hooks.py +++ b/reviewboard/extensions/hooks.py @@ -4,9 +4,9 @@ import inspect import warnings from django.utils import six -from djblets.extensions.hooks import (DataGridColumnsHook, ExtensionHook, - ExtensionHookPoint, SignalHook, - TemplateHook, URLHook) +from djblets.extensions.hooks import (AppliesToURLMixin, DataGridColumnsHook, + ExtensionHook, ExtensionHookPoint, + SignalHook, TemplateHook, URLHook) from reviewboard.accounts.backends import (register_auth_backend, unregister_auth_backend) @@ -23,6 +23,8 @@ from reviewboard.hostingsvcs.service import (register_hosting_service, unregister_hosting_service) from reviewboard.notifications.email import (register_email_hook, unregister_email_hook) +from reviewboard.reviews.actions import (BaseReviewRequestAction, + BaseReviewRequestMenuAction) from reviewboard.reviews.fields import (get_review_request_fieldset, register_review_request_fieldset, unregister_review_request_fieldset) @@ -30,6 +32,8 @@ from reviewboard.reviews.signals import (review_request_published, review_published, reply_published, review_request_closed) from reviewboard.reviews.ui.base import register_ui, unregister_ui +from reviewboard.urls import (diffviewer_url_names, + main_review_request_url_name) from reviewboard.webapi.server_info import (register_webapi_capabilities, unregister_webapi_capabilities) @@ -38,8 +42,8 @@ from reviewboard.webapi.server_info import (register_webapi_capabilities, class AuthBackendHook(ExtensionHook): """A hook for registering an authentication backend. - Authentication backends control user authentication, registration, and - user lookup, and user data manipulation. + Authentication backends control user authentication, registration, user + lookup, and user data manipulation. This hook takes the class of an authentication backend that should be made available to the server. @@ -466,87 +470,496 @@ class FileAttachmentThumbnailHook(ExtensionHook): class ActionHook(ExtensionHook): - """A hook for adding actions to a review request. - - Actions are displayed somewhere on the action bar (alongside Reviews, - Close, etc.) of the review request. The subclasses of ActionHook should - be used to determine placement. - - The provided actions parameter must be a list of actions. Each - action must be a dict with the following keys: - - * ``id``: The ID of this action (optional). - * ``image``: The path to the image used for the icon (optional). - * ``image_width``: The width of the image (optional). - * ``image_height``: The height of the image (optional). - * ``label``: The label for the action. - * ``url``: The URI to invoke when the action is clicked. - If you want to invoke a javascript action, this should - be '#', and you should use a selector on the `id` - field to attach the handler (as opposed to a - javascript: URL, which doesn't work on all browsers). + """A hook for injecting clickable actions into the UI. - If your hook needs to access the template context, it can override - get_actions and return results from there. + Actions are displayed either on the action bar of each review request or in + the page header. + + The provided ``actions`` parameter must be a list of actions. Each action + may be a :py:class:`dict` with the following keys: + + ``id`` (optional): + The ID of the action. + + ``label``: + The label for the action. + + ``url``: + The URL to invoke when the action is clicked. + + If we want to invoke a JavaScript action, then this should be ``#``, + and there should be a selector on the ``id`` field to attach the + handler (as opposed to a ``javascript:`` URL, which doesn't work on all + browsers). + + ``image`` (optional): + The path to the image used for the icon. + + ``image_width`` (optional): + The width of the image. + + ``image_height`` (optional): + The height of the image. + + If our hook needs to access the template context, then it can override + :py:meth:`get_actions` and return results from there. """ - def __init__(self, extension, actions=[], *args, **kwargs): + + def __init__(self, extension, actions=None, *args, **kwargs): + """Initialize this action hook. + + Args: + extension (djblets.extensions.extension.Extension): + The extension that is creating this action hook. + + actions (list, optional): + The list of actions (of type :py:class:`dict` or + :py:class:`~.actions.BaseReviewRequestAction`) to be added. + + *args (tuple): + Extra arguments. + + **kwargs (dict): + Extra keyword arguments. + """ super(ActionHook, self).__init__(extension, *args, **kwargs) - self.actions = actions + + self.actions = actions or [] def get_actions(self, context): - """Returns the list of action information for this action.""" + """Return the list of action information for this action hook. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template. + + Returns: + list: The list of action information for this action hook. + """ return self.actions +class _DictAction(BaseReviewRequestAction): + """An action for ActionHook-style dictionaries. + + For backwards compatibility, review request actions may also be supplied as + :py:class:`ActionHook`-style dictionaries. This helper class is used by + :py:meth:`convert_action` to convert these types of dictionaries into + instances of :py:class:`BaseReviewRequestAction`. + """ + + def __init__(self, action_dict, applies_to): + """Initialize this action. + + Args: + action_dict (dict): + A dictionary representing this action, as specified by the + :py:class:`ActionHook` class. + + applies_to (callable): + A callable that examines a given request and determines if this + action applies to the page. + """ + super(_DictAction, self).__init__() + + self.label = action_dict['label'] + self.action_id = action_dict.get( + 'id', + '%s-dummy-action' % self.label.lower().replace(' ', '-')) + self.url = action_dict['url'] + self._applies_to = applies_to + + def should_render(self, context): + """Return whether or not this action should render. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template + just before this action is to be rendered. + + Returns: + bool: Determines if this action should render. + """ + return self._applies_to(context['request']) + + +class _DictMenuAction(BaseReviewRequestMenuAction): + """A menu action for ReviewRequestDropdownActionHook-style dictionaries. + + For backwards compatibility, review request actions may also be supplied as + :py:class:`ReviewRequestDropdownActionHook`-style dictionaries. This helper + class is used by :py:meth:`convert_action` to convert these types of + dictionaries into instances of :py:class:`BaseReviewRequestMenuAction`. + """ + + def __init__(self, child_actions, action_dict, applies_to): + """Initialize this action. + + Args: + child_actions (list of dict or list of BaseReviewRequestAction): + The list of child actions to be contained by this menu action. + + action_dict (dict): + A dictionary representing this menu action, as specified by the + :py:class:`ReviewRequestDropdownActionHook` class. + + applies_to (callable): + A callable that examines a given request and determines if this + menu action applies to the page. + """ + super(_DictMenuAction, self).__init__(child_actions) + + self.label = action_dict['label'] + self.action_id = action_dict.get( + 'id', + '%s-dummy-menu-action' % self.label.lower().replace(' ', '-')) + self._applies_to = applies_to + + def should_render(self, context): + """Return whether or not this action should render. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template + just before this action is to be rendered. + + Returns: + bool: Determines if this action should render. + """ + return self._applies_to(context['request']) + + @six.add_metaclass(ExtensionHookPoint) -class ReviewRequestActionHook(ActionHook): - """A hook for adding an action to the review request page.""" +class BaseReviewRequestActionHook(AppliesToURLMixin, ActionHook): + """A base hook for adding review request actions to the action bar. + + Review request actions are displayed on the action bar (alongside default + actions such as :guilabel:`Download Diff` and :guilabel:`Ship It!`) of each + review request. This action bar is displayed on three main types of pages: + + #. **Review Request Pages**: + Where reviews are displayed. + + #. **File Attachment Pages**: + Where files like screenshots can be reviewed. + + #. **Diff Viewer Pages**: + Where diffs/interdiffs can be viewed side-by-side. + + Each action should be an instance of + :py:class:`~reviewboard.reviews.actions.BaseReviewRequestAction` (in + particular, each action could be an instance of the subclass + :py:class:`~reviewboard.reviews.actions.BaseReviewRequestMenuAction`). For + backwards compatibility, actions may also be supplied as + :py:class:`ActionHook`-style dictionaries. + """ + + def __init__(self, extension, actions=None, apply_to=None, *args, + **kwargs): + """Initialize this action hook. + + Args: + extension (djblets.extensions.extension.Extension): + The extension that is creating this action hook. + + actions (list, optional): + The list of actions (of type :py:class:`dict` or + :py:class:`~.actions.BaseReviewRequestAction`) to be added. + + apply_to (list of unicode, optional): + The list of URL names that this action hook will apply to. + + *args (tuple): + Extra arguments. + + **kwargs (dict): + Extra keyword arguments. + + Raises: + KeyError: + Some dictionary is not an :py:class:`ActionHook`-style + dictionary. + + ValueError: + Some review request action is neither a + :py:class:`~.actions.BaseReviewRequestAction` nor a + :py:class:`dict` instance. + """ + super(BaseReviewRequestActionHook, self).__init__( + extension, apply_to=apply_to or [], *args, **kwargs) + + self.actions = self._register_actions(actions or []) + + def _register_actions(self, actions): + """Register the given list of review request actions. + + Args: + actions (list, optional): + The list of actions (of type :py:class:`dict` or + :py:class:`~.actions.BaseReviewRequestAction`) to be added. + + Returns: + list of BaseReviewRequestAction: + The list of all registered actions. + + Raises: + KeyError: + Some dictionary is not an :py:class:`ActionHook`-style + dictionary. + + ValueError: + Some review request action is neither a + :py:class:`~.actions.BaseReviewRequestAction` nor a + :py:class:`dict` instance. + """ + registered_actions = [] + + # Since newly registered top-level actions are appended to the left of + # the other previously registered top-level actions, we must iterate + # through the actions in reverse. However, we don't want to mutate the + # original actions and we want to preserve the order of the original + # actions. Hence, we reverse twice in this method. + for action in reversed(actions): + action = self._normalize_action(action) + action.register() + registered_actions.append(action) + + registered_actions.reverse() + + return registered_actions + + def _normalize_action(self, action): + """Normalize the given review request action. + + For backwards compatibility, review request actions may also be + supplied as :py:class:`ActionHook`-style dictionaries. This helper + method normalizes the given review request action so that each review + request action is an instance of + :py:class:`~.actions.BaseReviewRequestAction`. + + Args: + action (dict or BaseReviewRequestAction): + The review request action to be normalized. + + Returns: + BaseReviewRequestAction: The normalized review request action. + + Raises: + KeyError: + The given dictionary is not an :py:class:`ActionHook`-style + dictionary. + + ValueError: + The given review request action is neither a + :py:class:`~.actions.BaseReviewRequestAction` nor a + :py:class:`dict` instance. + """ + if isinstance(action, BaseReviewRequestAction): + return action + + if isinstance(action, dict): + return self.convert_action(action) + + raise ValueError('Only BaseReviewRequestAction and dict instances are ' + 'supported') + + def convert_action(self, action_dict): + """Convert the given dictionary to a review request action instance. + + Args: + action_dict (dict): + A dictionary representing a review request action, as specified + by the :py:class:`ActionHook` class. + + Returns: + BaseReviewRequestAction: + The corresponding review request action instance. + + Raises: + KeyError: + The given dictionary is not an :py:class:`ActionHook`-style + dictionary. + """ + for key in ('label', 'url'): + if key not in action_dict: + raise KeyError('ActionHook-style dicts require a %s key' + % repr(key)) + + return _DictAction(action_dict, self.applies_to) + + +@six.add_metaclass(ExtensionHookPoint) +class ReviewRequestActionHook(BaseReviewRequestActionHook): + """A hook for adding review request actions to review request pages. + + By default, actions that are passed into this hook will only be displayed + on review request pages and not on any file attachment pages or diff + viewer pages. + """ + + def __init__(self, extension, actions=None, apply_to=None, *args, + **kwargs): + """Initialize this action hook. + + Args: + extension (djblets.extensions.extension.Extension): + The extension that is creating this action hook. + + actions (list, optional): + The list of actions (of type :py:class:`dict` or + :py:class:`~.actions.BaseReviewRequestAction`) to be added. + + apply_to (list of unicode, optional): + The list of URL names that this action hook will apply to. + + *args (tuple): + Extra arguments. + + **kwargs (dict): + Extra keyword arguments. + + Raises: + KeyError: + Some dictionary is not an :py:class:`ActionHook`-style + dictionary. + + ValueError: + Some review request action is neither a + :py:class:`~.actions.BaseReviewRequestAction` nor a + :py:class:`dict` instance. + """ + apply_to = apply_to or [main_review_request_url_name] + super(ReviewRequestActionHook, self).__init__( + extension, actions, apply_to, *args, **kwargs) @six.add_metaclass(ExtensionHookPoint) -class ReviewRequestDropdownActionHook(ActionHook): - """A hook for adding an drop down action to the review request page. - - The actions for a drop down action should contain: - - * ``id``: The ID of this action (optional). - * ``label``: The label of the drop-down. - * ``items``: A list of ActionHook-style dicts (see ActionHook params). - - For example:: - - actions = [{ - 'id': 'id 0', - 'label': 'Title', - 'items': [ - { - 'id': 'id 1', - 'label': 'Item 1', - 'url': '...', - }, - { - 'id': 'id 2', - 'label': 'Item 2', - 'url': '...', - } - ] - }] +class ReviewRequestDropdownActionHook(ReviewRequestActionHook): + """A hook for adding dropdown menu actions to review request pages. + + Each menu action should be an instance of + :py:class:`~reviewboard.reviews.actions.BaseReviewRequestMenuAction`. For + backwards compatibility, menu actions may also be supplied as dictionaries + with the following keys: + + ``id`` (optional): + The ID of the action. + + ``label``: + The label for the dropdown menu action. + + ``items``: + A list of :py:class:`ActionHook`-style dictionaries. + + Example: + .. code-block:: python + + actions = [{ + 'id': 'sample-menu-action', + 'label': 'Sample Menu', + 'items': [ + { + 'id': 'first-item-action', + 'label': 'Item 1', + 'url': '#', + }, + { + 'label': 'Item 2', + 'url': '#', + }, + ], + }] """ + def convert_action(self, action_dict): + """Convert the given dictionary to a review request action instance. + + Children action dictionaries are recursively converted to action + instances. + + Args: + action_dict (dict): + A dictionary representing a review request menu action, as + specified by the :py:class:`ReviewRequestDropdownActionHook` + class. + + Returns: + BaseReviewRequestMenuAction: + The corresponding review request menu action instance. + + Raises: + KeyError: + The given review request menu action dictionary is not a + :py:class:`ReviewRequestDropdownActionHook`-style dictionary. + """ + for key in ('label', 'items'): + if key not in action_dict: + raise KeyError('ReviewRequestDropdownActionHook-style dicts ' + 'require a %s key' % repr(key)) + + return _DictMenuAction( + [ + super(ReviewRequestDropdownActionHook, self).convert_action( + child_action_dict) + for child_action_dict in action_dict['items'] + ], + action_dict, + self.applies_to + ) + @six.add_metaclass(ExtensionHookPoint) -class DiffViewerActionHook(ActionHook): - """A hook for adding an action to the diff viewer page.""" +class DiffViewerActionHook(BaseReviewRequestActionHook): + """A hook for adding review request actions to diff viewer pages. + + By default, actions that are passed into this hook will only be displayed + on diff viewer pages and not on any review request pages or file attachment + pages. + """ + + def __init__(self, extension, actions=None, apply_to=diffviewer_url_names, + *args, **kwargs): + """Initialize this action hook. + + Args: + extension (djblets.extensions.extension.Extension): + The extension that is creating this action hook. + + actions (list, optional): + The list of actions (of type :py:class:`dict` or + :py:class:`~.actions.BaseReviewRequestAction`) to be added. + + apply_to (list of unicode, optional): + The list of URL names that this action hook will apply to. + + *args (tuple): + Extra arguments. + + **kwargs (dict): + Extra keyword arguments. + + Raises: + KeyError: + Some dictionary is not an :py:class:`ActionHook`-style + dictionary. + + ValueError: + Some review request action is neither a + :py:class:`~.actions.BaseReviewRequestAction` nor a + :py:class:`dict` instance. + """ + super(DiffViewerActionHook, self).__init__( + extension, actions, apply_to, *args, **kwargs) @six.add_metaclass(ExtensionHookPoint) class HeaderActionHook(ActionHook): - """A hook for putting an action in the page header.""" + """A hook for adding actions to the page header.""" @six.add_metaclass(ExtensionHookPoint) class HeaderDropdownActionHook(ActionHook): - """A hook for putting multiple actions into a header dropdown.""" + """A hook for adding dropdown menu actions to the page header.""" @six.add_metaclass(ExtensionHookPoint) @@ -906,6 +1319,7 @@ __all__ = [ 'ActionHook', 'AdminWidgetHook', 'AuthBackendHook', + 'BaseReviewRequestActionHook', 'CommentDetailDisplayHook', 'DashboardColumnsHook', 'DashboardSidebarItemsHook', diff --git a/reviewboard/extensions/templatetags/rb_extensions.py b/reviewboard/extensions/templatetags/rb_extensions.py index a633c16a0ae4083f221ae86062710af8655a755a..5862b03bddb471d59eac5d63032540a2913091d1 100644 --- a/reviewboard/extensions/templatetags/rb_extensions.py +++ b/reviewboard/extensions/templatetags/rb_extensions.py @@ -7,12 +7,9 @@ from django.template.loader import render_to_string from djblets.util.decorators import basictag from reviewboard.extensions.hooks import (CommentDetailDisplayHook, - DiffViewerActionHook, HeaderActionHook, HeaderDropdownActionHook, - NavigationBarHook, - ReviewRequestActionHook, - ReviewRequestDropdownActionHook) + NavigationBarHook) from reviewboard.site.urlresolvers import local_site_reverse @@ -51,30 +48,6 @@ def action_hooks(context, hook_cls, action_key="action", @register.tag @basictag(takes_context=True) -def diffviewer_action_hooks(context): - """Displays all registered action hooks for the diff viewer.""" - return action_hooks(context, DiffViewerActionHook) - - -@register.tag -@basictag(takes_context=True) -def review_request_action_hooks(context): - """Displays all registered action hooks for review requests.""" - return action_hooks(context, ReviewRequestActionHook) - - -@register.tag -@basictag(takes_context=True) -def review_request_dropdown_action_hooks(context): - """Displays all registered action hooks for review requests.""" - return action_hooks(context, - ReviewRequestDropdownActionHook, - "actions", - "extensions/action_dropdown.html") - - -@register.tag -@basictag(takes_context=True) def navigation_bar_hooks(context): """Displays all registered navigation bar entries.""" s = "" diff --git a/reviewboard/extensions/tests.py b/reviewboard/extensions/tests.py index e2b59c197ea2cf7a82fbf37da2faed1904df27e5..50e01276fc9660a42ff6b3840655d8d98435632f 100644 --- a/reviewboard/extensions/tests.py +++ b/reviewboard/extensions/tests.py @@ -11,13 +11,15 @@ from djblets.extensions.manager import ExtensionManager from djblets.extensions.models import RegisteredExtension from djblets.siteconfig.models import SiteConfiguration from kgb import SpyAgency +from mock import Mock +from reviewboard.admin.siteconfig import load_site_config from reviewboard.admin.widgets import (primary_widgets, secondary_widgets, Widget) -from reviewboard.admin.siteconfig import load_site_config from reviewboard.extensions.base import Extension from reviewboard.extensions.hooks import (AdminWidgetHook, + BaseReviewRequestActionHook, CommentDetailDisplayHook, DiffViewerActionHook, EmailHook, @@ -26,24 +28,28 @@ from reviewboard.extensions.hooks import (AdminWidgetHook, HostingServiceHook, NavigationBarHook, ReviewPublishedEmailHook, + ReviewReplyPublishedEmailHook, ReviewRequestActionHook, ReviewRequestApprovalHook, ReviewRequestClosedEmailHook, ReviewRequestDropdownActionHook, ReviewRequestFieldSetsHook, ReviewRequestPublishedEmailHook, - ReviewReplyPublishedEmailHook, WebAPICapabilitiesHook) from reviewboard.hostingsvcs.service import (get_hosting_service, HostingService) from reviewboard.notifications.email import get_email_address_for_user -from reviewboard.testing.testcase import TestCase -from reviewboard.reviews.models.review_request import ReviewRequest +from reviewboard.reviews.actions import (BaseReviewRequestAction, + BaseReviewRequestMenuAction, + clear_all_actions) from reviewboard.reviews.fields import (BaseReviewRequestField, BaseReviewRequestFieldSet) -from reviewboard.reviews.signals import (review_request_published, - review_published, reply_published, - review_request_closed) +from reviewboard.reviews.models.review_request import ReviewRequest +from reviewboard.reviews.signals import (reply_published, + review_published, + review_request_closed, + review_request_published) +from reviewboard.testing.testcase import TestCase from reviewboard.webapi.tests.base import BaseWebAPITestCase from reviewboard.webapi.tests.mimetypes import root_item_mimetype from reviewboard.webapi.tests.urls import get_root_url @@ -78,56 +84,281 @@ def set_siteconfig_settings(settings): load_site_config() - class DummyExtension(Extension): registration = RegisteredExtension() -class HookTests(TestCase): - """Tests the extension hooks.""" +class ActionHookTests(TestCase): + """Tests the action hooks in reviewboard.extensions.hooks.""" + + class _TestAction(BaseReviewRequestAction): + action_id = 'test-action' + label = 'Test Action' + + class _TestMenuAction(BaseReviewRequestMenuAction): + action_id = 'test-menu-instance-action' + label = 'Menu Instance' + + def _get_context(self, user_pk='123', is_authenticated=True, + url_name='review-request-detail', local_site_name=None, + status=ReviewRequest.PENDING_REVIEW, submitter_id='456', + is_public=True, display_id='789', has_diffs=True, + can_change_status=True, can_edit_reviewrequest=True, + delete_reviewrequest=True): + request = Mock() + request.resolver_match = Mock() + request.resolver_match.url_name = url_name + request.user = Mock() + request.user.pk = user_pk + request.user.is_authenticated.return_value = is_authenticated + request._local_site_name = local_site_name + + review_request = Mock() + review_request.status = status + review_request.submitter_id = submitter_id + review_request.public = is_public + review_request.display_id = display_id + + if not has_diffs: + review_request.get_draft.return_value = None + review_request.get_diffsets.return_value = None + + context = Context({ + 'request': request, + 'review_request': review_request, + 'perms': { + 'reviews': { + 'can_change_status': can_change_status, + 'can_edit_reviewrequest': can_edit_reviewrequest, + 'delete_reviewrequest': delete_reviewrequest, + }, + }, + }) + + return context + def setUp(self): - super(HookTests, self).setUp() + super(ActionHookTests, self).setUp() manager = ExtensionManager('') self.extension = DummyExtension(extension_manager=manager) def tearDown(self): - super(HookTests, self).tearDown() + super(ActionHookTests, self).tearDown() self.extension.shutdown() - def test_diffviewer_action_hook(self): - """Testing diff viewer action extension hooks""" - self._test_action_hook('diffviewer_action_hooks', DiffViewerActionHook) - def test_review_request_action_hook(self): - """Testing review request action extension hooks""" - self._test_action_hook('review_request_action_hooks', - ReviewRequestActionHook) + """Testing ReviewRequestActionHook renders on a review request page but + not on a file attachment or a diff viewer page + """ + self._test_base_review_request_action_hook( + 'review-request-detail', ReviewRequestActionHook, True) + self._test_base_review_request_action_hook( + 'file-attachment', ReviewRequestActionHook, False) + self._test_base_review_request_action_hook( + 'view-diff', ReviewRequestActionHook, False) - def test_review_request_dropdown_action_hook(self): - """Testing review request drop-down action extension hooks""" - self._test_dropdown_action_hook('review_request_dropdown_action_hooks', - ReviewRequestDropdownActionHook) + def test_diffviewer_action_hook(self): + """Testing DiffViewerActionHook renders on a diff viewer page but not + on a review request page or a file attachment page + """ + self._test_base_review_request_action_hook( + 'review-request-detail', DiffViewerActionHook, False) + self._test_base_review_request_action_hook( + 'file-attachment', DiffViewerActionHook, False) + self._test_base_review_request_action_hook( + 'view-diff', DiffViewerActionHook, True) - def test_action_hook_context_doesnt_leak(self): - """Testing ActionHooks' context won't leak state""" - action = { - 'label': 'Test Action', - 'id': 'test-action', - 'url': 'foo-url', + def test_review_request_dropdown_action_hook(self): + """Testing ReviewRequestDropdownActionHook renders on a review request + page but not on a file attachment or a diff viewer page + """ + self._test_review_request_dropdown_action_hook( + 'review-request-detail', ReviewRequestDropdownActionHook, True) + self._test_review_request_dropdown_action_hook( + 'file-attachment', ReviewRequestDropdownActionHook, False) + self._test_review_request_dropdown_action_hook( + 'view-diff', ReviewRequestDropdownActionHook, False) + + def test_action_hook_init_raises_key_error(self): + """Testing that action hook __init__ raises a KeyError""" + missing_url_action = { + 'id': 'missing-url-action', + 'label': 'This action dict is missing a mandatory URL key.', + } + missing_key = 'url' + error_message = ('ActionHook-style dicts require a %s key' + % repr(missing_key)) + action_hook_classes = [ + BaseReviewRequestActionHook, + ReviewRequestActionHook, + DiffViewerActionHook, + ] + + for hook_cls in action_hook_classes: + with self.assertRaisesMessage(KeyError, error_message): + hook_cls(extension=self.extension, actions=[ + missing_url_action, + ]) + + clear_all_actions() + + def test_action_hook_init_raises_value_error(self): + """Testing that BaseReviewRequestActionHook __init__ raises a + ValueError""" + unsupported_type_action = [{ + 'id': 'unsupported-type-action', + 'label': 'This action is a list, which is an unsupported type.', + 'url': '#', + }] + error_message = ('Only BaseReviewRequestAction and dict instances are ' + 'supported') + action_hook_classes = [ + BaseReviewRequestActionHook, + ReviewRequestActionHook, + DiffViewerActionHook, + ReviewRequestDropdownActionHook, + ] + + for hook_cls in action_hook_classes: + with self.assertRaisesMessage(ValueError, error_message): + hook_cls(extension=self.extension, actions=[ + unsupported_type_action, + ]) + + clear_all_actions() + + def test_dropdown_action_hook_init_raises_key_error(self): + """Testing that ReviewRequestDropdownActionHook __init__ raises a + KeyError""" + missing_items_menu_action = { + 'id': 'missing-items-menu-action', + 'label': 'This menu action dict is missing a mandatory items key.', } + missing_key = 'items' + error_message = ('ReviewRequestDropdownActionHook-style dicts require ' + 'a %s key' % repr(missing_key)) - ReviewRequestActionHook(extension=self.extension, actions=[action]) + with self.assertRaisesMessage(KeyError, error_message): + ReviewRequestDropdownActionHook(extension=self.extension, actions=[ + missing_items_menu_action, + ]) - context = Context({}) + clear_all_actions() - t = Template( - "{% load rb_extensions %}" - "{% review_request_action_hooks %}") - t.render(context) + def _test_base_review_request_action_hook(self, url_name, hook_cls, + should_render): + """Test if the action hook renders or not at the given URL. + + Args: + url_name (unicode): + The name of the URL where each action is to be rendered. + + hook_cls (class): + The class of the action hook to be tested. + should_render (bool): + The expected rendering behaviour. + """ + hook = hook_cls(extension=self.extension, actions=[ + { + 'id': 'with-id-action', + 'label': 'Yes ID', + 'url': 'with-id-url', + }, + self._TestAction(), + { + 'label': 'No ID', + 'url': 'without-id-url', + }, + ]) + context = self._get_context(url_name=url_name) + entries = hook.get_actions(context) + self.assertEqual(len(entries), 3) + self.assertEqual(entries[0].action_id, 'with-id-action') + self.assertEqual(entries[1].action_id, 'test-action') + self.assertEqual(entries[2].action_id, 'no-id-dummy-action') + + template = Template( + '{% load reviewtags %}' + '{% review_request_actions %}' + ) + content = template.render(context) self.assertNotIn('action', context) + self.assertEqual(should_render, 'href="with-id-url"' in content) + self.assertIn('>Test Action<', content) + self.assertEqual(should_render, 'id="no-id-dummy-action"' in content) + + clear_all_actions() + + content = template.render(context) + self.assertNotIn('href="with-id-url"', content) + self.assertNotIn('>Test Action<', content) + self.assertNotIn('id="no-id-dummy-action"', content) + + def _test_review_request_dropdown_action_hook(self, url_name, hook_cls, + should_render): + """Test if the dropdown action hook renders or not at the given URL. + + Args: + url_name (unicode): + The name of the URL where each action is to be rendered. + + hook_cls (class): + The class of the dropdown action hook to be tested. + + should_render (bool): + The expected rendering behaviour. + """ + hook = hook_cls(extension=self.extension, actions=[ + self._TestMenuAction([ + self._TestAction(), + ]), + { + 'id': 'test-menu-dict-action', + 'label': 'Menu Dict', + 'items': [ + { + 'id': 'with-id-action', + 'label': 'Yes ID', + 'url': 'with-id-url', + }, + { + 'label': 'No ID', + 'url': 'without-id-url', + }, + ] + }, + ]) + context = self._get_context(url_name=url_name) + entries = hook.get_actions(context) + self.assertEqual(len(entries), 2) + self.assertEqual(entries[0].action_id, 'test-menu-instance-action') + self.assertEqual(entries[1].action_id, 'test-menu-dict-action') + + template = Template( + '{% load reviewtags %}' + '{% review_request_actions %}' + ) + content = template.render(context) + self.assertNotIn('action', context) + self.assertIn('>Test Action<', content) + self.assertIn('>Menu Instance ▾<', content) + self.assertEqual(should_render, + 'id="test-menu-dict-action"' in content) + self.assertEqual(should_render, '>Menu Dict ▾<' in content) + self.assertEqual(should_render, 'href="with-id-url"' in content) + self.assertEqual(should_render, 'id="no-id-dummy-action"' in content) + + clear_all_actions() + + content = template.render(context) + self.assertNotIn('>Test Action<', content) + self.assertNotIn('>Menu Instance ▾<', content) + self.assertNotIn('id="test-menu-dict-action"', content) + self.assertNotIn('href="with-id-url"', content) + self.assertNotIn('id="no-id-dummy-action"', content) def _test_action_hook(self, template_tag_name, hook_cls): action = { @@ -194,6 +425,29 @@ class HookTests(TestCase): 'height="%(image_height)s" border="0" alt="" />' '%(label)s' % action) + def test_header_hooks(self): + """Testing HeaderActionHook""" + self._test_action_hook('header_action_hooks', HeaderActionHook) + + def test_header_dropdown_action_hook(self): + """Testing HeaderDropdownActionHook""" + self._test_dropdown_action_hook('header_dropdown_action_hooks', + HeaderDropdownActionHook) + + +class NavigationBarHookTests(TestCase): + """Tests the navigation bar hooks.""" + def setUp(self): + super(NavigationBarHookTests, self).setUp() + + manager = ExtensionManager('') + self.extension = DummyExtension(extension_manager=manager) + + def tearDown(self): + super(NavigationBarHookTests, self).tearDown() + + self.extension.shutdown() + def test_navigation_bar_hooks(self): """Testing navigation entry extension hooks""" entry = { @@ -355,15 +609,6 @@ class HookTests(TestCase): 'url': '/dashboard/', }) - def test_header_hooks(self): - """Testing header action extension hooks""" - self._test_action_hook('header_action_hooks', HeaderActionHook) - - def test_header_dropdown_action_hook(self): - """Testing header drop-down action extension hooks""" - self._test_dropdown_action_hook('header_dropdown_action_hooks', - HeaderDropdownActionHook) - class TestService(HostingService): name = 'test-service' @@ -430,7 +675,7 @@ class AdminWidgetHookTests(TestCase): self.assertIn(TestWidget, primary_widgets) def test_unregister(self): - """Testing AdminWidgetHook unitializing""" + """Testing AdminWidgetHook uninitializing""" hook = AdminWidgetHook(extension=self.extension, widget_cls=TestWidget) hook.shutdown() @@ -748,11 +993,11 @@ class SandboxTests(TestCase): context = Context({'comment': 'this is a comment'}) - t = Template( - "{% load rb_extensions %}" - "{% diffviewer_action_hooks %}") + template = Template( + '{% load reviewtags %}' + '{% review_request_actions %}') - t.render(context).strip() + template.render(context) def test_action_hooks_header_hook(self): """Testing sandboxing HeaderActionHook when @@ -787,11 +1032,11 @@ class SandboxTests(TestCase): context = Context({'comment': 'this is a comment'}) - t = Template( - "{% load rb_extensions %}" - "{% review_request_action_hooks %}") + template = Template( + '{% load reviewtags %}' + '{% review_request_actions %}') - t.render(context).strip() + template.render(context) def test_action_hooks_review_request_dropdown_hook(self): """Testing sandboxing ReviewRequestDropdownActionHook when @@ -800,14 +1045,14 @@ class SandboxTests(TestCase): context = Context({'comment': 'this is a comment'}) - t = Template( - "{% load rb_extensions %}" - "{% review_request_dropdown_action_hooks %}") + template = Template( + '{% load reviewtags %}' + '{% review_request_actions %}') - t.render(context).strip() + template.render(context) def test_is_empty_review_request_fieldset(self): - """Testing sandboxing ReivewRequestFieldset is_empty function in + """Testing sandboxing ReviewRequestFieldset is_empty function in for_review_request_fieldset""" fieldset = [BaseReviewRequestTestIsEmptyFieldset] ReviewRequestFieldSetsHook(extension=self.extension, diff --git a/reviewboard/reviews/actions.py b/reviewboard/reviews/actions.py new file mode 100644 index 0000000000000000000000000000000000000000..66564a787875497935af3271e267ca1fc4651af2 --- /dev/null +++ b/reviewboard/reviews/actions.py @@ -0,0 +1,464 @@ +from __future__ import unicode_literals + +from collections import deque + +from django.template.loader import render_to_string + +from reviewboard.reviews.errors import DepthLimitExceededError + + +#: The maximum depth limit of any action instance. +MAX_DEPTH_LIMIT = 2 + +#: The mapping of all action IDs to their corresponding action instances. +_all_actions = {} + +#: All top-level action IDs (in their left-to-right order of appearance). +_top_level_ids = deque() + +#: Determines if the default action instances have been populated yet. +_populated = False + + +class BaseReviewRequestAction(object): + """A base class for an action that can be applied to a review request. + + Creating an action requires subclassing :py:class:`BaseReviewRequestAction` + and overriding any fields/methods as desired. Different instances of the + same subclass can also override the class fields with their own instance + fields. + + Example: + .. code-block:: python + + class UsedOnceAction(BaseReviewRequestAction): + action_id = 'once' + label = 'This is used once.' + + class UsedMultipleAction(BaseReviewRequestAction): + def __init__(self, action_id, label): + super(UsedMultipleAction, self).__init__() + + self.action_id = 'repeat-' + action_id + self.label = 'This is used multiple times,' + + Note: + Since the same action will be rendered for multiple different users in + a multithreaded environment, the action state should not be modified + after initialization. If we want different action attributes at + runtime, then we can override one of the getter methods (such as + :py:meth:`get_label`), which by default will simply return the original + attribute from initialization. + """ + + #: The ID of this action. Must be unique across all types of actions and + #: menu actions, at any depth. + action_id = None + + #: The label that displays this action to the user. + label = None + + #: The URL to invoke if this action is clicked. + url = '#' + + #: Determines if this action should be initially hidden to the user. + hidden = False + + def __init__(self): + """Initialize this action. + + By default, actions are top-level and have no children. + """ + self._parent = None + self._max_depth = 0 + + def copy_to_dict(self, context): + """Copy this action instance to a dictionary. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + dict: The corresponding dictionary. + """ + return { + 'action_id': self.action_id, + 'label': self.get_label(context), + 'url': self.get_url(context), + 'hidden': self.get_hidden(context), + } + + def get_label(self, context): + """Return this action's label. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + unicode: The label that displays this action to the user. + """ + return self.label + + def get_url(self, context): + """Return this action's URL. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + unicode: The URL to invoke if this action is clicked. + """ + return self.url + + def get_hidden(self, context): + """Return whether this action should be initially hidden to the user. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + bool: Whether this action should be initially hidden to the user. + """ + return self.hidden + + def should_render(self, context): + """Return whether or not this action should render. + + The default implementation is to always render the action everywhere. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template + just before this action is to be rendered. + + Returns: + bool: Determines if this action should render. + """ + return True + + @property + def max_depth(self): + """Lazily compute the max depth of any action contained by this action. + + Top-level actions have a depth of zero, and child actions have a depth + that is one more than their parent action's depth. + + Algorithmically, the notion of max depth is equivalent to the notion of + height in the context of trees (from graph theory). We decided to use + this term instead so as not to confuse it with the dimensional height + of a UI element. + + Returns: + int: The max depth of any action contained by this action. + """ + return self._max_depth + + def reset_max_depth(self): + """Reset the max_depth of this action and all its ancestors to zero.""" + self._max_depth = 0 + + if self._parent: + self._parent.reset_max_depth() + + def render(self, context, action_key='action', + template_name='reviews/action.html'): + """Render this action instance and return the content as HTML. + + Args: + context (django.template.Context): + The collection of key-value pairs that is passed to the + template in order to render this action. + + action_key (unicode, optional): + The key to be used for this action in the context map. + + template_name (unicode, optional): + The name of the template to be used for rendering this action. + + Returns: + unicode: The action rendered in HTML. + """ + content = '' + + if self.should_render(context): + context.push() + + try: + context[action_key] = self.copy_to_dict(context) + content = render_to_string(template_name, context) + finally: + context.pop() + + return content + + def register(self, parent=None): + """Register this review request action instance. + + Note: + Newly registered top-level actions are appended to the left of the + other previously registered top-level actions. So if we intend to + register a collection of top-level actions in a certain order, then + we likely want to iterate through the actions in reverse. + + Args: + parent (BaseReviewRequestMenuAction, optional): + The parent action instance of this action instance. + + Raises: + KeyError: + A second registration is attempted (action IDs must be unique + across all types of actions and menu actions, at any depth). + + DepthLimitExceededError: + The maximum depth limit is exceeded. + """ + _populate_defaults() + + if self.action_id in _all_actions: + raise KeyError('%s already corresponds to a registered review ' + 'request action' % self.action_id) + + if self.max_depth > MAX_DEPTH_LIMIT: + raise DepthLimitExceededError(self.action_id, MAX_DEPTH_LIMIT) + + if parent: + parent.child_actions.append(self) + self._parent = parent + else: + _top_level_ids.appendleft(self.action_id) + + _all_actions[self.action_id] = self + + def unregister(self): + """Unregister this review request action instance. + + Note: + This method can mutate its parent's child actions. So if we are + iteratively unregistering a parent's child actions, then we should + consider first making a clone of the list of children. + + Raises: + KeyError: An unregistration is attempted before it's registered. + """ + _populate_defaults() + + try: + del _all_actions[self.action_id] + except KeyError: + raise KeyError('%s does not correspond to a registered review ' + 'request action' % self.action_id) + + if self._parent: + self._parent.child_actions.remove(self) + else: + _top_level_ids.remove(self.action_id) + + self.reset_max_depth() + + +class BaseReviewRequestMenuAction(BaseReviewRequestAction): + """A base class for an action with a dropdown menu. + + Note: + A menu action's child actions must always be pre-registered. + """ + + def __init__(self, child_actions=None): + """Initialize this menu action. + + Args: + child_actions (list of BaseReviewRequestAction, optional): + The list of child actions to be contained by this menu action. + + Raises: + KeyError: + A second registration is attempted (action IDs must be unique + across all types of actions and menu actions, at any depth). + + DepthLimitExceededError: + The maximum depth limit is exceeded. + """ + super(BaseReviewRequestMenuAction, self).__init__() + + self.child_actions = [] + child_actions = child_actions or [] + + for child_action in child_actions: + child_action.register(self) + + def copy_to_dict(self, context): + """Copy this menu action instance to a dictionary. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + dict: The corresponding dictionary. + """ + dict_copy = { + 'child_actions': self.child_actions, + } + dict_copy.update(super(BaseReviewRequestMenuAction, self).copy_to_dict( + context)) + + return dict_copy + + @property + def max_depth(self): + """Lazily compute the max depth of any action contained by this action. + + Returns: + int: The max depth of any action contained by this action. + """ + if self.child_actions and self._max_depth == 0: + self._max_depth = 1 + max(child_action.max_depth + for child_action in self.child_actions) + + return self._max_depth + + def render(self, context, action_key='menu_action', + template_name='reviews/menu_action.html'): + """Render this menu action instance and return the content as HTML. + + Args: + context (django.template.Context): + The collection of key-value pairs that is passed to the + template in order to render this menu action. + + action_key (unicode, optional): + The key to be used for this menu action in the context map. + + template_name (unicode, optional): + The name of the template to be used for rendering this menu + action. + + Returns: + unicode: The action rendered in HTML. + """ + return super(BaseReviewRequestMenuAction, self).render( + context, action_key, template_name) + + def unregister(self): + """Unregister this review request action instance. + + This menu action recursively unregisters its child action instances. + + Raises: + KeyError: An unregistration is attempted before it's registered. + """ + super(BaseReviewRequestMenuAction, self).unregister() + + # Unregistration will mutate self.child_actions, so we make a copy. + for child_action in list(self.child_actions): + child_action.unregister() + + +# TODO: Convert all this to use djblets.registries. +def _populate_defaults(): + """Populate the default action instances.""" + global _populated + + if not _populated: + _populated = True + + from reviewboard.reviews.default_actions import get_default_actions + + for default_action in reversed(get_default_actions()): + default_action.register() + + +def get_top_level_actions(): + """Return a generator of all top-level registered action instances. + + Yields: + BaseReviewRequestAction: + All top-level registered review request action instances. + """ + _populate_defaults() + + return (_all_actions[action_id] for action_id in _top_level_ids) + + +def register_actions(actions, parent_id=None): + """Register the given actions as children of the corresponding parent. + + If no parent_id is given, then the actions are assumed to be top-level. + + Args: + actions (iterable of BaseReviewRequestAction): + The collection of action instances to be registered. + + parent_id (unicode, optional): + The action ID of the parent of each action instance to be + registered. + + Raises: + KeyError: + The parent action cannot be found or a second registration is + attempted (action IDs must be unique across all types of actions + and menu actions, at any depth). + + DepthLimitExceededError: + The maximum depth limit is exceeded. + """ + _populate_defaults() + + if parent_id is None: + parent = None + else: + try: + parent = _all_actions[parent_id] + except KeyError: + raise KeyError('%s does not correspond to a registered review ' + 'request action' % parent_id) + + for action in reversed(actions): + action.register(parent) + + if parent: + parent.reset_max_depth() + + +def unregister_actions(action_ids): + """Unregister each of the actions corresponding to the given IDs. + + Args: + action_ids (iterable of unicode): + The collection of action IDs corresponding to the actions to be + removed. + + Raises: + KeyError: An unregistration is attempted before it's registered. + """ + _populate_defaults() + + for action_id in action_ids: + try: + action = _all_actions[action_id] + except KeyError: + raise KeyError('%s does not correspond to a registered review ' + 'request action' % action_id) + + action.unregister() + + +def clear_all_actions(): + """Clear all registered actions. + + This method is really only intended to be used by unit tests. We might be + able to remove this hack once we convert to djblets.registries. + + Warning: + This will clear **all** actions, even if they were registered in + separate extensions. + """ + global _populated + + _all_actions.clear() + _top_level_ids.clear() + _populated = False diff --git a/reviewboard/reviews/default_actions.py b/reviewboard/reviews/default_actions.py new file mode 100644 index 0000000000000000000000000000000000000000..b327c53d6a361972345b2680423368e62c49ea47 --- /dev/null +++ b/reviewboard/reviews/default_actions.py @@ -0,0 +1,225 @@ +from __future__ import unicode_literals + +from django.utils.translation import ugettext_lazy as _ + +from reviewboard.reviews.actions import (BaseReviewRequestAction, + BaseReviewRequestMenuAction) +from reviewboard.reviews.models import ReviewRequest +from reviewboard.site.urlresolvers import local_site_reverse +from reviewboard.urls import diffviewer_url_names + + +class CloseMenuAction(BaseReviewRequestMenuAction): + """A menu action for closing the corresponding review request.""" + + action_id = 'close-review-request-action' + label = _('Close') + + def should_render(self, context): + review_request = context['review_request'] + + if review_request.status != ReviewRequest.PENDING_REVIEW: + return False + + return (context['request'].user.pk == review_request.submitter_id or + context['perms']['reviews']['can_change_status'] and + review_request.public) + + +class SubmitAction(BaseReviewRequestAction): + """An action for submitting the review request.""" + + action_id = 'submit-review-request-action' + label = _('Submitted') + + def should_render(self, context): + return context['review_request'].public + + +class DiscardAction(BaseReviewRequestAction): + """An action for discarding the review request.""" + + action_id = 'discard-review-request-action' + label = _('Discarded') + + +class DeleteAction(BaseReviewRequestAction): + """An action for permanently deleting the review request.""" + + action_id = 'delete-review-request-action' + label = _('Delete Permanently') + + def should_render(self, context): + return context['perms']['reviews']['delete_reviewrequest'] + + +class UpdateMenuAction(BaseReviewRequestMenuAction): + """A menu action for updating the corresponding review request.""" + + action_id = 'update-review-request-action' + label = _('Update') + + def should_render(self, context): + review_request = context['review_request'] + + if review_request.status != ReviewRequest.PENDING_REVIEW: + return False + + return (context['request'].user.pk == review_request.submitter_id or + context['perms']['reviews']['can_edit_reviewrequest']) + + +class UploadDiffAction(BaseReviewRequestAction): + """An action for updating/uploading a diff for the review request.""" + + action_id = 'upload-diff-action' + + def get_label(self, context): + """Return this action's label. + + The label will change depending on whether or not the corresponding + review request already has a diff. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + unicode: The label that displays this action to the user. + """ + review_request = context['review_request'] + draft = review_request.get_draft(context['request'].user) + + if (draft and draft.diffset) or review_request.get_diffsets(): + return _('Update Diff') + + return _('Upload Diff') + + def should_render(self, context): + """Return whether or not this action should render. + + If the corresponding review request has a repository, then an upload + diff form exists, so we should render this UploadDiffAction. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template + just before this action is to be rendered. + + Returns: + bool: Determines if this action should render. + """ + return context['review_request'].repository is not None + + +class UploadFileAction(BaseReviewRequestAction): + """An action for uploading a file for the review request.""" + + action_id = 'upload-file-action' + label = _('Add File') + + +class DownloadDiffAction(BaseReviewRequestAction): + """An action for downloading a diff from the review request.""" + + action_id = 'download-diff-action' + label = _('Download Diff') + + def get_url(self, context): + """Return this action's URL. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + unicode: The URL to invoke if this action is clicked. + """ + return local_site_reverse('raw-diff', context['request'], kwargs={ + 'review_request_id': context['review_request'].display_id, + }) + + def get_hidden(self, context): + """Return whether this action should be initially hidden to the user. + + Args: + context (django.template.Context): + The collection of key-value pairs from the template. + + Returns: + bool: Whether this action should be initially hidden to the user. + """ + match = context['request'].resolver_match + + if match.url_name in diffviewer_url_names: + return match.url_name == 'view-interdiff' + + return super(DownloadDiffAction, self).get_hidden(context) + + def should_render(self, context): + """Return whether or not this action should render. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template + just before this action is to be rendered. + + Returns: + bool: Determines if this action should render. + """ + review_request = context['review_request'] + request = context['request'] + match = request.resolver_match + + # If we're on a diff viewer page, then this DownloadDiffAction should + # initially be rendered, but possibly hidden. + if match.url_name in diffviewer_url_names: + return True + + # Otherwise, we're either on a review request page or a file attachment + # page, so check if the corresponding review request has a diff. + draft = review_request.get_draft(request.user) + return (draft and draft.diffset is not None or + review_request.get_diffsets() is not None) + + +class EditReviewAction(BaseReviewRequestAction): + """An action for editing a review intended for the review request.""" + + action_id = 'review-action' + label = _('Review') + + def should_render(self, context): + return context['request'].user.is_authenticated() + + +class ShipItAction(BaseReviewRequestAction): + """An action for quickly approving the review request without comments.""" + + action_id = 'ship-it-action' + label = _('Ship It!') + + def should_render(self, context): + return context['request'].user.is_authenticated() + + +def get_default_actions(): + """Return a copy of all the default actions. + + Returns: + list of BaseReviewRequestAction: A copy of all the default actions. + """ + return [ + CloseMenuAction([ + SubmitAction(), + DiscardAction(), + DeleteAction(), + ]), + UpdateMenuAction([ + UploadDiffAction(), + UploadFileAction(), + ]), + DownloadDiffAction(), + EditReviewAction(), + ShipItAction(), + ] diff --git a/reviewboard/reviews/errors.py b/reviewboard/reviews/errors.py index 28d388eb087838328a57b6dfbde80503d5d31781..5d91d4d35f3dbc4f0a843bbc90b090ef8aafcd84 100644 --- a/reviewboard/reviews/errors.py +++ b/reviewboard/reviews/errors.py @@ -43,3 +43,31 @@ class NotModifiedError(PublishError): def __init__(self): super(NotModifiedError, self).__init__( 'The draft has no modifications.') + + +class DepthLimitExceededError(ValueError): + """An error that occurs when the maximum depth limit is exceeded. + + Review request actions cannot be arbitrarily nested. For example, if the + depth limit is 2, then this error would be triggered if an extension tried + to add a menu action as follows: + + .. code-block:: python + + BaseReviewRequestActionHook(self, actions=[ + DepthZeroMenuAction([ + DepthOneFirstItemAction(), + DepthOneMenuAction([ + DepthTwoMenuAction([ # This depth is acceptable. + DepthThreeTooDeepAction(), # This action is too deep. + ]), + ]), + DepthOneLastItemAction(), + ]), + ]) + """ + + def __init__(self, action_id, depth_limit): + super(DepthLimitExceededError, self).__init__( + '%s exceeds the maximum depth limit of %d' + % (action_id, depth_limit)) diff --git a/reviewboard/reviews/templatetags/reviewtags.py b/reviewboard/reviews/templatetags/reviewtags.py index 2674d22536ffa8e24b33aa9cd37fd650b44d7942..b23c5813e85dc6faff42232780760d28e1e70106 100644 --- a/reviewboard/reviews/templatetags/reviewtags.py +++ b/reviewboard/reviews/templatetags/reviewtags.py @@ -17,6 +17,7 @@ from djblets.util.decorators import basictag, blocktag from djblets.util.humanize import humanize_list from reviewboard.accounts.models import Profile, Trophy +from reviewboard.reviews.actions import get_top_level_actions from reviewboard.reviews.fields import (get_review_request_fieldset, get_review_request_fieldsets) from reviewboard.reviews.markdown_utils import (is_rich_text_default_for_user, @@ -312,6 +313,52 @@ def reviewer_list(review_request): for user in review_request.target_people.all()]) +@register.simple_tag(takes_context=True) +def review_request_actions(context): + """Render all registered review request actions. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template. + + Returns: + unicode: The HTML content to be rendered. + """ + content = [] + + for top_level_action in get_top_level_actions(): + try: + content.append(top_level_action.render(context)) + except Exception: + logging.exception('Error rendering top-level action %s', + top_level_action.action_id) + + return ''.join(content) + + +@register.simple_tag(takes_context=True) +def child_actions(context): + """Render all registered child actions. + + Args: + context (django.template.Context): + The collection of key-value pairs available in the template. + + Returns: + unicode: The HTML content to be rendered. + """ + content = [] + + for child_action in context['menu_action']['child_actions']: + try: + content.append(child_action.render(context)) + except Exception: + logging.exception('Error rendering child action %s', + child_action.action_id) + + return ''.join(content) + + @register.tag @blocktag(end_prefix='end_') def for_review_request_field(context, nodelist, review_request_details, diff --git a/reviewboard/reviews/tests.py b/reviewboard/reviews/tests.py index 2916359541cb4af51340a14800968af24dab75ad..480d4d116084c5f1e8134432adf8bf696e10650c 100644 --- a/reviewboard/reviews/tests.py +++ b/reviewboard/reviews/tests.py @@ -15,13 +15,22 @@ from djblets.auth.signals import user_registered from djblets.siteconfig.models import SiteConfiguration from djblets.testing.decorators import add_fixtures from kgb import SpyAgency +from mock import Mock from reviewboard.accounts.models import (Profile, LocalSiteProfile, _add_default_groups) from reviewboard.attachments.models import FileAttachment from reviewboard.changedescs.models import ChangeDescription -from reviewboard.reviews.errors import NotModifiedError, PublishError +from reviewboard.reviews.actions import (BaseReviewRequestAction, + BaseReviewRequestMenuAction, + clear_all_actions, + MAX_DEPTH_LIMIT, + register_actions, + unregister_actions) +from reviewboard.reviews.errors import (DepthLimitExceededError, + NotModifiedError, + PublishError) from reviewboard.reviews.forms import DefaultReviewerForm, GroupForm from reviewboard.reviews.markdown_utils import (markdown_render_conditional, normalize_text_for_edit) @@ -991,6 +1000,640 @@ class ReviewRequestTests(SpyAgency, TestCase): review_request.close(ReviewRequest.SUBMITTED) +class ActionTests(TestCase): + """Tests the actions in reviewboard.reviews.actions.""" + + fixtures = ['test_users'] + + class _FooAction(BaseReviewRequestAction): + action_id = 'foo-action' + label = 'Foo Action' + + class _BarAction(BaseReviewRequestAction): + action_id = 'bar-action' + label = 'Bar Action' + + class _BazAction(BaseReviewRequestMenuAction): + def __init__(self, action_id, child_actions=None): + super(ActionTests._BazAction, self).__init__(child_actions) + + self.action_id = 'baz-' + action_id + + class _TopLevelMenuAction(BaseReviewRequestMenuAction): + action_id = 'top-level-menu-action' + label = 'Top Level Menu Action' + + class _PoorlyCodedAction(BaseReviewRequestAction): + def get_label(self, context): + raise Exception + + def _get_content(self, user_pk='123', is_authenticated=True, + url_name='review-request-detail', local_site_name=None, + status=ReviewRequest.PENDING_REVIEW, submitter_id='456', + is_public=True, display_id='789', has_diffs=True, + can_change_status=True, can_edit_reviewrequest=True, + delete_reviewrequest=True): + request = Mock() + request.resolver_match = Mock() + request.resolver_match.url_name = url_name + request.user = Mock() + request.user.pk = user_pk + request.user.is_authenticated.return_value = is_authenticated + request._local_site_name = local_site_name + + review_request = Mock() + review_request.status = status + review_request.submitter_id = submitter_id + review_request.public = is_public + review_request.display_id = display_id + + if not has_diffs: + review_request.get_draft.return_value = None + review_request.get_diffsets.return_value = None + + context = Context({ + 'request': request, + 'review_request': review_request, + 'perms': { + 'reviews': { + 'can_change_status': can_change_status, + 'can_edit_reviewrequest': can_edit_reviewrequest, + 'delete_reviewrequest': delete_reviewrequest, + }, + }, + }) + + template = Template( + '{% load reviewtags %}' + '{% review_request_actions %}' + ) + + return template.render(context) + + def _get_long_action_list(self, length): + actions = [None] * length + actions[0] = self._BazAction('0') + + for d in range(1, len(actions)): + actions[d] = self._BazAction(str(d), [actions[d - 1]]) + + return actions + + def tearDown(self): + super(ActionTests, self).tearDown() + + # This prevents registered/unregistered/modified actions from leaking + # between different unit tests. + clear_all_actions() + + def test_register_then_unregister(self): + """Testing register then unregister for actions""" + foo_action = self._FooAction() + menu_action = self._TopLevelMenuAction([ + self._BarAction(), + ]) + + self.assertEqual(len(menu_action.child_actions), 1) + bar_action = menu_action.child_actions[0] + + content = self._get_content() + self.assertEqual(content.count('id="%s"' % foo_action.action_id), 0) + self.assertEqual(content.count('>%s<' % foo_action.label), 0) + self.assertEqual(content.count('id="%s"' % menu_action.action_id), 0) + self.assertEqual(content.count('>%s ▾<' % menu_action.label), 0) + self.assertEqual(content.count('id="%s"' % bar_action.action_id), 0) + self.assertEqual(content.count('>%s<' % bar_action.label), 0) + + foo_action.register() + + content = self._get_content() + self.assertEqual(content.count('id="%s"' % foo_action.action_id), 1) + self.assertEqual(content.count('>%s<' % foo_action.label), 1) + self.assertEqual(content.count('id="%s"' % menu_action.action_id), 0) + self.assertEqual(content.count('>%s ▾<' % menu_action.label), 0) + self.assertEqual(content.count('id="%s"' % bar_action.action_id), 0) + self.assertEqual(content.count('>%s<' % bar_action.label), 0) + + menu_action.register() + + content = self._get_content() + self.assertEqual(content.count('id="%s"' % foo_action.action_id), 1) + self.assertEqual(content.count('>%s<' % foo_action.label), 1) + self.assertEqual(content.count('id="%s"' % menu_action.action_id), 1) + self.assertEqual(content.count('>%s ▾<' % menu_action.label), 1) + self.assertEqual(content.count('id="%s"' % bar_action.action_id), 1) + self.assertEqual(content.count('>%s<' % bar_action.label), 1) + + foo_action.unregister() + + content = self._get_content() + self.assertEqual(content.count('id="%s"' % foo_action.action_id), 0) + self.assertEqual(content.count('>%s<' % foo_action.label), 0) + self.assertEqual(content.count('id="%s"' % menu_action.action_id), 1) + self.assertEqual(content.count('>%s ▾<' % menu_action.label), 1) + self.assertEqual(content.count('id="%s"' % bar_action.action_id), 1) + self.assertEqual(content.count('>%s<' % bar_action.label), 1) + + menu_action.unregister() + + content = self._get_content() + self.assertEqual(content.count('id="%s"' % foo_action.action_id), 0) + self.assertEqual(content.count('>%s<' % foo_action.label), 0) + self.assertEqual(content.count('id="%s"' % menu_action.action_id), 0) + self.assertEqual(content.count('>%s ▾<' % menu_action.label), 0) + self.assertEqual(content.count('id="%s"' % bar_action.action_id), 0) + self.assertEqual(content.count('>%s<' % bar_action.label), 0) + + def test_unregister_actions_with_register_actions(self): + """Testing unregister_actions with register_actions""" + foo_action = self._FooAction() + unregistered_ids = [ + 'discard-review-request-action', + 'update-review-request-action', + 'ship-it-action', + ] + removed_ids = unregistered_ids + [ + 'upload-diff-action', + 'upload-file-action', + ] + added_ids = [ + foo_action.action_id + ] + + # Test that foo_action really does render as a child of the parent + # Close menu (and not any other menu). + new_close_menu_html = '\n'.join([ + '
  • ', + ' Close ▾', + ' diff --git a/reviewboard/templates/extensions/action_dropdown.html b/reviewboard/templates/extensions/action_dropdown.html deleted file mode 100644 index 269bab960f26f1c3b89f4f84fcad9fdbe281d248..0000000000000000000000000000000000000000 --- a/reviewboard/templates/extensions/action_dropdown.html +++ /dev/null @@ -1,9 +0,0 @@ -{% load djblets_utils i18n staticfiles %} -
  • - {{actions.label}} ▾ - -
    • diff --git a/reviewboard/templates/reviews/action.html b/reviewboard/templates/reviews/action.html new file mode 100644 index 0000000000000000000000000000000000000000..3457c0736f7154ec05a72757e60d9d43d84053aa --- /dev/null +++ b/reviewboard/templates/reviews/action.html @@ -0,0 +1,4 @@ +
  • + +
    • diff --git a/reviewboard/templates/reviews/menu_action.html b/reviewboard/templates/reviews/menu_action.html new file mode 100644 index 0000000000000000000000000000000000000000..be7a6068b299d1074a0d1fbe866d83d26cd1b4ad --- /dev/null +++ b/reviewboard/templates/reviews/menu_action.html @@ -0,0 +1,8 @@ +{% load reviewtags %} +
  • + {{menu_action.label}} ▾ + +
    • diff --git a/reviewboard/templates/reviews/review_detail.html b/reviewboard/templates/reviews/review_detail.html index 40153a43a669aeabab9b459171540e8ad96fa347..bf924b6422c1cfa5b09256b4996ba04abaecb310 100644 --- a/reviewboard/templates/reviews/review_detail.html +++ b/reviewboard/templates/reviews/review_detail.html @@ -1,5 +1,5 @@ {% extends "reviews/reviewable_base.html" %} -{% load i18n djblets_deco djblets_js rb_extensions reviewtags staticfiles tz %} +{% load i18n djblets_deco djblets_js reviewtags staticfiles tz %} {% block title %}{{review_request_details.summary}} | {% trans "Review Request" %}{% endblock %} @@ -40,13 +40,7 @@
    • diff --git a/reviewboard/templates/reviews/review_request_actions_primary.html b/reviewboard/templates/reviews/review_request_actions_primary.html deleted file mode 100644 index 418b91e3fa80a181e08612d1686fff87fe2d7157..0000000000000000000000000000000000000000 --- a/reviewboard/templates/reviews/review_request_actions_primary.html +++ /dev/null @@ -1,5 +0,0 @@ -{% load i18n %} -{% if request.user.is_authenticated %} -
  • {% trans "Review" %}
    • -
  • {% trans "Ship It!" %}
    • -{% endif %} diff --git a/reviewboard/templates/reviews/review_request_actions_secondary.html b/reviewboard/templates/reviews/review_request_actions_secondary.html deleted file mode 100644 index 5065897b30394c43ccb597e96547b184ac29b0e2..0000000000000000000000000000000000000000 --- a/reviewboard/templates/reviews/review_request_actions_secondary.html +++ /dev/null @@ -1,31 +0,0 @@ -{% load djblets_utils i18n staticfiles %} - -{% if request.user.pk == review_request.submitter_id or perms.reviews.can_change_status and review_request.public %} -{% if review_request.status == 'P' %} -
  • - {% trans "Close" %} ▾ - -
    • -{% endif %} -{% endif %} -{% if request.user.pk == review_request.submitter_id or perms.reviews.can_edit_reviewrequest %} -{% if review_request.status == 'P' %} -
  • - {% trans "Update" %} ▾ - -
    • -{% endif %} -{% endif %} diff --git a/reviewboard/templates/reviews/ui/base.html b/reviewboard/templates/reviews/ui/base.html index 405a8f4f9a4d8e39b2a71611efc177d71e7f3183..fd0e7b84d9979798b16ee6a13310434979904368 100644 --- a/reviewboard/templates/reviews/ui/base.html +++ b/reviewboard/templates/reviews/ui/base.html @@ -33,11 +33,7 @@
    • diff --git a/reviewboard/urls.py b/reviewboard/urls.py index e3e159f8aa6a84fd0e6fb191a51ac5ce0154e3ce..869a1016fb82d9732754169f7378e977b9757e0f 100644 --- a/reviewboard/urls.py +++ b/reviewboard/urls.py @@ -33,8 +33,10 @@ reviewable_url_names = diffviewer_url_names + [ 'screenshot', ] +main_review_request_url_name = 'review-request-detail' + review_request_url_names = diffviewer_url_names + [ - 'review-request-detail', + main_review_request_url_name, ] diff --git a/reviewboard/webapi/tests/test_webhook.py b/reviewboard/webapi/tests/test_webhook.py index 96bca22b988caa8130928e11271b5d4392615c04..cb3b51c7c0d6098d42719d679eecef90a1e34b64 100644 --- a/reviewboard/webapi/tests/test_webhook.py +++ b/reviewboard/webapi/tests/test_webhook.py @@ -123,7 +123,7 @@ class ResourceListTests(ExtraDataListMixin, BaseWebAPITestCase): @add_fixtures(['test_scmtools']) def test_post_all_repositories_not_same_local_site(self): - """Testing adding a webhook with a local site adnd custom repositories + """Testing adding a webhook with a local site and custom repositories that are not all in the same local site """ local_site_1 = LocalSite.objects.create(name='local-site-1')