Fix review comments.

ns1/__init__.py

@@ -363,10 +363,10 @@ def searchZone(
     def createZone(
         self,
         zone,
-        name=None,
         zoneFile=None,
         callback=None,
         errback=None,
+        name=None,
         **kwargs
     ):
         """
@@ -378,8 +378,8 @@ def createZone(
         to populate the zone with.
 
         :param str zone: zone FQDN, like 'example.com'
-        :param str zone: zone name override, name will be zone FQDN if omitted
         :param str zoneFile: absolute path of a zone file
+        :param str name: zone name override, name will be zone FQDN if omitted
         :keyword int retry: retry time
         :keyword int refresh: refresh ttl
         :keyword int expiry: expiry ttl

ns1/rest/zones.py

@@ -37,10 +37,10 @@ def _buildBody(self, zone, **kwargs):
         return body
 
     def import_file(
-        self, zone, zoneFile, name=None, callback=None, errback=None, **kwargs
+        self, zone, zoneFile, callback=None, errback=None, **kwargs
     ):
         files = [("zonefile", (zoneFile, open(zoneFile, "rb"), "text/plain"))]
-        params = self._buildImportParams(kwargs, name=name)
+        params = self._buildImportParams(kwargs)
         return self._make_request(
             "PUT",
             "import/zonefile/%s" % (zone),
@@ -51,23 +51,23 @@ def import_file(
         )
 
     # Extra import args are specified as query parameters not fields in a JSON object.
-    def _buildImportParams(self, fields, name=None):
+    def _buildImportParams(self, fields):
         params = {}
         # Arrays of values should be passed as multiple instances of the same
         # parameter but the zonefile API expects parameters containing comma
         # seperated values.
         if fields.get("networks") is not None:
-            networksStrs = map(str, fields["networks"])
-            networksParam = ",".join(networksStrs)
-            params["networks"] = networksParam
+            networks_strs = map(str, fields["networks"])
+            networks_param = ",".join(networks_strs)
+            params["networks"] = networks_param
         if fields.get("views") is not None:
-            viewsParam = ",".join(fields["views"])
-            params["views"] = viewsParam
-        if name is not None:
-            params["name"] = name
+            views_param = ",".join(fields["views"])
+            params["views"] = views_param
+        if fields.get("name") is not None:
+            params["name"] = fields.get("name")
         return params
 
-    def create(self, zone, name=None, callback=None, errback=None, **kwargs):
+    def create(self, zone, callback=None, errback=None, name=None, **kwargs):
         body = self._buildBody(zone, **kwargs)
         if name is None:
             name = zone

ns1/zones.py

@@ -91,7 +91,7 @@ def success(result, *args):
         )
 
     def create(
-        self, name=None, zoneFile=None, callback=None, errback=None, **kwargs
+        self, zoneFile=None, callback=None, errback=None, name=None, **kwargs
     ):
         """
         Create a new zone. Pass a list of keywords and their values to
@@ -120,17 +120,17 @@ def success(result, *args):
             return self._rest.import_file(
                 self.zone,
                 zoneFile,
-                name=name,
                 callback=success,
                 errback=errback,
+                name=name,
                 **kwargs
             )
         else:
             return self._rest.create(
                 self.zone,
-                name=name,
                 callback=success,
                 errback=errback,
+                name=name,
                 **kwargs
             )
 

tests/unit/test_zone.py

@@ -81,6 +81,37 @@ def test_rest_zone_create(zones_config, zone, name, url):
     )
 
 
+@pytest.mark.parametrize(
+    "zone, name, url, params",
+    [
+        ("test.zone", None, "zones/test.zone", {}),
+        ("test.zone", "test.name", "zones/test.name", {}),
+        ("test.zone", "test2.name", "zones/test2.name", {"networks": [1, 2]}),
+        (
+            "test.zone",
+            "test3.name",
+            "zones/test3.name",
+            {"networks": [1, 2], "views": "testview"},
+        ),
+        (
+            "test.zone",
+            "test4.name",
+            "zones/test4.name",
+            {"hostmaster": "example:example.com"},
+        ),
+    ],
+)
+def test_rest_zone_create_with_params(zones_config, zone, name, url, params):
+    z = ns1.rest.zones.Zones(zones_config)
+    z._make_request = mock.MagicMock()
+    z.create(zone, name=name, **params)
+    body = params
+    body["zone"] = zone
+    z._make_request.assert_called_once_with(
+        "PUT", url, body=body, callback=None, errback=None
+    )
+
+
 @pytest.mark.parametrize(
     "zone, name, url, networks, views",
     [
@@ -113,24 +144,51 @@ def test_rest_zone_import_file(zones_config, zone, name, url, networks, views):
     z = ns1.rest.zones.Zones(zones_config)
     z._make_request = mock.MagicMock()
     params = {}
-    networksStrs = None
+    networks_strs = None
     if networks is not None:
-        networksStrs = map(str, networks)
-        params["networks"] = ",".join(networksStrs)
+        networks_strs = map(str, networks)
+        params["networks"] = ",".join(networks_strs)
     if views is not None:
         params["views"] = ",".join(views)
-    if name is not None:
-        params["name"] = name
 
     zoneFilePath = "{}/../../examples/importzone.db".format(
         os.path.dirname(os.path.abspath(__file__))
     )
+
+    def cb():
+        # Should never be printed but provides a function body.
+        print("Callback invoked!")
+
+    # Test without zone name parameter
+    z.import_file(
+        zone,
+        zoneFilePath,
+        callback=cb,
+        errback=None,
+        networks=networks,
+        views=views,
+    )
+
+    z._make_request.assert_called_once_with(
+        "PUT", url, files=mock.ANY, callback=cb, errback=None, params=params
+    )
+
+    # Test with new zone name parameter (extra argument)
+    z._make_request.reset_mock()
+
+    if name is not None:
+        params["name"] = name
     z.import_file(
-        zone, zoneFilePath, name=name, networks=networks, views=views
+        zone,
+        zoneFilePath,
+        networks=networks,
+        views=views,
+        name=name,
+        callback=cb,
     )
 
     z._make_request.assert_called_once_with(
-        "PUT", url, files=mock.ANY, params=params, callback=None, errback=None
+        "PUT", url, files=mock.ANY, callback=cb, errback=None, params=params
     )