Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating methods for custom field choices #115

Merged
merged 7 commits into from
Jul 27, 2023
Merged

Updating methods for custom field choices #115

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
afedd94
fix: Update methods for custom fields (#114)
nautics889 Apr 15, 2023
8085e23
fix: Tests for methods for custom fields (#114)
nautics889 Apr 16, 2023
69ef638
fix: Fixtures for tests (custom fields) (#114)
nautics889 Apr 16, 2023
b09543a
fix: Failing test for getting custom fields (#114)
nautics889 Apr 16, 2023
e8b586c
refactor: Update docstrings and naming (#114)
nautics889 Jul 15, 2023
e5efc1a
fix: backward compatibility
nautics889 Jul 23, 2023
f401a23
refactor: update `custom_choices()` (#114)
nautics889 Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 59 additions & 8 deletions pynautobot/core/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,73 @@ def choices(self):

return self._choices

def custom_choices(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have .custom_choices() call self.custom_fields() so it is backwards compatible?

@jvanderaa @pszulczewski do we have any specific patterns of notifying the end user the method is deprecated?

Copy link
Contributor Author

@nautics889 nautics889 Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, this is exactly the question that i had gotten, but i thought it depends on versioning. I guess if it's ok to wait new minor feature release (x.y.z) for this update, it can be as is. Otherwise if it's better to keep as it was, i'm ready to upload a new commit, containing the original method implemetation and both of new, but renamed. Just let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have a specific pattern today @joewesch. I think we should add in something for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to have it straight for here.

If there is a breaking change, we will do a major release, following SemVer. There is an up coming major release for supporting Nautobot 2.0 if we want to get that in with this.

On top of it we should start to look at adding the policy in with 2.0 release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to add .custom_choices() to call self.custom_fields() as @joewesch suggested and we will keep it in 1.x release train and remove in 2.0.
We can also add logging and call logger.warning() that the method will be deprecated in 2.x.

Copy link
Contributor Author

@nautics889 nautics889 Jul 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to add .custom_choices() to call self.custom_fields() as @joewesch suggested and we will keep it in 1.x release train and remove in 2.0. We can also add logging and call logger.warning() that the method will be deprecated in 2.x.

Done

I've restored original method as it was, except adding deprecation warning log. Original tests are left removed for obvious reasons i presume (or should they be restored as well?)

def custom_fields(self):
"""Returns custom-fields response from app

:Returns: Raw response from Nautobot's custom-fields endpoint.
:Raises: :py:class:`.RequestError` if called for an invalid endpoint.
:Example:

>>> nb.extras.custom_choices()
{'Testfield1': {'Testvalue2': 2, 'Testvalue1': 1},
'Testfield2': {'Othervalue2': 4, 'Othervalue1': 3}}
>>> nb.extras.custom_fields()
[
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "Test custom field",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"content_types": ["dcim.rack"],
"type": {"value": "integer", "label": "Integer"},
"label": "Test custom field",
"name": "test_custom_field",
"slug": "test_custom_field",
"description": "",
"required": False,
"filter_logic": {"value": "loose", "label": "Loose"},
"default": None,
"weight": 100,
"validation_minimum": None,
"validation_maximum": None,
"validation_regex": "",
"created": "2023-04-15",
"last_updated": "2023-04-15T17:45:11.839431Z",
"notes_url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/notes/",
},
]
"""
custom_fields = Request(
base="{}/{}/custom-fields/".format(
self.api.base_url,
self.name,
),
base=f"{self.api.base_url}/{self.name}/custom-fields/",
token=self.api.token,
http_session=self.api.http_session,
).get()
return custom_fields

def custom_field_choices(self):
"""Returns custom-field-choices response from app

:Returns: Raw response from Nautobot's custom-field-choices endpoint.
:Raises: :py:class:`.RequestError` if called for an invalid endpoint.
:Example:

>>> nb.extras.custom_field_choices()
[
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "First option",
"url": "http://localhost:8000/api/extras/custom-field-choices/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"field": {
"display": "Test custom field 2",
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"name": "test_custom_field_2"
},
"value": "First option",
"weight": 100,
"created": "2023-04-15",
"last_updated": "2023-04-15T18:11:57.163237Z"
},
]
"""
custom_fields = Request(
base=f"{self.api.base_url}/{self.name}/custom-field-choices/",
token=self.api.token,
http_session=self.api.http_session,
).get()
Expand Down
47 changes: 47 additions & 0 deletions tests/fixtures/extras/custom_field_choices.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
[
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "First option",
"url": "http://localhost:8000/api/extras/custom-field-choices/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"field": {
"display": "Test custom field 2",
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"name": "test_custom_field_2"
},
"value": "First option",
"weight": 100,
"created": "2023-04-15",
"last_updated": "2023-04-15T18:11:57.163237Z"
},
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "Second option",
"url": "http://localhost:8000/api/extras/custom-field-choices/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"field": {
"display": "Test custom field 2",
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"name": "test_custom_field_2"
},
"value": "Second option",
"weight": 100,
"created": "2023-04-15",
"last_updated": "2023-04-15T18:11:57.169962Z"
},
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "Third option",
"url": "http://localhost:8000/api/extras/custom-field-choices/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"field": {
"display": "Test custom field 2",
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"name": "test_custom_field_2"
},
"value": "Third option",
"weight": 100,
"created": "2023-04-15",
"last_updated": "2023-04-15T18:11:57.174825Z"
}
]
60 changes: 60 additions & 0 deletions tests/fixtures/extras/custom_fields.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
[
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "Test custom field",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"content_types": [
"dcim.rack"
],
"type": {
"value": "integer",
"label": "Integer"
},
"label": "Test custom field",
"name": "test_custom_field",
"slug": "test_custom_field",
"description": "",
"required": false,
"filter_logic": {
"value": "loose",
"label": "Loose"
},
"default": null,
"weight": 100,
"validation_minimum": null,
"validation_maximum": null,
"validation_regex": "",
"created": "2023-04-15",
"last_updated": "2023-04-15T17:45:11.839431Z",
"notes_url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/notes/"
},
{
"id": "5b39ba88-e5ab-4be2-89f5-5a016473b53c",
"display": "Test custom field 2",
"url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/",
"content_types": [
"dcim.rack"
],
"type": {
"value": "select",
"label": "Selection"
},
"label": "Test custom field 2",
"name": "test_custom_field_2",
"slug": "test_custom_field_2",
"description": "",
"required": false,
"filter_logic": {
"value": "loose",
"label": "Loose"
},
"default": null,
"weight": 100,
"validation_minimum": null,
"validation_maximum": null,
"validation_regex": "",
"created": "2023-04-15",
"last_updated": "2023-04-15T18:11:57.133408Z",
"notes_url": "http://localhost:8000/api/extras/custom-fields/5b39ba88-e5ab-4be2-89f5-5a016473b53c/notes/"
}
]
55 changes: 47 additions & 8 deletions tests/test_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,62 @@

