diff --git a/docs/manual/extending/coderef/index.rst b/docs/manual/extending/coderef/index.rst index 276717f0b23a7ebf1b46b7a3bb5b3bc817497f52..dcce86e8bb8cf9c5a9da459cf5c751b16a119447 100644 --- a/docs/manual/extending/coderef/index.rst +++ b/docs/manual/extending/coderef/index.rst @@ -44,6 +44,17 @@ User Accounts reviewboard.accounts.forms.registration +Actions +======= + +.. autosummary:: + :toctree: python + + reviewboard.actions.base + reviewboard.actions.header + reviewboard.actions.registry + + File Attachments ================ @@ -168,7 +179,6 @@ Review Requests and Reviews reviewboard.reviews.actions reviewboard.reviews.chunk_generators reviewboard.reviews.context - reviewboard.reviews.default_actions reviewboard.reviews.detail reviewboard.reviews.errors reviewboard.reviews.fields diff --git a/docs/manual/extending/extensions/hooks/action-hooks.rst b/docs/manual/extending/extensions/hooks/action-hooks.rst index 30fee53cc6daa089ff0385e94dd16af81fab25ea..4815966aaf3f6d8e55d3b207a4b94a695c7b8c29 100644 --- a/docs/manual/extending/extensions/hooks/action-hooks.rst +++ b/docs/manual/extending/extensions/hooks/action-hooks.rst @@ -10,22 +10,27 @@ 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: +.. autosummary:: + + ~reviewboard.extensions.hooks.ReviewRequestActionHook + ~reviewboard.extensions.hooks.DiffViewerActionHook + ~reviewboard.extensions.hooks.HeaderActionHook + +These hooks accept a subclass of :py:class:`~reviewboard.actions.base.BaseAction`, depending on the hook being used: + +:py:class:`~reviewboard.reviews.actions.ReviewRequestAction`: + This action class is used for `~.hooks.DiffViewerActionHook` and + `~.hooks.ReviewRequestActionHook`. + +:py:class:`~reviewboard.actinos.header.HeaderAction`: + This action class is used for `~.hooks.HeaderActionHook`. + +These classes are intended to be subclassed by your extension to provide customized behaviour. + +These hooks also support old-style action hook dictionaries from before Review +Board 4.0. A list of dictionaries that define the actions to be inserted can be +passed instead of a subclass of the above classes. These dictionaries must have +the following keys: * **id**: The ID of the action (optional) @@ -48,56 +53,177 @@ the actions you'd like to insert. These dictionaries have the following fields: * **image_height**: The height of the image (optional). -There are also two hooks to provide drop-down menus in the action bars: +.. versionchanged:: 4.0 + * The :py:class:`~.hooks.ActionHook` class is now deprecated. + * The method of passing in a list of dictionaries to these hooks is + deprecated. Instead, a list of subclasses of :py:class:`~reviewboard. + actions.base.BaseAction` should be passed instead. + +.. seealso:: The :py:mod:`reviewboard.actions` module. + +There are also hooks to provide dropdown menus: + +.. autosummary:: + + ~reviewboard.extensions.hooks.DiffViewerDropdownActionHook + ~reviewboard.extensions.hooks.ReviewRequestDropdownActionHook + ~reviewboard.extensions.hooks.HeaderDropdownActionHook -+---------------------------------------------+-------------------------+ -| Class | Location | -+=============================================+=========================+ -| :py:class:`ReviewRequestDropdownActionHook` | The bar at the top of a | -| | review request. | -+---------------------------------------------+-------------------------+ -| :py:class:`HeaderDropdownActionHook` | The page header. | -+---------------------------------------------+-------------------------+ +These hooks are work the same as the basic action hooks, except they accept a +list of subclasses of :py:class:`~reviewboard.actions.base.BaseMenuAction`. +Again, which subclass to use depends on the hook being used: + +:py:class:`~reviewboard.reviews.actions.ReviewRequestMenuAction`: + This action class is used for + :py:class:`~.hooks.DiffViewerDropdownActionHook` and + :py:class:`~.hooks.ReviewRequestDropdownActionHook` + +:py:class:`~reviewboard.actions.header.HeaderMenuAction`: + This action class is used for :py:class:`~.hooks.HeaderDropdownActionHook`. 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. +.. versionchanged:: 4.0 + + * Up to two levels of action nesting are now possible. + * The method of passing in a list of dictionaries to these hooks is + deprecated. Instead, a list of subclasses of :py:class:`~reviewboard. + actions.base.BaseMenuAction` should be pased instead. + +.. seealso:: The :py:mod:`reviewboard.actions` module. + + +Modifying Review Request Actions +================================ + +.. versionadded:: 3.0 + +The :py:data:`reviewboard.reviews.actions.review_request_actions` registry is +used to remove (and re-add) default review request actions from an extension. +Specifically the :py:meth:`~reviewboard.reviews.actions. +ReviewRequestActionRegistry.unregister` method is used to remove default +actions and the :py:meth:`reviewboard.reviews.actions. +ReviewRequestActionRegistry.register` method is used to re-add default actions. + +Note: any third-party actions should use one of the hooks above instead of +directly mutating the state of the actions registries. + 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': '...', - }, - ], - }, - ]) + from reviewboard.extensions.base import Extension + from reviewboard.extensions.hooks import (HeaderDropdownActionHook, + ReviewRequestActionHook, + ReviewRequestDropdownActionHook) + from reviewboard.reviews.actions import (CloseMenuAction, + ReviewRequestAction, + ReviewRequestMenuAction, + review_request_actions) + from reviewboard.urls import reviewable_url_names + + + class NewCloseAction(ReviewRequestAction): + action_id = 'new-close-action' + label = 'New Close Action!' + + + class SampleMenuAction(ReviewRequestMenuAction): + action_id = 'sample-menu-action' + label = 'Sample Menu' + + + class FirstItemAction(ReviewRequestAction): + action_id = 'first-item-action' + label = 'First Item' + + + class SampleSubmenuAction(ReviewRequestMenuAction): + action_id = 'sample-submenu-action' + label = 'Sample Submenu' + + + class SubItemAction(ReviewRequestAction): + action_id = 'sub-item-action' + label = 'Sub Item' + + + class LastItemAction(ReviewRequestAction): + action_id = 'last-item-action' + label = 'Last Item' + + + class ReviewableDropdownActionHook(ReviewRequestDropdownActionHook): + """A special case action hook. + + This hook renders the given dropdown menu on: + + * Review request pages. + * Diffviewer pages + * File attachment pages. + """ + + default_apply_to = reviewable_url_names + + + class SampleExtension(Extension): + def initialize(self): + # Register a new action in the "Close" menu. + review_request_actions.register( + NewCloseAction(), + parent_id=CloseMenuAction.action_id) + + # 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. + ReviewableDropdownActionHook( + 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() + + # Since this action was not registered via a hook, we must manually + # remove it. + review_request_actions.unregister_by_attr( + 'action_id', + NewCloseAction.action_id) diff --git a/docs/manual/extending/index.rst b/docs/manual/extending/index.rst index 425a63c4606dc80fefbf7cb71fb831282dbea783..2252841cb96555aa91931d23a0e6b01a2ac3f902 100644 --- a/docs/manual/extending/index.rst +++ b/docs/manual/extending/index.rst @@ -84,10 +84,11 @@ extension hooks available to you. Adds a new avatar service, which can be used to provide pictures for user accounts. -:ref:`Action Hooks `: +**Action Hooks**: A series of hooks used to add new actions for review requests, the diff viewer, and the account/support header at the top of the page. + The following action hooks are available: .. To do: Add dedicated doc pages for these. @@ -107,6 +108,10 @@ extension hooks available to you. :py:class:`~reviewboard.extensions.hooks.ReviewRequestDropdownActionHook`: Adds drop-down actions to the review request actions bar. + .. seealso:: + + Information about writing :ref:`action-hooks`. + **API Hooks:** :ref:`webapi-capabilities-hook`: Adds new capability flags to the :ref:`root API resource diff --git a/reviewboard/actions/__init__.py b/reviewboard/actions/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/reviewboard/actions/base.py b/reviewboard/actions/base.py new file mode 100644 index 0000000000000000000000000000000000000000..217dc3d2e8c5495657ce93f50114377bd1b43f02 --- /dev/null +++ b/reviewboard/actions/base.py @@ -0,0 +1,195 @@ +"""Base classes for actions.""" + +from __future__ import unicode_literals + +from django.template.loader import render_to_string + + +class BaseAction(object): + """A base class for an action. + + Creating an action requires subclassing :py:class:`BaseAction` and = + overriding any fields and/or methods as desired. Different instances of the + same subclass can override the class fields with their own instance + fields. + + This class is not intended to be subclassed directly. Instead, one of the + following classes should be subclassed, depending on where you want the + action to be rendered. + + * :py:class:`reviewboard.actions.header.HeaderAction` + * :py:class:`reviewboard.actions.header.HeaderMenuAction` + * :py:class:`reviewboard.reviews.actions.ReviewRequestAction` + * :py:class:`reviewboard.reviews.actions.ReviewRequestMenuAction` + + Classes subclassing :py:class:`~reviewboard.actions.header.HeaderAction` or + :py:class:`~reviewboard.actions.header.HeaderMenuAction` will be rendered + in the page header. + + Classes subclassing + :py:class:`~reviewboard.reviews.actions.ReviewRequestAction` and + :py:class:`~reviewboard.reviews.actions.ReviewRequestMenuAction` will be + rendered in the review request header on review request and diff viewer + pages. + + Example: + .. code-block:: python + + class UsedOnceAction(HeaderAction): + action_id = 'once' + label = 'This is used once' + + class UsedMultipleAction(HeaderAction): + def __init__(self, action_id, label): + self.action_id = 'repeat-%s' % action_id + self.label = label + Note: + Since the same action will be rendered for multiple users in a + multi-threaded environment, the action's state should not be modified + after initialization. If different attributes are required at runtime, + the getter methods such as :py:meth:`get_label` can be overridden + instead. By default, these methods just return the original attribute. + """ + + @property + def registry(self): + raise NotImplementedError('%r (type: %s) does not have the registry ' + 'attribute set' + % (self, type(self))) + + #: The ID of this action. + #: + #: This must be unique across all types of actions. + action_id = None + + #: The label for the action. + #: + #: This will be displayed to the user. + label = None + + #: The URL to invoke if this action is clicked. + url = '#' + + #: Whether or not this action should be hidden from the user. + hidden = False + + template_name = 'actions/action.html' + context_key = 'action' + + def get_label(self, context): + """Return the label of this action. + + By default, this returns :py:attr:`label`. subclasses can override + this behaviour. + + Args: + context (django.template.Context): + The parent rendering context. + + + Returns: + unicode: + The label. + """ + return self.label + + def get_url(self, context): + """Return the target URL of this action. + + By default, this returns :py:attr:`url`. subclasses can override this + behaviour. + + Args: + context (django.template.Context): + The parent rendering context. + + Returns: + unicode: + The target URL. + """ + return self.url + + def get_hidden(self, context): + """Return whether or not this action should be hidden. + + By default, this returns :py:attr:`hidden`. subclasses can override + this behaviour. + + Args: + context (django.template.Context): + The parent rendering context. + + Returns: + bool: + Whether or not this action should be hidden. + """ + return self.hidden + + def should_render(self, context): + """Return whether or not this action should be rendered. + + By default, this always returns ``True``. subclasses can override this + behaviour. + + Args: + context (django.template.Context): + The parent rendering context. + + Returns: + bool: + Whether or not this action should be rendered. + """ + return True + + def render(self, context): + if self.should_render(context): + context.push() + + try: + context[self.context_key] = self.get_render_context(context) + from pprint import pprint; pprint(context) + return render_to_string(self.template_name, context) + finally: + context.pop() + + return '' + + def get_render_context(self, context): + """Return the rendering context for this action. + + Args: + context (django.template.Context): + The parent rendering context. + + Returns: + dict: + A dictionary of information necessary to render this action. + """ + return { + 'id': self.action_id, + 'label': self.get_label(context), + 'url': self.get_url(context), + 'hidden': self.get_hidden(context) + } + + def __repr__(self): + return '<%s(action_id=%s)>' % (type(self).__name__, self.action_id) + + +class BaseMenuAction(BaseAction): + """The base class for menu actions. + + This class is not intended to be subclassed directly. + """ + + context_key = 'menu' + template_name = 'actions/header_action_dropdown.html' + + @property + def child_actions(self): + return self.registry.get_child_actions(self.action_id) + + def get_render_context(self, context): + values = super(BaseMenuAction, self).get_render_context(context) + values['child_actions'] = list(self.child_actions) + return values diff --git a/reviewboard/actions/header.py b/reviewboard/actions/header.py new file mode 100644 index 0000000000000000000000000000000000000000..cf0d2a368be2e53f0ce34a77b53bc4aa6a3fee17 --- /dev/null +++ b/reviewboard/actions/header.py @@ -0,0 +1,40 @@ +"""Header actions.""" + +from reviewboard.actions.base import BaseAction, BaseMenuAction +from reviewboard.actions.registry import ActionRegistry + +header_actions = ActionRegistry() + + +class HeaderAction(BaseAction): + registry = header_actions + + #: An optional image URL to render with this action. + image = None + + #: The + image_width = None + image_height = None + + def get_image(self, context): + return self.image + + def get_image_height(self, context): + return self.image_height + + def get_image_width(self, context): + return self.image_width + + def get_render_context(self, context): + values = super(HeaderAction, self).get_render_context(context) + + values.update({ + 'image': self.get_image(context), + 'image_height': self.get_image_height(context), + 'image_width': self.get_image_width(context), + }) + return values + + +class HeaderMenuAction(BaseMenuAction): + registry = header_actions diff --git a/reviewboard/actions/registry.py b/reviewboard/actions/registry.py new file mode 100644 index 0000000000000000000000000000000000000000..9f77779c1f883f8e358535946e6e95c01c91440f --- /dev/null +++ b/reviewboard/actions/registry.py @@ -0,0 +1,213 @@ +"""The registry for actions.""" + +from collections import OrderedDict + +from django.utils.translation import ugettext_lazy as _ +from djblets.registries.registry import ALREADY_REGISTERED, NOT_REGISTERED + +from reviewboard.actions.base import BaseAction +from reviewboard.registries.registry import Registry +from reviewboard.reviews.errors import DepthLimitExceededError + + +PARENT_IS_CHILD = 'PARENT_IS_CHILD' +PARENT_NOT_FOUND = 'PARENT_NOT_FOUND' + + +class ActionRegistry(Registry): + """A registry for tracking a set of actions.""" + + default_errors = dict( + Registry.default_errors, + **{ + ALREADY_REGISTERED: _( + 'Could not register action %(item)s: it is already registered.' + ), + + NOT_REGISTERED: _( + 'No action with %(attr_name)s = %(attr_value)r registered.' + ), + + PARENT_IS_CHILD: _( + 'Could not retrieve children of action with action_id = ' + '%(parent_id)s: this action is a child action.' + ), + + PARENT_NOT_FOUND: _( + 'Could not retrieve children of action with action_id = ' + '%(parent_id)s: this action is not registered.' + ), + } + ) + + lookup_attrs = ('action_id',) + + lookup_error_class = KeyError + + #: The maximum nesting depth for actions. + MAX_DEPTH = 2 + + def __init__(self): + """Initialize""" + super(ActionRegistry, self).__init__() + + # A mapping of top-level actions to the IDs of their children. + # + # The keys in this dictionary will be all the top-level actions. Any + # element registered that does not have its ID as a key in this + # dictionary therefore *must* be a child action. + self._child_actions = OrderedDict() + + # A mapping of child action IDs to the IDs of their parents. + self._action_parents = {} + + def register(self, action, parent_id=None): + """Register an action + + Args: + action (BaseReviewRequestAction): + The action to register. + + parent_id (unicode, optional): + The ID of this action's parent, if any. + + If provided, the action will be nested under that action. + Actions may only be nested a single level. + + Raises: + djblets.registries.errors.ItemLookupError: + If the parent action is not registered. + + reviewboard.reviews.errors.DepthLimitExceededError: + If the action would be nested too deeply. + """ + if parent_id: + if self.get_action(parent_id) is None: + raise self.lookup_error_class(self.format_error( + NOT_REGISTERED, + attr_name='action_id', + attr_value=parent_id, + )) + elif parent_id not in self._child_actions: + raise DepthLimitExceededError(action.action_id, self.MAX_DEPTH) + else: + self._child_actions[parent_id].append(action.action_id) + self._action_parents[action.action_id] = parent_id + else: + self._child_actions[action.action_id] = [] + + super(ActionRegistry, self).register(action) + + def populate(self): + if self._populated: + return + + self._populated = True + + for item in self.get_defaults(): + if isinstance(item, tuple): + parent, children = item + assert isinstance(parent, BaseAction) + + self.register(parent) + + for child in children: + self.register(child, parent_id=parent.action_id) + else: + assert isinstance(item, BaseAction) + self.register(item) + + def reset(self): + print 'reset()' + print list(self.get_root_actions()) + print list(self) + if self.populated: + for root_action in list(self.get_root_actions()): + self.unregister(root_action) + + super(ActionRegistry, self).reset() + + def unregister(self, action): + """Unregister an action. + + Args: + action (BaseReviewRequestAction): + The action to unregister. + + Raises: + djblets.registries.errors.ItemLookupError: + Raised if the item is not found in the registry. + """ + super(ActionRegistry, self).unregister(action) + + if action.action_id in self._child_actions: + # This is a parent action and we must unregister all child actions. + child_ids = self._child_actions[action.action_id] + + for child_id in list(child_ids): + self.unregister_by_attr('action_id', child_id) + + del self._child_actions[action.action_id] + else: + parent_id = self._action_parents.pop(action.action_id) + self._child_actions[parent_id].remove(action.action_id) + + def get_action(self, action_id): + """Return the action with the given action ID. + + Args: + action_id (unicode): + The ID of the action to return. + + Returns: + BaseReviewRequestAction: + The action if it is registered or ``None`` otherwise. + """ + return self.get('action_id', action_id) + + def get_child_actions(self, parent_id): + """Yield the child actions. + + Args: + parent_id (unicode): + The ID of the parent whose children are to be yielded. + + Yields: + BaseReviewRequestAction: + The review request action instances that are children of the given + action. + + Raises: + djblets.registries.errors.ItemLookupError: + If the parent action is not registered. + + ValueError: + If the parent action is not actually a parent action. + """ + self.populate() + + if self.get_action(parent_id) is None: + raise self.lookup_error_class(self.format_error( + PARENT_NOT_FOUND, + parent_id=parent_id + )) + elif parent_id not in self._child_actions: + raise ValueError(self.format_error( + PARENT_IS_CHILD, + parent_id=parent_id + )) + + for action_id in self._child_actions[parent_id]: + yield self.get_action(action_id) + + def get_root_actions(self): + """Yield the top-level actions. + + Yields: + BaseReviewRequestAction: + The top-level registered review request action instances. + """ + self.populate() + + for action_id in self._child_actions: + yield self.get_action(action_id) diff --git a/reviewboard/actions/templatetags/__init__.py b/reviewboard/actions/templatetags/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/reviewboard/actions/templatetags/actions.py b/reviewboard/actions/templatetags/actions.py new file mode 100644 index 0000000000000000000000000000000000000000..90d05829ed17746e286327860cdb2c5ec0b31306 --- /dev/null +++ b/reviewboard/actions/templatetags/actions.py @@ -0,0 +1,33 @@ +"""Action templatetags.""" + +from __future__ import unicode_literals + +import logging + +from django.template import Library + + +register = Library() + + +@register.simple_tag(takes_context=True) +def child_actions(context, menu): + """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 menu.get('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) diff --git a/reviewboard/actions/tests.py b/reviewboard/actions/tests.py new file mode 100644 index 0000000000000000000000000000000000000000..75873d9642fdebda7562f199731e4fbff2a18a6a --- /dev/null +++ b/reviewboard/actions/tests.py @@ -0,0 +1,145 @@ +from __future__ import unicode_literals + +from django.template import Context +from djblets.registries.errors import AlreadyRegisteredError + +from reviewboard.actions.registry import ActionRegistry +from reviewboard.actions.base import BaseAction, BaseMenuAction +from reviewboard.reviews.errors import DepthLimitExceededError +from reviewboard.testing import TestCase + + +class ItemAction(BaseAction): + """An item action for testing.""" + + @property + def registry(self): + return self._registry + + def __init__(self, registry, action_id='item-action', label='Item Action'): + self._registry = registry + self.action_id = action_id + self.label = label + + +class MenuAction(BaseMenuAction): + """A menu action for testing.""" + + @property + def registry(self): + return self._registry + + def __init__(self, registry, action_id='menu-action', label='Menu Action'): + self._registry = registry + self.action_id = action_id + self.label = label + + +class PoorlyCodedAction(ItemAction): + """An action for testing that raises an exception.""" + + def get_label(self, context): + raise Exception() + + +class ActionRegistryTests(TestCase): + """Unit tests for the review request actions registry.""" + + def test_register_actions_with_invalid_parent_id(self): + """Testing ActionRegistry.register with an invalid parent ID""" + registry = ActionRegistry() + action = ItemAction(registry) + + with self.assertRaises(KeyError): + registry.register(action, parent_id='bad-id') + + def test_register_actions_with_already_registered_action(self): + """Testing ActionRegistry.register with an already registered action""" + registry = ActionRegistry() + action = ItemAction(registry) + + registry.register(action) + + with self.assertRaises(AlreadyRegisteredError): + registry.register(action) + + def test_register_actions_with_too_deep(self): + """Testing ActionRegistry.register with exceeding max nesting""" + class TestingActionRegistry(ActionRegistry): + def get_defaults(self): + return [ + (MenuAction(self), ( + ItemAction(self, action_id='nested'), + )), + ] + + registry = TestingActionRegistry() + invalid_action = ItemAction(registry, action_id='invalid') + + with self.assertRaises(DepthLimitExceededError): + registry.register(invalid_action, parent_id='nested') + + def test_unregister_actions(self): + """Testing ActionRegistry.unregister""" + class TestingActionRegistry(ActionRegistry): + def get_defaults(self): + return [ + (MenuAction(self), ( + ItemAction(self, action_id='nested-item'), + )), + ItemAction(self, action_id='top-level-item'), + ] + + registry = TestingActionRegistry() + + self.assertEqual(len(registry), 3) + + registry.unregister_by_attr('action_id', 'nested-item') + registry.unregister_by_attr('action_id', 'top-level-item') + + self.assertEqual(set(registry), + {registry.get_action('menu-action')}) + + def test_unregister_actions_with_child_action(self): + """Testing ActionRegistry.unregister with nested actions""" + class TestingActionRegistry(ActionRegistry): + def get_defaults(self): + return [ + (MenuAction(self), ( + ItemAction(self), + )) + ] + + registry = TestingActionRegistry() + + self.assertEqual(len(registry), 2) + + registry.unregister_by_attr('action_id', 'menu-action') + + self.assertEqual(set(registry), set()) + + def test_unregister_actions_with_unregistered_action(self): + """Testing ActionRegistry.unregister with unregistered action""" + registry = ActionRegistry() + foo_action = ItemAction(registry) + + with self.assertRaises(KeyError): + registry.unregister(foo_action) + + +class BaseActionTests(TestCase): + """Unit tests for BaseAction.""" + + def test_render_pops_context_even_after_error(self): + """Testing BaseAction.render pops the context after an exception""" + registry = ActionRegistry() + + context = Context({'comment': 'this is a comment'}) + old_dict_count = len(context.dicts) + poorly_coded_action = PoorlyCodedAction(registry) + + with self.assertRaises(Exception): + poorly_coded_action.render(context) + + new_dict_count = len(context.dicts) + self.assertEquals(old_dict_count, new_dict_count) diff --git a/reviewboard/extensions/actions.py b/reviewboard/extensions/actions.py new file mode 100644 index 0000000000000000000000000000000000000000..de0eee3583868772abfa007a04be434e6fee1f0f --- /dev/null +++ b/reviewboard/extensions/actions.py @@ -0,0 +1,46 @@ +from copy import copy + + +class DictActionMixin(object): + """An action for ActionHook-style dictionaries. + + For backwards compatibility, actions may also be supplied as a dictionary + of information. This mixin is used (with an appropriate base class) to + convert such a dictionary into a new-style action class. + """ + + def __init__(self, action, applies_to=None): + """Initialize this action. + + Args: + action (dict): + A dictionary representing this action, as specified by the + :py:class:`ActionHook` class. + + applies_to (callable, optional): + A callable that examines a given context and determines if this + action applies to the page. + """ + action = copy(action) + + try: + action['action_id'] = action.pop('id') + except KeyError: + action['action_id'] = ('%s-action' + % action['label'].lower().replace(' ', '-')) + + self.__dict__.update(action) + 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 is None or self._applies_to(context['request']) diff --git a/reviewboard/extensions/hooks.py b/reviewboard/extensions/hooks.py index 6bc8a3025773518c21212d326d88c2ac3a0e244b..5959072f16d24535adc82d5a226f50f9e8ecf852 100644 --- a/reviewboard/extensions/hooks.py +++ b/reviewboard/extensions/hooks.py @@ -7,6 +7,7 @@ import warnings from django.template.context import RequestContext from django.template.loader import render_to_string from django.utils import six +from django.utils.functional import cached_property from djblets.extensions.hooks import (AppliesToURLMixin, BaseRegistryHook, BaseRegistryMultiItemHook, @@ -22,6 +23,10 @@ from djblets.registries.errors import ItemLookupError from reviewboard.accounts.backends import auth_backends from reviewboard.accounts.pages import AccountPage +from reviewboard.actions.base import BaseAction +from reviewboard.actions.header import (HeaderAction, HeaderMenuAction, + header_actions) +from reviewboard.extensions.actions import DictActionMixin from reviewboard.admin.widgets import (register_admin_widget, unregister_admin_widget) from reviewboard.attachments.mimetypes import (register_mimetype_handler, @@ -34,8 +39,9 @@ from reviewboard.hostingsvcs.service import (register_hosting_service, from reviewboard.integrations.base import GetIntegrationManagerMixin from reviewboard.notifications.email import (register_email_hook, unregister_email_hook) -from reviewboard.reviews.actions import (BaseReviewRequestAction, - BaseReviewRequestMenuAction) +from reviewboard.reviews.actions import (ReviewRequestAction, + ReviewRequestMenuAction, + review_request_actions) from reviewboard.reviews.features import class_based_actions_feature from reviewboard.reviews.fields import (get_review_request_fieldset, register_review_request_fieldset, @@ -836,17 +842,40 @@ class ActionHook(ExtensionHook): ``image_height`` (optional): The height of the image. - If our hook needs to access the template context, then it can override + If the hook needs to access the template context, then it can override :py:meth:`get_actions` and return results from there. + + .. deprecated: 4.0 + + :py:class:`ReviewRequestActionHook` should be used instead of this + class. """ + @property + def registry(self): + """The registry for the hook. + + Subclasses should override this. + """ + warnings.warn( + 'ActionHook() is no longer meant to be used directly. Use ' + 'ReviewRequestActionHook() instead.', + DeprecationWarning) + return review_request_actions + + @cached_property + def dict_action_class(self): + return type(b'DictAction', (DictActionMixin, self.base_action_class), { + 'registry': self.registry, + }) + def initialize(self, actions=None, *args, **kwargs): """Initialize this action hook. Args: actions (list, optional): The list of actions (of type :py:class:`dict` or - :py:class:`~.actions.BaseReviewRequestAction`) to be added. + :py:class:`~.actions.BaseAction`) to be added. *args (tuple): Extra positional arguments. @@ -854,7 +883,26 @@ class ActionHook(ExtensionHook): **kwargs (dict): Extra keyword arguments. """ - self.actions = actions or [] + if actions is None: + actions = [] + + if (not class_based_actions_feature.is_enabled() and + any(not isinstance(action, dict) for action in actions)): + logging.error( + 'The class-based actions API is experimental and will ' + 'change in a future release. It must be enabled before ' + 'it can be used. The actions from %r will not be ' + 'registered.' + % self + ) + actions = [] + + actions = [ + self.normalize_action(action) + for action in actions + ] + + self.actions = self._register_actions(actions) def get_actions(self, context): """Return the list of action information for this action hook. @@ -864,177 +912,54 @@ class ActionHook(ExtensionHook): The collection of key-value pairs available in the template. Returns: - list: The list of action information for this action hook. + 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-dict-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-dict-menu-action' % self.label.lower().replace(' ', '-')) - self._applies_to = applies_to - - def should_render(self, context): - """Return whether or not this action should render. + def normalize_action(self, action): + """Convert the given dictionary to a review request action instance. Args: - context (django.template.Context): - The collection of key-value pairs available in the template - just before this action is to be rendered. + action (dict or BaseAction): + A dictionary representing a review request action, as specified + by the :py:class:`ActionHook` class. Returns: - bool: Determines if this action should render. - """ - return self._applies_to(context['request']) - - -@six.add_metaclass(ExtensionHookPoint) -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 initialize(self, actions=None, apply_to=None, *args, **kwargs): - """Initialize this action hook. - - Args: - 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 positional arguments. - - **kwargs (dict): - Extra keyword arguments. + BaseAction: + The corresponding review request action instance. Raises: KeyError: - Some dictionary is not an :py:class:`ActionHook`-style + The given 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).initialize( - apply_to=apply_to or [], - *args, **kwargs) - - if actions is None: - actions = [] + if isinstance(action, BaseAction): + return action + elif isinstance(action, dict): + for key in ('label', 'url'): + if key not in action: + raise KeyError('ActionHook-style dicts require a %r key' + % repr(key)) - if (not class_based_actions_feature.is_enabled() and - any(not isinstance(action, dict) for action in actions)): - logging.error( - 'The class-based actions API is experimental and will ' - 'change in a future release. It must be enabled before ' - 'it can be used. The actions from %r will not be ' - 'registered.' - % self - ) - actions = [] + return self.__from_dict_action(action) + else: + raise TypeError('Unexpected action "%r"; expected dict or ' + 'BaseAction' + % action) self.actions = self._register_actions(actions) def shutdown(self): """Shutdown the hook and unregister all actions.""" for action in self.actions: - action.unregister() + self.registry.unregister(action) + + def __from_dict_action(self, action): + if isinstance(self, AppliesToURLMixin): + return self.dict_action_class(action, self.applies_to) + else: + return self.dict_action_class(action) def _register_actions(self, actions): """Register the given list of review request actions. @@ -1042,100 +967,111 @@ class BaseReviewRequestActionHook(AppliesToURLMixin, ActionHook): Args: actions (list, optional): The list of actions (of type :py:class:`dict` or - :py:class:`~.actions.BaseReviewRequestAction`) to be added. + :py:class:`~.actions.BaseAction`) to be added. Returns: - list of BaseReviewRequestAction: + list of BaseAction: The list of all registered actions. Raises: KeyError: - Some dictionary is not an :py:class:`ActionHook`-style - dictionary. + A 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. + TypeError: + A provided action was an improper type. """ 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. + # original actions. for action in reversed(actions): - action = self._normalize_action(action) - action.register() - registered_actions.append(action) - - registered_actions.reverse() + if isinstance(action, tuple): + parent, children = action + self.registry.register(parent) - return registered_actions + for child in children: + self.registry.register(child, parent_id=parent.action_id) + registered_actions.append(child) - 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. + registered_actions.append(parent) + else: + assert isinstance(action, BaseAction) + self.registry.register(action) + registered_actions.append(action) - Raises: - KeyError: - The given dictionary is not an :py:class:`ActionHook`-style - dictionary. + return registered_actions - 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) +class DropdownActionHookMixin(object): + """A mixin for registering dropdown menus with action hooks.""" - raise ValueError('Only BaseReviewRequestAction and dict instances are ' - 'supported') + @cached_property + def dict_menu_action_class(self): + return type(b'DictMenuAction', + (DictActionMixin, self.base_menu_action_class), + {'registry': self.registry}) - def convert_action(self, action_dict): + def normalize_action(self, action): """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 action, as specified - by the :py:class:`ActionHook` class. + action (dict or tuple): + A dictionary representing a review request menu action, as + specified by the :py:class:`ReviewRequestDropdownActionHook` + class. Returns: - BaseReviewRequestAction: - The corresponding review request action instance. + tuple: + A 2-tuple of: + + * The action dropdown (:py:class:`DictMenuAction`) + * The list of child actions (:py:class:`list` of + :py:class:`DictActions `). Raises: KeyError: - The given dictionary is not an :py:class:`ActionHook`-style - dictionary. + The given review request menu action dictionary is not a + :py:class:`ReviewRequestDropdownActionHook`-style dictionary. """ - for key in ('label', 'url'): - if key not in action_dict: - raise KeyError('ActionHook-style dicts require a %s key' - % repr(key)) + if isinstance(action, tuple): + return action + elif isinstance(action, dict): + for key in ('label', 'items'): + if key not in action: + raise KeyError( + 'ReviewRequestDropdownActionHook-style dicts require ' + 'a %r key' + % key + ) + + return ( + self.__action_from_dict(action), + [ + super(DropdownActionHookMixin, self).normalize_action( + child_action + ) + for child_action in action['items'] + ] + ) + else: + raise TypeError('Unexpected action "%r"; expected dict or ' + 'tuple' + % action) - return _DictAction(action_dict, self.applies_to) + def __action_from_dict(self, action): + if isinstance(self, AppliesToURLMixin): + return self.dict_menu_action_class(action, self.applies_to) + else: + return self.dict_menu_action_class(action) @six.add_metaclass(ExtensionHookPoint) -class ReviewRequestActionHook(BaseReviewRequestActionHook): +class ReviewRequestActionHook(AppliesToURLMixin, ActionHook): """A hook for adding review request actions to review request pages. By default, actions that are passed into this hook will only be displayed @@ -1143,13 +1079,18 @@ class ReviewRequestActionHook(BaseReviewRequestActionHook): viewer pages. """ - def initialize(self, actions=None, apply_to=None): + registry = review_request_actions + default_apply_to = [main_review_request_url_name] + base_action_class = ReviewRequestAction + base_menu_action_class = ReviewRequestMenuAction + + def initialize(self, actions=None, apply_to=None, *args, **kwargs): """Initialize this action hook. Args: actions (list, optional): The list of actions (of type :py:class:`dict` or - :py:class:`~.actions.BaseReviewRequestAction`) to be added. + :py:class:`~.actions.BaseAction`) to be added. apply_to (list of unicode, optional): The list of URL names that this action hook will apply to. @@ -1163,16 +1104,19 @@ class ReviewRequestActionHook(BaseReviewRequestActionHook): ValueError: Some review request action is neither a - :py:class:`~.actions.BaseReviewRequestAction` nor a + :py:class:`~.actions.BaseAction` nor a :py:class:`dict` instance. """ super(ReviewRequestActionHook, self).initialize( - actions=actions, - apply_to=apply_to or [main_review_request_url_name]) + actions=actions or [], + apply_to=apply_to or self.default_apply_to, + *args, + **kwargs) @six.add_metaclass(ExtensionHookPoint) -class ReviewRequestDropdownActionHook(ReviewRequestActionHook): +class ReviewRequestDropdownActionHook(DropdownActionHookMixin, + ReviewRequestActionHook): """A hook for adding dropdown menu actions to review request pages. Each menu action should be an instance of @@ -1209,45 +1153,9 @@ class ReviewRequestDropdownActionHook(ReviewRequestActionHook): }] """ - 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(BaseReviewRequestActionHook): +class DiffViewerActionHook(ReviewRequestActionHook): """A hook for adding review request actions to diff viewer pages. By default, actions that are passed into this hook will only be displayed @@ -1255,39 +1163,26 @@ class DiffViewerActionHook(BaseReviewRequestActionHook): pages. """ - def initialize(self, actions=None, apply_to=diffviewer_url_names): - """Initialize this action hook. - - Args: - actions (list, optional): - The list of actions (of type :py:class:`dict` or - :py:class:`~.actions.BaseReviewRequestAction`) to be added. + default_apply_to = diffviewer_url_names - apply_to (list of unicode, optional): - The list of URL names that this action hook will apply to. - 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).initialize( - actions, - apply_to=apply_to or diffviewer_url_names) +@six.add_metaclass(ExtensionHookPoint) +class DiffViewerDropdownActionHook(DropdownActionHookMixin, + DiffViewerActionHook): + """A hook for adding dropdown menu actions to diff viewer pages.""" @six.add_metaclass(ExtensionHookPoint) class HeaderActionHook(ActionHook): """A hook for adding actions to the page header.""" + registry = header_actions + base_action_class = HeaderAction + base_menu_action_class = HeaderMenuAction + @six.add_metaclass(ExtensionHookPoint) -class HeaderDropdownActionHook(ActionHook): +class HeaderDropdownActionHook(DropdownActionHookMixin, HeaderActionHook): """A hook for adding dropdown menu actions to the page header.""" @@ -1812,21 +1707,21 @@ class APIExtraDataAccessHook(ExtensionHook): Example: .. code-block:: python - obj.extra_data = { - 'foo': { - 'bar' : 'private_data', - 'baz' : 'public_data' - } - } - - ... - - APIExtraDataAccessHook( - extension, - resource, - [ - (('foo', 'bar'), ExtraDataAccessLevel.ACCESS_STATE_PRIVATE, - ]) + obj.extra_data = { + 'foo': { + 'bar' : 'private_data', + 'baz' : 'public_data' + } + } + + ... + + APIExtraDataAccessHook( + extension, + resource, + [ + (('foo', 'bar'), ExtraDataAccessLevel.ACCESS_STATE_PRIVATE, + ]) """ def initialize(self, resource, field_set): @@ -1888,7 +1783,6 @@ __all__ = [ 'APIExtraDataAccessHook', 'AuthBackendHook', 'AvatarServiceHook', - 'BaseReviewRequestActionHook', 'CommentDetailDisplayHook', 'ConsentRequirementHook', 'DashboardColumnsHook', @@ -1896,6 +1790,8 @@ __all__ = [ 'DataGridColumnsHook', 'DataGridSidebarItemsHook', 'DiffViewerActionHook', + 'DiffViewerDropdownActionHook', + 'DropdownActionHookMixin', 'EmailHook', 'ExtensionHook', 'FileAttachmentThumbnailHook', diff --git a/reviewboard/extensions/templatetags/rb_extensions.py b/reviewboard/extensions/templatetags/rb_extensions.py index e8936b68f5099a01f5c1f0d9db1b12b1aafa733e..0569ed5b8a314699b55ad8c42c6dcbfb53a56aa3 100644 --- a/reviewboard/extensions/templatetags/rb_extensions.py +++ b/reviewboard/extensions/templatetags/rb_extensions.py @@ -15,28 +15,14 @@ from reviewboard.site.urlresolvers import local_site_reverse register = template.Library() -def action_hooks(context, hook_cls, action_key="action", - template_name="extensions/action.html"): +def action_hooks(context, hook_cls): """Displays all registered action hooks from the specified ActionHook.""" html = [] for hook in hook_cls.hooks: try: - for actions in hook.get_actions(context): - if actions: - context.push() - context[action_key] = actions - - try: - html.append(render_to_string(template_name, context)) - except Exception as e: - logging.error( - 'Error when rendering template for action "%s" ' - 'for hook %r in extension "%s": %s', - action_key, hook, hook.extension.id, e, - exc_info=1) - - context.pop() + for action in hook.get_actions(context): + html.append(action.render(context)) except Exception as e: logging.error('Error when running get_actions() on hook %r ' 'in extension "%s": %s', @@ -84,10 +70,7 @@ def header_action_hooks(context): @register.simple_tag(takes_context=True) def header_dropdown_action_hooks(context): """Displays all multi-entry action hooks for the header bar.""" - return action_hooks(context, - HeaderDropdownActionHook, - "actions", - "extensions/header_action_dropdown.html") + return action_hooks(context, HeaderDropdownActionHook) @register.simple_tag(takes_context=True) diff --git a/reviewboard/extensions/tests.py b/reviewboard/extensions/tests.py index 5c6b8e4da07632baf11cea8123d6399be4f2ce84..315d128306f9e2e22b7d5843d9050dbff4b260d8 100644 --- a/reviewboard/extensions/tests.py +++ b/reviewboard/extensions/tests.py @@ -7,6 +7,7 @@ from django.core import mail from django.template import Context, Template from django.test.client import RequestFactory from django.utils import six +from django.utils.six.moves import zip from djblets.avatars.tests import DummyAvatarService from djblets.extensions.extension import ExtensionInfo from djblets.extensions.manager import ExtensionManager @@ -18,6 +19,7 @@ from djblets.siteconfig.models import SiteConfiguration from kgb import SpyAgency from mock import Mock +from reviewboard.actions.header import header_actions from reviewboard.admin.siteconfig import load_site_config from reviewboard.admin.widgets import (primary_widgets, secondary_widgets, @@ -27,7 +29,6 @@ from reviewboard.extensions.base import Extension from reviewboard.extensions.hooks import (AdminWidgetHook, APIExtraDataAccessHook, AvatarServiceHook, - BaseReviewRequestActionHook, CommentDetailDisplayHook, DiffViewerActionHook, EmailHook, @@ -47,10 +48,10 @@ from reviewboard.extensions.hooks import (AdminWidgetHook, WebAPICapabilitiesHook) from reviewboard.hostingsvcs.service import (get_hosting_service, HostingService) -from reviewboard.reviews.actions import (BaseReviewRequestAction, - BaseReviewRequestMenuAction, - clear_all_actions) from reviewboard.scmtools.errors import FileNotFoundError +from reviewboard.reviews.actions import (ReviewRequestAction, + ReviewRequestMenuAction, + review_request_actions) from reviewboard.reviews.features import ClassBasedActionsFeature from reviewboard.reviews.models.review_request import ReviewRequest from reviewboard.reviews.fields import (BaseReviewRequestField, @@ -76,16 +77,17 @@ class ExtensionManagerMixin(object): class DummyExtension(Extension): registration = RegisteredExtension() + id = 'reviewboard.extensions.tests.DummyExtension' class ActionHookTests(ExtensionManagerMixin, TestCase): """Tests the action hooks in reviewboard.extensions.hooks.""" - class _TestAction(BaseReviewRequestAction): + class TestAction(ReviewRequestAction): action_id = 'test-action' label = 'Test Action' - class _TestMenuAction(BaseReviewRequestMenuAction): + class TestMenuAction(ReviewRequestMenuAction): action_id = 'test-menu-instance-action' label = 'Menu Instance' @@ -98,7 +100,8 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): super(ActionHookTests, self).tearDown() self.extension.shutdown() - clear_all_actions() + review_request_actions.reset() + header_actions.reset() def test_review_request_action_hook(self): """Testing ReviewRequestActionHook renders on a review request page but @@ -106,11 +109,11 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): """ with override_feature_check(ClassBasedActionsFeature.feature_id, enabled=True): - self._test_base_review_request_action_hook( + self._test_review_request_action_hook( 'review-request-detail', ReviewRequestActionHook, True) - self._test_base_review_request_action_hook( + self._test_review_request_action_hook( 'file-attachment', ReviewRequestActionHook, False) - self._test_base_review_request_action_hook( + self._test_review_request_action_hook( 'view-diff', ReviewRequestActionHook, False) def test_diffviewer_action_hook(self): @@ -119,11 +122,11 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): """ with override_feature_check(ClassBasedActionsFeature.feature_id, enabled=True): - self._test_base_review_request_action_hook( + self._test_review_request_action_hook( 'review-request-detail', DiffViewerActionHook, False) - self._test_base_review_request_action_hook( + self._test_review_request_action_hook( 'file-attachment', DiffViewerActionHook, False) - self._test_base_review_request_action_hook( + self._test_review_request_action_hook( 'view-diff', DiffViewerActionHook, True) def test_review_request_dropdown_action_hook(self): @@ -145,65 +148,63 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): '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): + with self.assertRaises(KeyError): hook_cls(extension=self.extension, actions=[ missing_url_action, ]) def test_action_hook_init_raises_value_error(self): - """Testing that BaseReviewRequestActionHook __init__ raises a - ValueError""" + """Testing that ReviewRequestActionHook __init__ raises a + TypeError + """ 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, ] + error_msgs = [ + 'Unexpected action "%r"; expected dict or BaseAction', + 'Unexpected action "%r"; expected dict or BaseAction', + 'Unexpected action "%r"; expected dict or tuple', + ] + with override_feature_check(ClassBasedActionsFeature.feature_id, enabled=True): - for hook_cls in action_hook_classes: - with self.assertRaisesMessage(ValueError, error_message): + for hook_cls, error_msg in zip(action_hook_classes, error_msgs): + error_msg %= unsupported_type_action + with self.assertRaisesMessage(TypeError, error_msg): hook_cls(extension=self.extension, actions=[ unsupported_type_action, ]) def test_dropdown_action_hook_init_raises_key_error(self): """Testing that ReviewRequestDropdownActionHook __init__ raises a - KeyError""" + 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)) - with self.assertRaisesMessage(KeyError, error_message): + with self.assertRaises(KeyError): ReviewRequestDropdownActionHook(extension=self.extension, actions=[ missing_items_menu_action, ]) - - def _test_base_review_request_action_hook(self, url_name, hook_cls, - should_render): + def _test_review_request_action_hook(self, url_name, hook_cls, + should_render): """Test if the action hook renders or not at the given URL. Args: @@ -222,7 +223,7 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): 'label': 'Yes ID', 'url': 'with-id-url', }, - self._TestAction(), + self.TestAction(), { 'label': 'No ID', 'url': 'without-id-url', @@ -233,9 +234,10 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): 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-dict-action') + + self.assertEqual( + {a.action_id for a in entries}, + {'with-id-action', 'test-action', 'no-id-action'}) template = Template( '{% load reviewtags %}' @@ -245,16 +247,10 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): 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-dict-action"' in content) + self.assertEqual(should_render, 'id="no-id-action"' in content) finally: hook.disable_hook() - content = template.render(context) - self.assertNotIn('href="with-id-url"', content) - self.assertNotIn('>Test Action<', content) - self.assertNotIn('id="no-id-dict-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. @@ -270,9 +266,9 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): The expected rendering behaviour. """ hook = hook_cls(extension=self.extension, actions=[ - self._TestMenuAction([ - self._TestAction(), - ]), + (self.TestMenuAction(), ( + self.TestAction(), + )), { 'id': 'test-menu-dict-action', 'label': 'Menu Dict', @@ -292,10 +288,17 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): try: 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') + self.assertNotIn('action', context) + + self.assertEqual( + {e.action_id for e in hook.get_actions(context)}, + { + 'test-menu-dict-action', + 'with-id-action', + 'no-id-action', + self.TestMenuAction.action_id, + self.TestAction.action_id, + }) dropdown_icon_html = \ '' @@ -305,7 +308,6 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): '{% review_request_actions %}' ) content = template.render(context) - self.assertNotIn('action', context) self.assertInHTML('Test Action', content) self.assertInHTML( @@ -316,7 +318,7 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): for s in (('id="test-menu-dict-action"', 'href="with-id-url"', - 'id="no-id-dict-action"')): + 'id="no-id-action"')): if should_render: self.assertIn(s, content) else: @@ -333,37 +335,6 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): finally: hook.disable_hook() - 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-dict-action"', content) - - def _test_action_hook(self, template_tag_name, hook_cls): - action = { - 'label': 'Test Action', - 'id': 'test-action', - 'image': 'test-image', - 'image_width': 42, - 'image_height': 42, - 'url': 'foo-url', - } - - hook = hook_cls(extension=self.extension, actions=[action]) - - context = Context({}) - entries = hook.get_actions(context) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0], action) - - t = Template( - "{% load rb_extensions %}" - "{% " + template_tag_name + " %}") - - self.assertEqual(t.render(context).strip(), - self._build_action_template(action)) - def _test_dropdown_action_hook(self, template_tag_name, hook_cls): action = { 'id': 'test-menu', @@ -384,13 +355,17 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): actions=[action]) context = Context({}) - entries = hook.get_actions(context) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0], action) + self.assertEqual( + {action.action_id for action in hook.get_actions(context)}, + {'test-menu', 'test-action'}) t = Template( - "{% load rb_extensions %}" - "{% " + template_tag_name + " %}") + ('{%% load rb_extensions %%}' + '{%% %(template_tag_name)s %%}') + % { + 'template_tag_name': template_tag_name, + } + ) content = t.render(context).strip() @@ -411,7 +386,26 @@ class ActionHookTests(ExtensionManagerMixin, TestCase): def test_header_hooks(self): """Testing HeaderActionHook""" - self._test_action_hook('header_action_hooks', HeaderActionHook) + action = { + 'label': 'Test Action', + 'id': 'test-action', + 'image': 'test-image', + 'image_width': 42, + 'image_height': 42, + 'url': 'foo-url', + } + + HeaderActionHook(extension=self.extension, actions=[action]) + + context = Context({}) + + t = Template( + '{% load rb_extensions %}' + '{% header_action_hooks %}' + ) + + self.assertHTMLEqual(t.render(context).strip(), + self._build_action_template(action)) def test_header_dropdown_action_hook(self): """Testing HeaderDropdownActionHook""" diff --git a/reviewboard/reviews/actions.py b/reviewboard/reviews/actions.py index 66564a787875497935af3271e267ca1fc4651af2..527ca1e30bf43101962ac208293bda32d1e92600 100644 --- a/reviewboard/reviews/actions.py +++ b/reviewboard/reviews/actions.py @@ -1,134 +1,143 @@ +"""Review request actions.""" + from __future__ import unicode_literals -from collections import deque +from django.utils.translation import ugettext_lazy as _ -from django.template.loader import render_to_string +from reviewboard.actions.registry import ActionRegistry +from reviewboard.actions.base import BaseAction, BaseMenuAction +from reviewboard.reviews.features import general_comments_feature +from reviewboard.reviews.models import ReviewRequest +from reviewboard.site.urlresolvers import local_site_reverse -from reviewboard.reviews.errors import DepthLimitExceededError +class ReviewRequestActionRegistry(ActionRegistry): + def get_defaults(self): + """Return the default review request actions. -#: The maximum depth limit of any action instance. -MAX_DEPTH_LIMIT = 2 + Returns: + list of tuple or reviewboard.reviews.actions.ReviewRequestAction: + A list of all default review request actions. + """ + return [ + (CloseMenuAction(), ( + SubmitAction(), + DiscardAction(), + DeleteAction(), + )), + (UpdateMenuAction(), ( + UploadDiffAction(), + UploadFileAction(), + )), + DownloadDiffAction(), + EditReviewAction(), + AddGeneralCommentAction(), + ShipItAction(), + ] -#: 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() +review_request_actions = ReviewRequestActionRegistry() -#: Determines if the default action instances have been populated yet. -_populated = False +class ReviewRequestAction(BaseAction): + registry = review_request_actions -class BaseReviewRequestAction(object): - """A base class for an action that can be applied to a review request. + template_name = 'reviews/action.html' - 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 ReviewRequestMenuAction(BaseMenuAction): + registry = review_request_actions - class UsedOnceAction(BaseReviewRequestAction): - action_id = 'once' - label = 'This is used once.' + template_name = 'reviews/menu_action.html' - 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,' +class CloseMenuAction(ReviewRequestMenuAction): + """A menu action for closing the corresponding review request.""" - 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. - """ + action_id = 'close-review-request-action' + label = _('Close') - #: The ID of this action. Must be unique across all types of actions and - #: menu actions, at any depth. - action_id = None + def should_render(self, context): + review_request = context['review_request'] - #: The label that displays this action to the user. - label = None + return (review_request.status == ReviewRequest.PENDING_REVIEW and + (context['request'].user.pk == review_request.submitter_id or + (context['perms']['reviews']['can_change_status'] and + review_request.public))) - #: The URL to invoke if this action is clicked. - url = '#' - #: Determines if this action should be initially hidden to the user. - hidden = False +class SubmitAction(ReviewRequestAction): + """An action for submitting the review request.""" - def __init__(self): - """Initialize this action. + action_id = 'submit-review-request-action' + label = _('Submitted') - By default, actions are top-level and have no children. - """ - self._parent = None - self._max_depth = 0 + def should_render(self, context): + return context['review_request'].public - 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. +class DiscardAction(ReviewRequestAction): + """An action for discarding the review request.""" - 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), - } + action_id = 'discard-review-request-action' + label = _('Discarded') - def get_label(self, context): - """Return this action's label. - Args: - context (django.template.Context): - The collection of key-value pairs from the template. +class DeleteAction(ReviewRequestAction): + """An action for permanently deleting the review request.""" - Returns: - unicode: The label that displays this action to the user. - """ - return self.label + action_id = 'delete-review-request-action' + label = _('Delete Permanently') - def get_url(self, context): - """Return this action's URL. + def should_render(self, context): + return context['perms']['reviews']['delete_reviewrequest'] - 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 +class UpdateMenuAction(ReviewRequestMenuAction): + """A menu action for updating the corresponding review request.""" - def get_hidden(self, context): - """Return whether this action should be initially hidden to the user. + action_id = 'update-review-request-action' + label = _('Update') + + def should_render(self, context): + review_request = context['review_request'] + + return (review_request.status == ReviewRequest.PENDING_REVIEW and + (context['request'].user.pk == review_request.submitter_id or + context['perms']['reviews']['can_edit_reviewrequest'])) + + +class UploadDiffAction(ReviewRequestAction): + """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: - bool: Whether this action should be initially hidden to the user. + unicode: The label that displays this action to the user. """ - return self.hidden + 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. - The default implementation is to always render the action everywhere. + 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): @@ -138,327 +147,116 @@ class BaseReviewRequestAction(object): Returns: bool: Determines if this action should render. """ - return True + return context['review_request'].repository_id is not None - @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. +class UploadFileAction(ReviewRequestAction): + """An action for uploading a file for the review request.""" - 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. + action_id = 'upload-file-action' + label = _('Add File') - 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 +class DownloadDiffAction(ReviewRequestAction): + """An action for downloading a diff from the review request.""" - if self._parent: - self._parent.reset_max_depth() + action_id = 'download-diff-action' + label = _('Download Diff') - def render(self, context, action_key='action', - template_name='reviews/action.html'): - """Render this action instance and return the content as HTML. + def get_url(self, context): + """Return this action's URL. 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. + The collection of key-value pairs from the template. 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. + unicode: The URL to invoke if this action is clicked. """ - _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() - + from reviewboard.urls import diffviewer_url_names -class BaseReviewRequestMenuAction(BaseReviewRequestAction): - """A base class for an action with a dropdown menu. + match = context['request'].resolver_match - Note: - A menu action's child actions must always be pre-registered. - """ + # We want to use a relative URL in the diff viewer as we will not be + # re-rendering the page when switching between revisions. + if match.url_name in diffviewer_url_names: + return 'raw/' - def __init__(self, child_actions=None): - """Initialize this menu action. + return local_site_reverse('raw-diff', context['request'], kwargs={ + 'review_request_id': context['review_request'].display_id, + }) - 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. + 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: - dict: The corresponding dictionary. + bool: Whether this action should be initially hidden to the user. """ - dict_copy = { - 'child_actions': self.child_actions, - } - dict_copy.update(super(BaseReviewRequestMenuAction, self).copy_to_dict( - context)) + from reviewboard.urls import diffviewer_url_names - return dict_copy + match = context['request'].resolver_match - @property - def max_depth(self): - """Lazily compute the max depth of any action contained by this action. + if match.url_name in diffviewer_url_names: + return match.url_name == 'view-interdiff' - 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 + return super(DownloadDiffAction, self).get_hidden(context) - 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. + def should_render(self, context): + """Return whether or not this action should render. 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. + The collection of key-value pairs available in the template + just before this action is to be rendered. 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. + bool: Determines if this action should render. """ - 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 + from reviewboard.urls import diffviewer_url_names - for default_action in reversed(get_default_actions()): - default_action.register() + 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 -def get_top_level_actions(): - """Return a generator of all top-level registered action instances. + return review_request.repository_id is not None - Yields: - BaseReviewRequestAction: - All top-level registered review request action instances. - """ - _populate_defaults() - return (_all_actions[action_id] for action_id in _top_level_ids) +class EditReviewAction(ReviewRequestAction): + """An action for editing a review intended for the review request.""" + action_id = 'review-action' + label = _('Review') -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. + def should_render(self, context): + return context['request'].user.is_authenticated() - 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) +class AddGeneralCommentAction(ReviewRequestAction): + """An action for adding a new general comment to a review.""" - action.unregister() + action_id = 'general-comment-action' + label = _('Add General Comment') + def should_render(self, context): + request = context['request'] + return (request.user.is_authenticated() and + general_comments_feature.is_enabled(request=request)) -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. +class ShipItAction(ReviewRequestAction): + """An action for quickly approving the review request without comments.""" - Warning: - This will clear **all** actions, even if they were registered in - separate extensions. - """ - global _populated + action_id = 'ship-it-action' + label = _('Ship It!') - _all_actions.clear() - _top_level_ids.clear() - _populated = False + def should_render(self, context): + return context['request'].user.is_authenticated() diff --git a/reviewboard/reviews/default_actions.py b/reviewboard/reviews/default_actions.py deleted file mode 100644 index e9fd5c5ec245b7eed980d0bae4572969bd3b2d6f..0000000000000000000000000000000000000000 --- a/reviewboard/reviews/default_actions.py +++ /dev/null @@ -1,238 +0,0 @@ -from __future__ import unicode_literals - -from django.utils.translation import ugettext_lazy as _ - -from reviewboard.reviews.actions import (BaseReviewRequestAction, - BaseReviewRequestMenuAction) -from reviewboard.reviews.features import general_comments_feature -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'] - - return (review_request.status == ReviewRequest.PENDING_REVIEW and - (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'] - - return (review_request.status == ReviewRequest.PENDING_REVIEW and - (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_id 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. - """ - match = context['request'].resolver_match - - # We want to use a relative URL in the diff viewer as we will not be - # re-rendering the page when switching between revisions. - if match.url_name in diffviewer_url_names: - return 'raw/' - - 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 - - return review_request.repository_id 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 AddGeneralCommentAction(BaseReviewRequestAction): - """An action for adding a new general comment to a review.""" - - action_id = 'general-comment-action' - label = _('Add General Comment') - - def should_render(self, context): - request = context['request'] - return (request.user.is_authenticated() and - general_comments_feature.is_enabled(request=request)) - - -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(), - AddGeneralCommentAction(), - ShipItAction(), - ] diff --git a/reviewboard/reviews/templatetags/reviewtags.py b/reviewboard/reviews/templatetags/reviewtags.py index 213a9c8f54956ead50cf59439a09f35b810654c1..edd9a141db9f25d5695181a678f1aa143b2dfd7e 100644 --- a/reviewboard/reviews/templatetags/reviewtags.py +++ b/reviewboard/reviews/templatetags/reviewtags.py @@ -19,7 +19,7 @@ from djblets.util.templatetags.djblets_js import json_dumps_items from reviewboard.accounts.models import Profile, Trophy from reviewboard.accounts.trophies import UnknownTrophy from reviewboard.diffviewer.diffutils import get_displayed_diff_line_ranges -from reviewboard.reviews.actions import get_top_level_actions +from reviewboard.reviews.actions import review_request_actions from reviewboard.reviews.fields import (get_review_request_field, get_review_request_fieldset, get_review_request_fieldsets) @@ -306,8 +306,8 @@ def reviewer_list(review_request): for user in review_request.target_people.all()]) -@register.simple_tag(takes_context=True) -def review_request_actions(context): +@register.simple_tag(takes_context=True, name='review_request_actions') +def render_review_request_actions(context): """Render all registered review request actions. Args: @@ -319,7 +319,7 @@ def review_request_actions(context): """ content = [] - for top_level_action in get_top_level_actions(): + for top_level_action in review_request_actions.get_root_actions(): try: content.append(top_level_action.render(context)) except Exception: @@ -329,29 +329,6 @@ def review_request_actions(context): 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/test_actions.py b/reviewboard/reviews/tests/test_actions.py index af3cb38ee18928f13a44e360bc35cf22f6ccc7b0..3394d236f9fc5dae6f0e4653931373f5f651b6a6 100644 --- a/reviewboard/reviews/tests/test_actions.py +++ b/reviewboard/reviews/tests/test_actions.py @@ -1,304 +1,26 @@ +"""Tests for review request actions.""" + from __future__ import unicode_literals from django.contrib.auth.models import AnonymousUser, User -from django.template import Context from django.test.client import RequestFactory -from django.utils import six from djblets.testing.decorators import add_fixtures from mock import Mock -from reviewboard.reviews.actions import (BaseReviewRequestAction, - BaseReviewRequestMenuAction, - MAX_DEPTH_LIMIT, - clear_all_actions, - get_top_level_actions, - register_actions, - unregister_actions) -from reviewboard.reviews.default_actions import (AddGeneralCommentAction, - CloseMenuAction, - DeleteAction, - DownloadDiffAction, - EditReviewAction, - ShipItAction, - SubmitAction, - UpdateMenuAction, - UploadDiffAction) -from reviewboard.reviews.errors import DepthLimitExceededError +from reviewboard.reviews.actions import (AddGeneralCommentAction, + CloseMenuAction, + DeleteAction, + DownloadDiffAction, + EditReviewAction, + ShipItAction, + SubmitAction, + UpdateMenuAction, + UploadDiffAction) from reviewboard.reviews.models import ReviewRequest from reviewboard.testing import TestCase -class FooAction(BaseReviewRequestAction): - action_id = 'foo-action' - label = 'Foo Action' - - -class BarAction(BaseReviewRequestMenuAction): - def __init__(self, action_id, child_actions=None): - super(BarAction, self).__init__(child_actions) - - self.action_id = 'bar-' + 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 - - -class ActionsTestCase(TestCase): - """Test case for unit tests dealing with actions.""" - - def tearDown(self): - super(ActionsTestCase, self).tearDown() - - # This prevents registered/unregistered/modified actions from leaking - # between different unit tests. - clear_all_actions() - - def make_nested_actions(self, depth): - """Return a nested list of actions to register. - - This returns a list of actions, each entry nested within the prior - entry, of the given length. The resulting list is intended to be - registered. - - Args: - depth (int): - The nested depth for the actions. - - Returns: - list of reviewboard.reviews.actions.BaseReviewRequestAction: - The list of actions. - """ - actions = [None] * depth - actions[0] = BarAction('0') - - for i in range(1, depth): - actions[i] = BarAction(six.text_type(i), [actions[i - 1]]) - - return actions - - -class ActionRegistryTests(ActionsTestCase): - """Unit tests for the review request actions registry.""" - - def test_register_actions_with_invalid_parent_id(self): - """Testing register_actions with an invalid parent ID""" - foo_action = FooAction() - - message = ( - 'bad-id does not correspond to a registered review request action' - ) - - foo_action.register() - - with self.assertRaisesMessage(KeyError, message): - register_actions([foo_action], 'bad-id') - - def test_register_actions_with_already_registered_action(self): - """Testing register_actions with an already registered action""" - foo_action = FooAction() - - message = ( - '%s already corresponds to a registered review request action' - % foo_action.action_id - ) - - foo_action.register() - - with self.assertRaisesMessage(KeyError, message): - register_actions([foo_action]) - - def test_register_actions_with_max_depth(self): - """Testing register_actions with max_depth""" - actions = self.make_nested_actions(MAX_DEPTH_LIMIT) - extra_action = BarAction('extra') - foo_action = FooAction() - - for d, action in enumerate(actions): - self.assertEquals(action.max_depth, d) - - register_actions([extra_action], actions[0].action_id) - actions = [extra_action] + actions - - for d, action in enumerate(actions): - self.assertEquals(action.max_depth, d) - - register_actions([foo_action]) - self.assertEquals(foo_action.max_depth, 0) - - def test_register_actions_with_too_deep(self): - """Testing register_actions with exceeding max depth""" - actions = self.make_nested_actions(MAX_DEPTH_LIMIT + 1) - invalid_action = BarAction(str(len(actions)), [actions[-1]]) - - error_message = ( - '%s exceeds the maximum depth limit of %d' - % (invalid_action.action_id, MAX_DEPTH_LIMIT) - ) - - with self.assertRaisesMessage(DepthLimitExceededError, error_message): - register_actions([invalid_action]) - - def test_unregister_actions(self): - """Testing unregister_actions""" - - orig_action_ids = { - action.action_id - for action in get_top_level_actions() - } - self.assertIn('update-review-request-action', orig_action_ids) - self.assertIn('review-action', orig_action_ids) - - unregister_actions(['update-review-request-action', 'review-action']) - - new_action_ids = { - action.action_id - for action in get_top_level_actions() - } - self.assertEqual(len(orig_action_ids), len(new_action_ids) + 2) - self.assertNotIn('update-review-request-action', new_action_ids) - self.assertNotIn('review-action', new_action_ids) - - def test_unregister_actions_with_child_action(self): - """Testing unregister_actions with child action""" - menu_action = TopLevelMenuAction([ - FooAction() - ]) - - self.assertEqual(len(menu_action.child_actions), 1) - unregister_actions([FooAction.action_id]) - self.assertEqual(len(menu_action.child_actions), 0) - - def test_unregister_actions_with_unregistered_action(self): - """Testing unregister_actions with unregistered action""" - foo_action = FooAction() - error_message = ( - '%s does not correspond to a registered review request action' - % foo_action.action_id - ) - - with self.assertRaisesMessage(KeyError, error_message): - unregister_actions([foo_action.action_id]) - - def test_unregister_actions_with_max_depth(self): - """Testing unregister_actions with max_depth""" - actions = self.make_nested_actions(MAX_DEPTH_LIMIT + 1) - - unregister_actions([actions[0].action_id]) - extra_action = BarAction(str(len(actions)), [actions[-1]]) - extra_action.register() - self.assertEquals(extra_action.max_depth, MAX_DEPTH_LIMIT) - - -class BaseReviewRequestActionTests(ActionsTestCase): - """Unit tests for BaseReviewRequestAction.""" - - def test_register_then_unregister(self): - """Testing BaseReviewRequestAction.register then unregister for - actions - """ - foo_action = FooAction() - foo_action.register() - - self.assertIn(foo_action.action_id, ( - action.action_id - for action in get_top_level_actions() - )) - - foo_action.unregister() - - self.assertNotIn(foo_action.action_id, ( - action.action_id - for action in get_top_level_actions() - )) - - def test_register_with_already_registered(self): - """Testing BaseReviewRequestAction.register with already registered - action - """ - foo_action = FooAction() - error_message = ( - '%s already corresponds to a registered review request action' - % foo_action.action_id - ) - - foo_action.register() - - with self.assertRaisesMessage(KeyError, error_message): - foo_action.register() - - def test_register_with_too_deep(self): - """Testing BaseReviewRequestAction.register with exceeding max depth""" - actions = self.make_nested_actions(MAX_DEPTH_LIMIT + 1) - invalid_action = BarAction(str(len(actions)), [actions[-1]]) - error_message = ( - '%s exceeds the maximum depth limit of %d' - % (invalid_action.action_id, MAX_DEPTH_LIMIT) - ) - - with self.assertRaisesMessage(DepthLimitExceededError, error_message): - invalid_action.register() - - def test_unregister_with_unregistered_action(self): - """Testing BaseReviewRequestAction.unregister with unregistered - action - """ - foo_action = FooAction() - - message = ( - '%s does not correspond to a registered review request action' - % foo_action.action_id - ) - - with self.assertRaisesMessage(KeyError, message): - foo_action.unregister() - - def test_unregister_with_max_depth(self): - """Testing BaseReviewRequestAction.unregister with max_depth""" - actions = self.make_nested_actions(MAX_DEPTH_LIMIT + 1) - actions[0].unregister() - extra_action = BarAction(str(len(actions)), [actions[-1]]) - - extra_action.register() - self.assertEquals(extra_action.max_depth, MAX_DEPTH_LIMIT) - - def test_init_already_registered_in_menu(self): - """Testing BaseReviewRequestAction.__init__ for already registered - action when nested in a menu action - """ - foo_action = FooAction() - error_message = ('%s already corresponds to a registered review ' - 'request action') % foo_action.action_id - - foo_action.register() - - with self.assertRaisesMessage(KeyError, error_message): - TopLevelMenuAction([ - foo_action, - ]) - - def test_render_pops_context_even_after_error(self): - """Testing BaseReviewRequestAction.render pops the context after an - error - """ - context = Context({'comment': 'this is a comment'}) - old_dict_count = len(context.dicts) - poorly_coded_action = PoorlyCodedAction() - - with self.assertRaises(Exception): - poorly_coded_action.render(context) - - new_dict_count = len(context.dicts) - self.assertEquals(old_dict_count, new_dict_count) - - -class AddGeneralCommentActionTests(ActionsTestCase): +class AddGeneralCommentActionTests(TestCase): """Unit tests for AddGeneralCommentAction.""" fixtures = ['test_users'] @@ -327,7 +49,7 @@ class AddGeneralCommentActionTests(ActionsTestCase): self.assertFalse(self.action.should_render({'request': request})) -class CloseMenuActionTests(ActionsTestCase): +class CloseMenuActionTests(TestCase): """Unit tests for CloseMenuAction.""" fixtures = ['test_users'] @@ -470,7 +192,7 @@ class CloseMenuActionTests(ActionsTestCase): })) -class DeleteActionTests(ActionsTestCase): +class DeleteActionTests(TestCase): """Unit tests for DeleteAction.""" def setUp(self): @@ -501,7 +223,7 @@ class DeleteActionTests(ActionsTestCase): })) -class DownloadDiffActionTests(ActionsTestCase): +class DownloadDiffActionTests(TestCase): """Unit tests for DownloadDiffAction.""" fixtures = ['test_users'] @@ -690,7 +412,7 @@ class DownloadDiffActionTests(ActionsTestCase): })) -class EditReviewActionTests(ActionsTestCase): +class EditReviewActionTests(TestCase): """Unit tests for EditReviewAction.""" fixtures = ['test_users'] @@ -715,7 +437,7 @@ class EditReviewActionTests(ActionsTestCase): self.assertFalse(self.action.should_render({'request': request})) -class ShipItActionTests(ActionsTestCase): +class ShipItActionTests(TestCase): """Unit tests for ShipItAction.""" fixtures = ['test_users'] @@ -740,7 +462,7 @@ class ShipItActionTests(ActionsTestCase): self.assertFalse(self.action.should_render({'request': request})) -class SubmitActionTests(ActionsTestCase): +class SubmitActionTests(TestCase): """Unit tests for SubmitAction.""" fixtures = ['test_users'] @@ -764,7 +486,7 @@ class SubmitActionTests(ActionsTestCase): })) -class UpdateMenuActionTests(ActionsTestCase): +class UpdateMenuActionTests(TestCase): """Unit tests for UpdateMenuAction.""" fixtures = ['test_users'] @@ -869,7 +591,7 @@ class UpdateMenuActionTests(ActionsTestCase): })) -class UploadDiffActionTests(ActionsTestCase): +class UploadDiffActionTests(TestCase): """Unit tests for UploadDiffAction.""" fixtures = ['test_users'] diff --git a/reviewboard/settings.py b/reviewboard/settings.py index 6a10c8c45a93bca403cc666eebc586b382eda7d3..0f5826dab39396f239790cf7a9e70e7d76894f5a 100644 --- a/reviewboard/settings.py +++ b/reviewboard/settings.py @@ -156,6 +156,7 @@ RB_BUILTIN_APPS = [ 'pipeline', # Must be after djblets.pipeline 'reviewboard', 'reviewboard.accounts', + 'reviewboard.actions', 'reviewboard.admin', 'reviewboard.attachments', 'reviewboard.avatars', diff --git a/reviewboard/templates/actions/header_action_dropdown.html b/reviewboard/templates/actions/header_action_dropdown.html new file mode 100644 index 0000000000000000000000000000000000000000..4665b9e0c8b9ece64a29b00e62ed15be18316ece --- /dev/null +++ b/reviewboard/templates/actions/header_action_dropdown.html @@ -0,0 +1,9 @@ +{% load actions djblets_utils i18n staticfiles %} +
  • + {{menu.label}} + + +
      +{% child_actions menu %} +
    +
    • diff --git a/reviewboard/templates/extensions/action.html b/reviewboard/templates/actions/action.html similarity index 100% rename from reviewboard/templates/extensions/action.html rename to reviewboard/templates/actions/action.html diff --git a/reviewboard/templates/extensions/header_action_dropdown.html b/reviewboard/templates/extensions/header_action_dropdown.html deleted file mode 100644 index 64141527fc6df419d7ca93218baa81949fa429ef..0000000000000000000000000000000000000000 --- a/reviewboard/templates/extensions/header_action_dropdown.html +++ /dev/null @@ -1,10 +0,0 @@ -{% load djblets_utils i18n staticfiles %} -
  • - {{actions.label}} - -
      -{% for action in actions.items %} -{% include "extensions/action.html" %} -{% endfor %} -
    -
    • diff --git a/reviewboard/templates/reviews/action.html b/reviewboard/templates/reviews/action.html index 7aeeb65987ffd30bdd3a885d4972287429a7672a..eeae2d0f8c657483cebe234ddd77fbc62e33e2f7 100644 --- a/reviewboard/templates/reviews/action.html +++ b/reviewboard/templates/reviews/action.html @@ -1,3 +1,3 @@
  • - +
    • diff --git a/reviewboard/templates/reviews/menu_action.html b/reviewboard/templates/reviews/menu_action.html index 77c653703b3a7cd90fab84dfeaab3c036fd40965..9fa32680225ab6cf15db3912385ccdfeb12388d6 100644 --- a/reviewboard/templates/reviews/menu_action.html +++ b/reviewboard/templates/reviews/menu_action.html @@ -1,9 +1,9 @@ -{% load reviewtags %} +{% load actions %}
  • - {{menu_action.label}} + {{menu.label}}
      -{% child_actions %} +{% child_actions menu %}