Merge pull request #1360 from ATGE/issues/1346

Fix order item-list --prices location

Author: allmightyspiff

SoftLayer/CLI/order/item_list.py

@@ -13,15 +13,15 @@
 
 
 @click.command()
-@click.argument('location', required=False, nargs=-1, type=click.UNPROCESSED)
 @click.argument('package_keyname')
-@click.option('--keyword', help="A word (or string) used to filter item names.")
-@click.option('--category', help="Category code to filter items by")
+@click.option('--keyword', '-k', help="A word (or string) used to filter item names.")
+@click.option('--category', '-c', help="Category code to filter items by")
 @click.option('--prices', '-p', is_flag=True, help='Use --prices to list the server item prices, and to list the '
                                                    'Item Prices by location, add it to the --prices option using '
                                                    'location KeyName, e.g. --prices AMSTERDAM02')
+@click.argument('location', required=False)
 @environment.pass_env
-def cli(env, location, package_keyname, keyword, category, prices):
+def cli(env, package_keyname, keyword, category, prices, location=None):
     """List package items used for ordering.
 
     The item keyNames listed can be used with `slcli order place` to specify
@@ -57,14 +57,13 @@ def cli(env, location, package_keyname, keyword, category, prices):
     if prices:
         _item_list_prices(categories, sorted_items, tables)
         if location:
-            location = location[0]
             location_prices = manager.get_item_prices_by_location(location, package_keyname)
             _location_item_prices(location_prices, location, tables)
     else:
         table_items_detail = formatting.Table(COLUMNS)
-        for catname in sorted(categories):
-            for item in sorted_items[catname]:
-                table_items_detail.add_row([catname, item['keyName'], item['description'], get_price(item)])
+        for category_name in sorted(categories):
+            for item in sorted_items[category_name]:
+                table_items_detail.add_row([category_name, item['keyName'], item['description'], get_price(item)])
         tables.append(table_items_detail)
     env.fout(formatting.listing(tables, separator='\n'))
 
@@ -94,13 +93,13 @@ def get_price(item):
 def _item_list_prices(categories, sorted_items, tables):
     """Add the item prices cost and capacity restriction to the table"""
     table_prices = formatting.Table(COLUMNS_ITEM_PRICES)
-    for catname in sorted(categories):
-        for item in sorted_items[catname]:
+    for category in sorted(categories):
+        for item in sorted_items[category]:
             for price in item['prices']:
                 if not price.get('locationGroupId'):
-                    cr_max = _get_price_data(price, 'capacityRestrictionMaximum')
-                    cr_min = _get_price_data(price, 'capacityRestrictionMinimum')
-                    cr_type = _get_price_data(price, 'capacityRestrictionType')
+                    cr_max = get_item_price_data(price, 'capacityRestrictionMaximum')
+                    cr_min = get_item_price_data(price, 'capacityRestrictionMinimum')
+                    cr_type = get_item_price_data(price, 'capacityRestrictionType')
                     table_prices.add_row([item['keyName'], price['id'],
                                           get_item_price_data(price, 'hourlyRecurringFee'),
                                           get_item_price_data(price, 'recurringFee'),
@@ -117,33 +116,22 @@ def get_item_price_data(price, item_attribute):
 
 
 def _location_item_prices(location_prices, location, tables):
-    """Get a specific data from HS price.
+    """Add a location prices table to tables.
 
-    :param price: Hardware Server price.
-    :param string item: Hardware Server price data.
+    :param list location_prices : Location prices.
+    :param string location : Location.
+    :param list tables: Table list to add location prices table.
     """
     location_prices_table = formatting.Table(COLUMNS_ITEM_PRICES_LOCATION, title="Item Prices for %s" % location)
     location_prices_table.sortby = 'keyName'
     location_prices_table.align = 'l'
     for price in location_prices:
-        cr_max = _get_price_data(price, 'capacityRestrictionMaximum')
-        cr_min = _get_price_data(price, 'capacityRestrictionMinimum')
-        cr_type = _get_price_data(price, 'capacityRestrictionType')
+        cr_max = get_item_price_data(price, 'capacityRestrictionMaximum')
+        cr_min = get_item_price_data(price, 'capacityRestrictionMinimum')
+        cr_type = get_item_price_data(price, 'capacityRestrictionType')
         location_prices_table.add_row(
             [price['item']['keyName'], price['id'],
-             _get_price_data(price, 'hourlyRecurringFee'),
-             _get_price_data(price, 'recurringFee'),
+             get_item_price_data(price, 'hourlyRecurringFee'),
+             get_item_price_data(price, 'recurringFee'),
              "%s - %s %s" % (cr_min, cr_max, cr_type)])
     tables.append(location_prices_table)
