launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/clean-up-buglisting-classes into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/clean-up-buglisting-classes/+merge/124053

Summary
=======
This branch is a precursor to one that addresses bug 984871. This branch moves
creation of the buglisting classes to use Y.Base in order to reclaim LoC used
for the fix.

Implementation
==============
Y.Base.create is used for the BugListingModel and BugListingNavigator. No
functional changes are made.

`from_page` is not moved inside the `create` declaration, as doing so breaks
the tests and makes its factory use less obvious.

Tests
=====
bin/test -vvct buglisting --layer=YUI

QA
==
Ensure the buglisting still works as expected, including next/prev results.

LoC
===
This is LoC negative.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/javascript/buglisting.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/clean-up-buglisting-classes/+merge/124053
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/clean-up-buglisting-classes into lp:launchpad.

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2012-09-12 01:36:22 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2012-09-12 20:37:19 +0000
@@ -30,22 +30,8 @@
      *  - batch_key: A string representing the position and ordering of the
      *    current batch, as returned by listing_navigator.get_batch_key
      */
-    module.BugListingModel = function () {
-        module.BugListingModel.superclass.constructor.apply(this, arguments);
-    };
-
-
-    module.BugListingModel.NAME = 'buglisting-model';
-
-
-    module.BugListingModel.ATTRS = {
-        field_visibility_defaults: {
-            value: null
-        }
-    };
-
-
-    Y.extend(module.BugListingModel, Y.Base, {
+    module.BugListingModel = Y.Base.create('buglisting-model', Y.Base, [],
+    {
         /**
          * Initializer sets up the History object that stores most of the
          * model data.
@@ -100,9 +86,12 @@
             var url = '?' + Y.QueryString.stringify(query);
             this.get('history').addValue('batch_key', batch_key, {url: url});
         }
+    }, {
+        ATTRS: {
+            field_visibility_defaults: { value: null }
+        }    
     });
 
-
     /**
      * Constructor.
      * current_url is used to determine search params.
@@ -114,45 +103,38 @@
      * io_provider is something providing the Y.io interface, typically used
      * for testing.  Defaults to Y.io.
      */
-    module.BugListingNavigator = function(config) {
-        module.BugListingNavigator.superclass.constructor.apply(
-            this, arguments);
-    };
-
-    module.BugListingNavigator.ATTRS = {
-    };
-
-    Y.extend(
-        module.BugListingNavigator,
-        Y.lp.app.listing_navigator.ListingNavigator, {
-        _bindUI: function () {
-            Y.lp.app.inlinehelp.init_help();
-        },
-
-        /**
-         * Return the model to use for rendering the batch.  This will include
-         * updates to field visibility.
-         */
-        get_render_model: function(current_batch) {
-            return Y.merge(
-                current_batch.mustache_model,
-                this.get('model').get_field_visibility());
-        },
-
-        make_model: function(batch_key, cache) {
-            return new module.BugListingModel({
-                batch_key: batch_key,
-                total: cache.total,
-                field_visibility: cache.field_visibility,
-                field_visibility_defaults: cache.field_visibility_defaults
-            });
-        }
-    });
+    module.BugListingNavigator = Y.Base.create(
+        '', Y.lp.app.listing_navigator.ListingNavigator, [],
+        {
+            _bindUI: function () {
+                Y.lp.app.inlinehelp.init_help();
+            },
+
+            /**
+             * Return the model to use for rendering the batch.  This will include
+             * updates to field visibility.
+             */
+            get_render_model: function(current_batch) {
+                return Y.merge(
+                    current_batch.mustache_model,
+                    this.get('model').get_field_visibility());
+            },
+
+            make_model: function(batch_key, cache) {
+                return new module.BugListingModel({
+                    batch_key: batch_key,
+                    total: cache.total,
+                    field_visibility: cache.field_visibility,
+                    field_visibility_defaults: cache.field_visibility_defaults
+                });
+            },
+
+        },{});
 
     /**
      * Factory to return a BugListingNavigator for the given page.
      */
-    module.BugListingNavigator.from_page = function() {
+    module.BugListingNavigator.from_page = function () {
         var target = Y.one('#client-listing');
         if (Y.Lang.isNull(target)){
             return null;
@@ -180,8 +162,7 @@
         navigator.clickAction('.last', navigator.last_batch);
         navigator.update_navigation_links();
         return navigator;
-    };
-
+    }
 
     /**
      * Helper view object for managing the buglisting code on the actual table
@@ -189,8 +170,7 @@
      *
      * @class TableView
      * @extends Y.Base
-     * @namespace lp.bugs.buglisting
-     *
+     * @namespace lp.bugs.buglisting *
      */
     module.TableView = Y.Base.create('buglisting-tableview', Y.Base,
         [], {