Merge lp:~hatch/juju-gui/remove-old-notifications into lp:juju-gui/experimental

[Jeff Pihach]

Remove remaining old unused notifications code.

https://codereview.appspot.com/23240043/

[Jeff Pihach (hatch)]

Reviewers: mp+194422_code.launchpad.net,

Message:
Please take a look.

Description:
Remove remaining old unused notifications code.

https://code.launchpad.net/~hatch/juju-gui/remove-old-notifications/+merge/194422

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/23240043/

Affected files (+3, -68 lines):
   M CHANGES.yaml
   A [revision details]
   M app/app.js
   D app/templates/notifications_overview.handlebars
   M app/views/notifications.js

Index: CHANGES.yaml
=== modified file 'CHANGES.yaml'
--- CHANGES.yaml 2013-11-07 19:57:08 +0000
+++ CHANGES.yaml 2013-11-07 21:23:39 +0000
@@ -20,7 +20,7 @@

  # Release identifiers in this file should always be "unreleased" or
  # [NUMBER].[NUMBER].[NUMBER] per http://semver.org/ .
-- unreleased
+- unreleased:
  - 0.12.0:
      - >
       (BETA) In arguably the biggest single new feature of the GUI since its

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/app.js
=== modified file 'app/app.js'
--- app/app.js 2013-11-07 15:49:57 +0000
+++ app/app.js 2013-11-07 21:10:06 +0000
@@ -125,10 +125,6 @@
        notifications: {
          type: 'juju.views.NotificationsView',
          preserve: true
- },
-
- notifications_overview: {
- type: 'juju.views.NotificationsOverview'
        }

      },
