Merge lp:~jakedahn/horizon/lp760239 into lp:~hudson-openstack/horizon/trunk

[Jake Dahn]

This branch adds styling to the permission denied page.

[Devin Carlen (devcamcar)]

10 + 'Permission Denied %s' % e.message)

let's drop the %s and e.message. they are redundant here

review: Needs Fixing

[Devin Carlen (devcamcar)]

8 + except exceptions.NovaUnauthorizedError, e:
9 + messages.error(request, 'Permission Denied')
10 + return redirect('dashboard_permission_denied')

Hey, in this case, we don't need to redirect. Permission denied as an error panel is sufficient.

Essentially we want handle_exception (or wrap_exception or whatever I called it) to catch the NovaUnauthorizedError. This will be a method of last resort - as we identify other places that need to explicitly catch the NovaUnauthorizedException we will add try/excepts there as well.

review: Needs Fixing

[Jake Dahn (jakedahn)]

I think we still need to have the redirect.

If there is no redirect it just displays an h1 with the text 'permission denied' - no style or other html, just an h1. The redirect is to send it to /denied which displays the error with consistent style of the rest of the dashboard.

review: Needs Resubmitting

[Devin Carlen (devcamcar)]

If you are getting redirected to "permission denied" text, then you aren't try/except'ing the error where it's happening.

Each view is wrapped with a decorator @handle_nova_error, like so:

@login_required
@handle_nova_error
def launch(request, project_id, image_id):
    ...

In django_nova/exceptions.py's handle_nova_error method, the following is done if a NovaUnauthorizedException bubbles up and isn't handled by the view itself (launch() in the example above).

def handle_nova_error(func):
    """
    Decorator for handling nova errors in a generalized way.
    """
    def decorator(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except NovaUnavailableError:
            return redirect('nova_unavailable')
    return decorator

So are you saying its never trapping the error and somehow isn't even rendering the permission_denied template?

review: Needs Fixing

[lp:~jakedahn/horizon/lp760239]

35. By Jake Dahn on 2011-04-22

removing redirect from images view, as it is handled by the @handle_nova_error decorator

[Jake Dahn (jakedahn)]

Oops, jakefail - was running server from another branch while writing code in this branch - hence the mismatch in expected behavior.

I've removed the redirect and everything works as expected.

review: Needs Resubmitting

[Devin Carlen (devcamcar)]

cool, lgtm

review: Approve

=== modified file 'django-nova/src/django_nova/views/images.py'
--- django-nova/src/django_nova/views/images.py	2011-04-13 19:05:52 +0000
+++ django-nova/src/django_nova/views/images.py	2011-04-22 03:30:51 +0000
@@ -88,6 +88,8 @@
             except exceptions.NovaApiError, e:
                 messages.error(request,
                                'Unable to launch: %s' % e.message)
+            except exceptions.NovaUnauthorizedError, e:
+                messages.error(request, 'Permission Denied')
             else:
                 for instance in reservation.instances:
                     messages.success(request,

=== modified file 'openstack-dashboard/dashboard/templates/permission_denied.html'
--- openstack-dashboard/dashboard/templates/permission_denied.html	2011-01-12 21:43:31 +0000
+++ openstack-dashboard/dashboard/templates/permission_denied.html	2011-04-22 03:30:51 +0000
@@ -1,11 +1,26 @@
-{% extends "dashboard/base.html" %}
+{% extends "base.html" %}
 
 {% block title %} - Permission Denied{% endblock %}
-{% block pageclass %}denied{% endblock %}
 
 {% block content %}
-  <div id="page_head">
-    <h2 id="page_heading">Permission Denied</h2>
-    <p id="page_description">You do not have permission to view the requested page.</p>
+  <div id="right_content">
+    <div id="page_head">
+      <h2 id="page_heading">Permission Denied</h2>
+      <p id="page_description">You do not have permission to view the requested page.</p>
+    </div>
   </div>
 {% endblock %}
+
+{% block sidebar %}
+  <div id="sidebar">
+    <ul id="navigation">
+      {% block nav_home %}
+      <li><h3><a href="/">Home</a></h3></li>
+      {% endblock %}
+
+      {% block nav_projects %}
+      <li><h3><a href="/">Projects</a></h3></li>
+      {% endblock %}
+    </ul>
+  </div> <!-- end sidebar -->
+{% endblock %}