import pynautobot

from .util import Response

host = "http://localhost:8000"

def_kwargs = {
"token": "abc123",
}


class AppCustomChoicesTestCase(unittest.TestCase):
class AppCustomFieldsTestCase(unittest.TestCase):
@patch(
"pynautobot.core.query.Request.get",
return_value={"Testfield1": {"TF1_1": 1, "TF1_2": 2}, "Testfield2": {"TF2_1": 3, "TF2_2": 4}},
"requests.sessions.Session.get",
return_value=Response(fixture="extras/custom_fields.json"),
)
def test_custom_choices(self, *_):
def test_custom_fields(self, session_get_mock):
api = pynautobot.api(host, **def_kwargs)
choices = api.extras.custom_choices()
self.assertEqual(len(choices), 2)
self.assertEqual(sorted(choices.keys()), ["Testfield1", "Testfield2"])
cfs = api.extras.custom_fields()

session_get_mock.assert_called_once()
expect_url = f"{api.base_url}/extras/custom-fields/"
self.assertGreaterEqual(len(session_get_mock.call_args), 2)
url_passed_in_args = expect_url in session_get_mock.call_args[0]
url_passed_in_kwargs = expect_url == session_get_mock.call_args[1].get("url")
self.assertTrue(url_passed_in_args or url_passed_in_kwargs)

self.assertIsInstance(cfs, list)
self.assertEqual(len(cfs), 2)
for field in cfs:
self.assertIsInstance(field.get("name"), str)
self.assertIsInstance(field.get("content_types"), list)
self.assertIsInstance(field.get("slug"), str)
self.assertIn("type", field)


class AppCustomFieldChoicesTestCase(unittest.TestCase):
@patch(
"requests.sessions.Session.get",
return_value=Response(fixture="extras/custom_field_choices.json"),
)
def test_custom_field_choices(self, session_get_mock):
api = pynautobot.api(host, **def_kwargs)
choices = api.extras.custom_field_choices()

session_get_mock.assert_called_once()
expect_url = f"{api.base_url}/extras/custom-field-choices/"
self.assertGreaterEqual(len(session_get_mock.call_args), 2)
url_passed_in_args = expect_url in session_get_mock.call_args[0]
url_passed_in_kwargs = expect_url == session_get_mock.call_args[1].get("url")
self.assertTrue(url_passed_in_args or url_passed_in_kwargs)

self.assertIsInstance(choices, list)
self.assertEqual(len(choices), 3)
for choice in choices:
self.assertIsInstance(choice.get("field"), dict)
self.assertIsInstance(choice.get("value"), str)
self.assertIn(choice.get("value"), ("First option", "Second option", "Third option"))


class AppConfigTestCase(unittest.TestCase):
Expand All @@ -44,7 +83,7 @@ class PluginAppCustomChoicesTestCase(unittest.TestCase):
)
def test_custom_choices(self, *_):
api = pynautobot.api(host, **def_kwargs)
choices = api.plugins.test_plugin.custom_choices()
choices = api.plugins.test_plugin.custom_fields()
self.assertEqual(len(choices), 2)
self.assertEqual(sorted(choices.keys()), ["Testfield1", "Testfield2"])

Expand Down