@@ -757,17 +753,6 @@
      // Route handlers

      /**
- * @method show_notifications_overview
- */
- show_notifications_overview: function(req) {
- this.showView('notifications_overview', {
- env: this.env,
- notifications: this.db.notifications,
- nsRouter: this.nsRouter
- });
- },
-
- /**
       * Show the login screen.
       *
       * @method showLogin
@@ -1309,10 +1294,6 @@
              callbacks: 'show_charm',
              model: 'browser-charm',
              namespace: 'gui'},
- // Notifications.
- { path: '/notifications/',
- callbacks: 'show_notifications_overview',
- namespace: 'gui'},
            // Authorization
            { path: '/login/', callbacks: 'showLogin' }
          ]

Index: app/templates/notifications_overview.handlebars
=== removed file 'app/templates/notifications_overview.handlebars'
--- app/templates/notifications_overview.handlebars 2012-09-28 20:12:35
+0000
+++ app/templates/notifications_overview.handlebars 1970-01-01 00:00:00
+0000
@@ -1,16 +0,0 @@
-<ul class="unstyled">
- {{#notifications}}
- <li id="{{clientId}}" class="notice">
- <div{{#if seen}} class="seen" {{/if}} class="{{level}}">
- <h4>{{title}}</h4>
- <div>
- {{message}}
- </div>
- {{#if link}}
- <a href="{{link}}"><i class="icon-arrow-right"></i>{{link_title}}</a>
- {{/if}}
- <small data-timestamp="{{timestamp}}"
class="timestamp">{{humanizeTime timestamp}}</small>
- </div>
- </li>
- {{/notifications}}
-</ul>

Ind...

[Jeff Pihach (hatch)]

No tests were removed as part of this branch because the tests for this
section were removed already. There are a series of notification tests
which are skipped right now which still fail and will need to be
re-written but do not impact this code removal.

https://codereview.appspot.com/23240043/

[Gary Poster (gary)]

LGTM. I wonder if we should delete the skipped tests. Do their
existence bring value somehow?

https://codereview.appspot.com/23240043/

[Jeff Pihach (hatch)]

Thanks for the review!

We left them in because we should have tests for the area which we are
skipping we just need to
assign time to fixing them/refactoring the notifications.

https://codereview.appspot.com/23240043/

[Jeff Pihach (hatch)]

*** Submitted:

Remove remaining old unused notifications code.

R=gary.poster
CC=
https://codereview.appspot.com/23240043

https://codereview.appspot.com/23240043/

=== modified file 'CHANGES.yaml'
--- CHANGES.yaml	2013-11-07 19:57:08 +0000
+++ CHANGES.yaml	2013-11-07 21:26:54 +0000
@@ -20,7 +20,7 @@
 
 # Release identifiers in this file should always be "unreleased" or
 # [NUMBER].[NUMBER].[NUMBER] per http://semver.org/ .
-- unreleased
+- unreleased:
 - 0.12.0:
     - >
      (BETA) In arguably the biggest single new feature of the GUI since its

=== modified file 'app/app.js'
--- app/app.js	2013-11-07 15:49:57 +0000
+++ app/app.js	2013-11-07 21:26:54 +0000
@@ -125,10 +125,6 @@
       notifications: {
         type: 'juju.views.NotificationsView',
         preserve: true
-      },
-
-      notifications_overview: {
-        type: 'juju.views.NotificationsOverview'
       }
 
     },
@@ -757,17 +753,6 @@
     // Route handlers
 
     /**
-     * @method show_notifications_overview
-     */
-    show_notifications_overview: function(req) {
-      this.showView('notifications_overview', {
-        env: this.env,
-        notifications: this.db.notifications,
-        nsRouter: this.nsRouter
-      });
-    },
-
-    /**
      * Show the login screen.
      *
      * @method showLogin
@@ -1309,10 +1294,6 @@
             callbacks: 'show_charm',
             model: 'browser-charm',
             namespace: 'gui'},
-          // Notifications.
-          { path: '/notifications/',
-            callbacks: 'show_notifications_overview',
-            namespace: 'gui'},
           // Authorization
           { path: '/login/', callbacks: 'showLogin' }
         ]

=== removed file 'app/templates/notifications_overview.handlebars'
--- app/templates/notifications_overview.handlebars	2012-09-28 20:12:35 +0000
+++ app/templates/notifications_overview.handlebars	1970-01-01 00:00:00 +0000
@@ -1,16 +0,0 @@
-<ul class="unstyled">
-    {{#notifications}}
-  <li id="{{clientId}}" class="notice">
-    <div{{#if seen}} class="seen" {{/if}} class="{{level}}">
-      <h4>{{title}}</h4>
-      <div>
-        {{message}}
-      </div>
-      {{#if link}}
-      <a href="{{link}}"><i class="icon-arrow-right"></i>{{link_title}}</a>
-      {{/if}}
-      <small data-timestamp="{{timestamp}}" class="timestamp">{{humanizeTime timestamp}}</small>
-    </div>
-  </li>
-  {{/notifications}}
-</ul>

=== modified file 'app/views/notifications.js'
--- app/views/notifications.js	2013-10-29 22:30:57 +0000
+++ app/views/notifications.js	2013-11-07 21:26:54 +0000
@@ -292,38 +292,6 @@
       });
   views.NotificationsView = NotificationsView;
 
-  /**
-   * The 'View All Notifications' view.
-   *
-   * @class NotificationsOverview
-   */
-  var NotificationsOverview = Y.Base.create('NotificationsOverview',
-      NotificationsBaseView, [], {
-        template: Templates.notifications_overview,
-        events: {
-          'li.notice': {
-            click: 'notificationSelect'
-          }
-        },
-        // Actions for selecting a notice
-        selection: {hide: false},
-
-        /**
-         * The overview shows all events by default when real filtering
-         * is present this will have to take options.
-         *
-         * @method getShowable
-         */
-        getShowable: function() {
-          var notifications = this.get('notifications');
-          return notifications.map(function(n) {
-            return n.getAttrs();
-          });
-        }
-      });
-
-  views.NotificationsOverview = NotificationsOverview;
-
 }, '0.1.0', {
   requires: [
     'view',