fix team code review comments

Author: caberos

SoftLayer/CLI/vlan/cancel.py

@@ -7,7 +7,6 @@
 from SoftLayer.CLI import environment
 from SoftLayer.CLI import exceptions
 from SoftLayer.CLI import formatting
-from SoftLayer.managers.billing import BillingManager
 
 
 @click.command()
@@ -17,14 +16,20 @@ def cli(env, identifier):
     """Cancel network vlan."""
 
     mgr = SoftLayer.NetworkManager(env.client)
-    billing = BillingManager(env.client)
+
     if not (env.skip_confirmations or formatting.no_going_back(identifier)):
         raise exceptions.CLIAbort('Aborted')
 
+    reasons = mgr.get_cancel_failure_reasons(identifier)
+    if len(reasons) > 0:
+        raise exceptions.CLIAbort(reasons)
     item = mgr.get_vlan(identifier).get('billingItem')
     if item:
-        billing.cancel_item(item.get('id'), 'cancel by cli command')
-        env.fout('Cancel Successfully')
+        mgr.cancel_item(item.get('id'),
+                        True,
+                        'Cancel by cli command',
+                        'Cancel by cli command')
     else:
-        res = mgr.get_cancel_failure_reasons(identifier)
-        raise exceptions.ArgumentError(res)
+        raise exceptions.CLIAbort(
+            "VLAN is an automatically assigned and free of charge VLAN,"
+            " it will automatically be removed from your account when it is empty")

SoftLayer/fixtures/SoftLayer_Network_Vlan.py

@@ -21,3 +21,8 @@
 getList = [getObject]
 
 cancel = True
+
+getCancelFailureReasons = [
+    "1 bare metal server(s) still on the VLAN ",
+    "1 virtual guest(s) still on the VLAN "
+]

SoftLayer/managers/billing.py

@@ -755,8 +755,22 @@ def set_subnet_ipddress_note(self, identifier, note):
         return result
 
     def get_cancel_failure_reasons(self, identifier):
-        """get the reasons by cannot cancel the VLAN
+        """get the reasons why we cannot cancel the VLAN.
 
         :param integer identifier:  the instance ID
         """
         return self.vlan.getCancelFailureReasons(id=identifier)
+
+    def cancel_item(self, identifier, cancel_immediately,
+                    reason_cancel, customer_note):
+        """Cancel a billing item immediately, deleting all its data.
+
+        :param integer identifier: the instance ID to cancel
+        :param string reason_cancel: reason cancel
+        """
+        return self.client.call('SoftLayer_Billing_Item', 'cancelItem',
+                                True,
+                                cancel_immediately,
+                                reason_cancel,
+                                customer_note,
+                                id=identifier)

SoftLayer/managers/network.py

@@ -103,9 +103,17 @@ def test_vlan_list(self):
     @mock.patch('SoftLayer.CLI.formatting.no_going_back')
     def test_vlan_cancel(self, confirm_mock):
         confirm_mock.return_value = True
+        mock = self.set_mock('SoftLayer_Network_Vlan', 'getCancelFailureReasons')
+        mock.return_value = []
         result = self.run_command(['vlan', 'cancel', '1234'])
         self.assert_no_fail(result)
 
+    @mock.patch('SoftLayer.CLI.formatting.no_going_back')
+    def test_vlan_cancel_error(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['vlan', 'cancel', '1234'])
+        self.assertTrue(result.exit_code, 2)
+
     @mock.patch('SoftLayer.CLI.formatting.no_going_back')
     def test_vlan_cancel_fail(self, confirm_mock):
         confirm_mock.return_value = False