fix the christopher code review

Author: caberos

SoftLayer/CLI/routes.py

@@ -323,6 +323,7 @@
 
     ('vlan', 'SoftLayer.CLI.vlan'),
     ('vlan:detail', 'SoftLayer.CLI.vlan.detail:cli'),
+    ('vlan:edit', 'SoftLayer.CLI.vlan.edit:cli'),
     ('vlan:list', 'SoftLayer.CLI.vlan.list:cli'),
 
     ('summary', 'SoftLayer.CLI.summary:cli'),

SoftLayer/CLI/subnet/detail.py

@@ -26,7 +26,7 @@ def cli(env, identifier, no_vs, no_hardware):
     subnet_id = helpers.resolve_id(mgr.resolve_subnet_ids, identifier,
                                    name='subnet')
 
-    mask = 'mask[ipAddresses[id, ipAddress], datacenter, virtualGuests,hardware]'
+    mask = 'mask[ipAddresses[id, ipAddress,note], datacenter, virtualGuests, hardware]'
 
     subnet = mgr.get_subnet(subnet_id, mask=mask)
 
@@ -50,9 +50,9 @@ def cli(env, identifier, no_vs, no_hardware):
 
     ip_address = subnet.get('ipAddresses')
 
-    ip_table = formatting.KeyValueTable(['ipAddress', 'value'])
+    ip_table = formatting.KeyValueTable(['id', 'ip', 'note'])
     for address in ip_address:
-        ip_table.add_row([address.get('id'), address.get('ipAddress')])
+        ip_table.add_row([address.get('id'), address.get('ipAddress'), address.get('note')])
 
     table.add_row(['ipAddresses', ip_table])
 

SoftLayer/CLI/subnet/edit_ip.py

@@ -9,19 +9,19 @@
 
 @click.command()
 @click.argument('identifier')
-@click.option('--ip-address', required=True,
-              help='Assume the ipAddress to set the note.')
 @click.option('--note', help="set ip address note of subnet")
 @environment.pass_env
-def cli(env, identifier, ip_address, note):
-    """Set the note of the ipAddress subnet"""
+def cli(env, identifier, note):
+    """Set the note of the ipAddress"""
 
     data = {
         'note': note
     }
     mgr = SoftLayer.NetworkManager(env.client)
-    ips = mgr.get_subnet(identifier, mask='id,ipAddresses[id,ipAddress]').get('ipAddresses')
-
-    for address in ips:
-        if ip_address == address.get('ipAddress'):
-            mgr.set_subnet_ipddress_note(address.get('id'), data)
+    ip_id = None
+    if str.isdigit(identifier):
+        ip_id = identifier
+    else:
+        ip_object = mgr.get_ip_by_address(identifier)
+        ip_id = ip_object.get('id')
+    mgr.set_subnet_ipddress_note(ip_id, data)

SoftLayer/CLI/vlan/edit.py

@@ -0,0 +1,37 @@
+"""Edit a vlan's details."""
+# :license: MIT, see LICENSE for more details.
+
+import click
+
+import SoftLayer
+from SoftLayer.CLI import environment
+from SoftLayer.CLI import helpers
+
+
+@click.command()
+@click.argument('identifier')
+@click.option('--name', '-n',
+              help="The optional name for this VLAN")
+@click.option('--note', '-e',
+              help="The note for this vlan.")
+@click.option('--tags', '-g',
+              multiple=True,
+              help='Tags to set e.g. "tag1,tag2", or empty string to remove all'
+              )
+@environment.pass_env
+def cli(env, identifier, name, note, tags):
+    """Edit a vlan's details."""
+
+    new_tags = None
+
+    if tags:
+        new_tags = ','.join(tags)
+
+    mgr = SoftLayer.NetworkManager(env.client)
+    vlan_id = helpers.resolve_id(mgr.resolve_vlan_ids, identifier, 'VLAN')
+    vlan = mgr.edit(vlan_id, name=name, note=note, tags=new_tags)
+
+    if vlan:
+        click.secho("Vlan edited successfully", fg='green')
+    else:
+        click.secho("Failed to edit the vlan", fg='red')

SoftLayer/fixtures/SoftLayer_Network_Vlan.py

@@ -7,3 +7,6 @@
     'vlanNumber': 4444,
     'firewallInterfaces': None
 }
+
+editObject = True
+setTags = True

SoftLayer/managers/network.py