-
-
-def _get_price_data(price, item):
-    """Get a specific data from HS price.
-
-    :param price: Hardware Server price.
-    :param string item: Hardware Server price data.
-    """
-    result = '-'
-    if item in price:
-        result = price[item]
-    return result

SoftLayer/managers/ordering.py

@@ -669,14 +669,39 @@ def get_location_id(self, location):
         return datacenter[0]['id']
 
     def get_item_prices_by_location(self, location, package_keyname):
-        """Returns the hardware server item prices by location.
+        """Returns the item prices by location.
 
         :param string package_keyname: The package for which to get the items.
-        :param string location: location to get the item prices.
+        :param string location: location name or keyname to get the item prices.
         """
-        object_mask = "filteredMask[pricingLocationGroup[locations[regions]]]"
+        object_mask = "filteredMask[pricingLocationGroup[locations]]"
+        location_name = self.resolve_location_name(location)
         object_filter = {
-            "itemPrices": {"pricingLocationGroup": {"locations": {"regions": {"keyname": {"operation": location}}}}}}
+            "itemPrices": {"pricingLocationGroup": {"locations": {"name": {"operation": location_name}}}}}
         package = self.get_package_by_key(package_keyname)
+
         return self.client.call('SoftLayer_Product_Package', 'getItemPrices', mask=object_mask, filter=object_filter,
                                 id=package['id'])
+
+    def resolve_location_name(self, location_key):
+        """Resolves a location name using a string location key.
+
+        :param string location_key: A string location used to resolve the location name.
+        :return: An location name.
+        """
+
+        default_region_keyname = 'unknown'
+        if not location_key or location_key == default_region_keyname:
+            raise exceptions.SoftLayerError("Invalid location {}".format(location_key))
+
+        default_regions = [{'keyname': default_region_keyname}]
+        index_first = 0
+        object_mask = "mask[regions]"
+        locations = self.client.call('SoftLayer_Location', 'getDatacenters', mask=object_mask)
+        for location in locations:
+            location_name = location.get('name')
+            if location_name == location_key:
+                return location_key
+            if location.get('regions', default_regions)[index_first].get('keyname') == location_key:
+                return location_name
+        raise exceptions.SoftLayerError("Location {} does not exist".format(location_key))

tests/CLI/modules/order_tests.py

@@ -46,7 +46,7 @@ def test_item_list(self):
         self.assertIn('item2', result.output)
 
     def test_item_list_prices(self):
-        result = self.run_command(['order', 'item-list', '--prices', 'package'])
+        result = self.run_command(['order', 'item-list', 'package', '--prices'])
 
         self.assert_no_fail(result)
         output = json.loads(result.output)
@@ -55,8 +55,28 @@ def test_item_list_prices(self):
         self.assertEqual(output[0][1]['keyName'], 'KeyName015')
         self.assert_called_with('SoftLayer_Product_Package', 'getItems')
 
-    def test_item_list_location(self):
-        result = self.run_command(['order', 'item-list', '--prices', 'AMSTERDAM02', 'package'])
+    def test_item_list_location_keyname(self):
+        result = self.run_command(['order', 'item-list', 'package', '--prices', 'DALLAS13', ])
+
+        self.assert_no_fail(result)
+        output = json.loads(result.output)
+        self.assertEqual(output[0][0]['Hourly'], 0.0)
+        self.assertEqual(output[0][1]['keyName'], 'KeyName015')
+        self.assertEqual(output[0][1]['priceId'], 1144)
+        self.assert_called_with('SoftLayer_Product_Package', 'getItemPrices')
+
+    def test_item_list_location_name(self):
+        result = self.run_command(['order', 'item-list', 'package', '--prices', 'dal13', ])
+
+        self.assert_no_fail(result)
+        output = json.loads(result.output)
+        self.assertEqual(output[0][0]['Hourly'], 0.0)
+        self.assertEqual(output[0][1]['keyName'], 'KeyName015')
+        self.assertEqual(output[0][1]['priceId'], 1144)
+        self.assert_called_with('SoftLayer_Product_Package', 'getItemPrices')
+
+    def test_item_list_category_keyword(self):
+        result = self.run_command(['order', 'item-list', 'package', '--prices', 'dal13', '-c', 'os', '-k' 'test'])
 
         self.assert_no_fail(result)
         output = json.loads(result.output)

tests/managers/ordering_tests.py

@@ -776,40 +776,6 @@ def test_get_item_capacity_intel(self):
 
         self.assertEqual(24, int(item_capacity))
 
-    def test_get_item_prices_by_location(self):
-        options = self.ordering.get_item_prices_by_location("MONTREAL", "MONTREAL")
-        item_prices = [
-            {
-                "hourlyRecurringFee": ".093",
-                "id": 204015,
-                "recurringFee": "62",
-                "item": {
-                    "description": "4 x 2.0 GHz or higher Cores",
-                    "id": 859,
-                    "keyName": "GUEST_CORES_4",
-                },
-                "pricingLocationGroup": {
-                    "id": 503,
-                    "locations": [
-                        {
-                            "id": 449610,
-                            "longName": "Montreal 1",
-                            "name": "mon01",
-                            "regions": [
-                                {
-                                    "description": "MON01 - Montreal",
-                                    "keyname": "MONTREAL",
-                                }
-                            ]
-                        }
-                    ]
-                }
-            }
-        ]
-
-        self.assertEqual(options[0]['item']['keyName'], item_prices[0]['item']['keyName'])
-        self.assertEqual(options[0]['hourlyRecurringFee'], item_prices[0]['hourlyRecurringFee'])
-
     def test_get_oder_detail_mask(self):
         order_id = 12345
         test_mask = 'mask[id]'
@@ -830,3 +796,48 @@ def test_get_oder_detail_default_mask(self):
             'items[description],userRecord[displayName,userStatus]]')
         self.ordering.get_order_detail(order_id)
         self.assert_called_with('SoftLayer_Billing_Order', 'getObject', identifier=order_id, mask=_default_mask)
+
+    def test_get_item_prices_by_location_name(self):
+        object_mask = "filteredMask[pricingLocationGroup[locations]]"
+        object_filter = {
+            "itemPrices": {"pricingLocationGroup": {"locations": {"name": {"operation": 'dal13'}}}}}
+        self.ordering.get_item_prices_by_location('dal13', 'TEST')
+
+        self.assert_called_with('SoftLayer_Product_Package', 'getItemPrices', mask=object_mask, filter=object_filter)
+
+    def test_get_item_prices_by_location_keyname(self):
+        object_mask = "filteredMask[pricingLocationGroup[locations]]"
+        object_filter = {
+            "itemPrices": {"pricingLocationGroup": {"locations": {"name": {"operation": 'dal13'}}}}}
+        self.ordering.get_item_prices_by_location('DALLAS13', 'TEST')
+
+        self.assert_called_with('SoftLayer_Product_Package', 'getItemPrices', mask=object_mask, filter=object_filter)
+
+    def test_resolve_location_name(self):
+        location_name_expected = 'dal13'
+        object_mask = "mask[regions]"
+        location_name = self.ordering.resolve_location_name('DALLAS13')
+        self.assertEqual(location_name, location_name_expected)
+        self.assert_called_with('SoftLayer_Location', 'getDatacenters', mask=object_mask)
+
+    def test_resolve_location_name_by_keyname(self):
+        location_name_expected = 'dal13'
+        object_mask = "mask[regions]"
+        location_name = self.ordering.resolve_location_name('DALLAS13')
+        self.assertEqual(location_name, location_name_expected)
+        self.assert_called_with('SoftLayer_Location', 'getDatacenters', mask=object_mask)
+
+    def test_resolve_location_name_by_name(self):
+        location_name_expected = 'dal13'
+        object_mask = "mask[regions]"
+        location_name = self.ordering.resolve_location_name('dal13')
+        self.assertEqual(location_name, location_name_expected)
+        self.assert_called_with('SoftLayer_Location', 'getDatacenters', mask=object_mask)
+
+    def test_resolve_location_name_invalid(self):
+        exc = self.assertRaises(exceptions.SoftLayerError, self.ordering.resolve_location_name, None)
+        self.assertIn("Invalid location", str(exc))
+
+    def test_resolve_location_name_not_exist(self):
+        exc = self.assertRaises(exceptions.SoftLayerError, self.ordering.resolve_location_name, "UNKNOWN_LOCATION_TEST")
+        self.assertIn("does not exist", str(exc))