@@ -7,12 +7,17 @@
 """
 import collections
 import json
+import logging
+
+from SoftLayer.decoration import retry
 
 from SoftLayer import exceptions
 from SoftLayer import utils
 
 from SoftLayer.managers import event_log
 
+LOGGER = logging.getLogger(__name__)
+
 # pylint: disable=too-many-public-methods
 
 DEFAULT_SUBNET_MASK = ','.join(['hardware',
@@ -688,7 +693,53 @@ def get_nas_credentials(self, identifier, **kwargs):
         result = self.network_storage.getObject(id=identifier, **kwargs)
         return result
 
+    def edit(self, instance_id, name=None, note=None, tags=None):
+        """Edit a vlan.
+
+        :param integer instance_id: the instance ID to edit.
+        :param string name: valid name.
+        :param string note: note about this particular vlan.
+        :param string tags: tags to set on the vlan as a comma separated list.
+                            Use the empty string to remove all tags.
+        :returns: bool -- True or an Exception
+        """
+
+        obj = {}
+
+        if tags is not None:
+            self.set_tags(tags, vlan_id=instance_id)
+
+        if name:
+            obj['name'] = name
+
+        if note:
+            obj['note'] = note
+
+        if not obj:
+            return True
+
+        return self.vlan.editObject(obj, id=instance_id)
+
+    @retry(logger=LOGGER)
+    def set_tags(self, tags, vlan_id):
+        """Sets tags on a vlan with a retry decorator
+
+        Just calls vlan.setTags, but if it fails from an APIError will retry.
+        """
+        self.vlan.setTags(tags, id=vlan_id)
+
+    def get_ip_by_address(self, ip_address):
+        """get the ip address object
+
+         :param string ip_address: the ip address to edit.
+         """
+        return self.client.call('SoftLayer_Network_Subnet_IpAddress', 'getByIpAddress', ip_address)
+
     def set_subnet_ipddress_note(self, identifier, note):
-        """Set the ip address note of the subnet"""
+        """Set the ip address note of the subnet
+
+         :param integer identifier: the ip address ID to edit.
+          :param json note: the note to edit.
+        """
         result = self.client.call('SoftLayer_Network_Subnet_IpAddress', 'editObject', note, id=identifier)
         return result

docs/cli/vlan.rst

@@ -7,6 +7,10 @@ VLANs
     :prog: vlan detail
     :show-nested:
 
+.. click:: SoftLayer.CLI.vlan.edit:cli
+    :prog: vlan edit
+    :show-nested:
+
 .. click:: SoftLayer.CLI.vlan.list:cli
     :prog: vlan list
     :show-nested:

tests/CLI/modules/subnet_tests.py

@@ -139,6 +139,11 @@ def test_create_subnet_static_ipv6(self, confirm_mock):
         self.assertEqual(output, json.loads(result.output))
 
     def test_editrou_Ip(self):
-        result = self.run_command(['subnet', 'edit-ip', '123456', '--ip-address=16.26.26.26', '--note=test'])
+        result = self.run_command(['subnet', 'edit-ip', '16.26.26.26', '--note=test'])
+        self.assert_no_fail(result)
+        self.assertTrue(result)
+
+    def test_editrou_Id(self):
+        result = self.run_command(['subnet', 'edit-ip', '123456', '--note=test'])
         self.assert_no_fail(result)
         self.assertTrue(result)

tests/CLI/modules/vlan_tests.py

@@ -4,6 +4,8 @@
 
     :license: MIT, see LICENSE for more details.
 """
+import mock
+
 from SoftLayer import testing
 
 
@@ -76,3 +78,19 @@ def test_detail_hardware_without_hostname(self):
         vlan_mock.return_value = getObject
         result = self.run_command(['vlan', 'detail', '1234'])
         self.assert_no_fail(result)
+
+    @mock.patch('SoftLayer.CLI.vlan.edit.click')
+    def test_vlan_edit(self, click):
+        result = self.run_command(['vlan', 'edit', '--name=nameTest', '--note=noteTest', '--tags=tag1,tag2', '100'])
+        click.secho.assert_called_with('Vlan edited successfully', fg='green')
+        self.assert_no_fail(result)
+        self.assert_called_with('SoftLayer_Network_Vlan', 'editObject', identifier=100)
+
+    @mock.patch('SoftLayer.CLI.vlan.edit.click')
+    def test_vlan_edit_failure(self, click):
+        mock = self.set_mock('SoftLayer_Network_Vlan', 'editObject')
+        mock.return_value = False
+        result = self.run_command(['vlan', 'edit', '--name=nameTest', '--note=noteTest', '--tags=tag1,tag2', '100'])
+        click.secho.assert_called_with('Failed to edit the vlan', fg='red')
+        self.assert_no_fail(result)
+        self.assert_called_with('SoftLayer_Network_Vlan', 'editObject', identifier=100)

tests/managers/network_tests.py

@@ -625,3 +625,12 @@ def test_get_cci_event_logs(self):
         _filter = {'objectName': {'operation': 'CCI'}}
         self.assert_called_with('SoftLayer_Event_Log', 'getAllObjects', filter=_filter)
         self.assertEqual(100, log['accountId'])
+
+    def test_vlan_edit(self):
+        vlan_id = 100
+        name = "test"
+        note = "test note"
+        tags = "tag1,tag2"
+
+        self.network.edit(vlan_id, name, note, tags)
+        self.assert_called_with('SoftLayer_Network_Vlan', 'editObject')