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.

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

Add API Endpoints #143

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Add API Endpoints #143

wants to merge 3 commits into from

Conversation

jdrew82
Copy link
Contributor

@jdrew82 jdrew82 commented Jun 21, 2023

This PR is for addressing #38 to make the Sync and SyncLogEntry objects available via API endpoints.

@jdrew82 jdrew82 requested a review from a team as a code owner June 21, 2023 21:44
nautobot_ssot/api/views.py Outdated Show resolved Hide resolved

User = get_user_model()


class SyncAPITest(APIViewTestCases.APIViewTestCase): # pylint: disable=too-many-ancestors
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not support all the "Not implemented" ones? Shouldn't for example deletion be supported via the API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think some of the tests are certainly valid. I just wasn't sure how to go about actually testing them. I can work on building them out though. This was meant as an initial stab at adding the API endpoints to satisfy the referenced issue and to also get ready for the Nautobot 2.0 React UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that we should get the unit tests in along with the feature, otherwise its unclear when/if the unit tests will ever be enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I just need time to figure out the tests. I'm not familiar with how the API unit tests work so still working through that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here you should use separate subclasses of APIViewTestCases as this is a read-only API. That should cut down on the necessary @skip annotations.

pass


class SyncLogEntryAPITest(APIViewTestCases.APIViewTestCase): # pylint: disable=too-many-ancestors
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about the different base class here since this is also read-only

@jdrew82 jdrew82 added the type: enhancement New feature or request label Sep 20, 2023
@jdrew82 jdrew82 marked this pull request as draft September 28, 2023 13:30
@jdrew82
Copy link
Contributor Author

jdrew82 commented Sep 28, 2023

Putting this on hold until I have time to work on the tests after the 2.0 release.

@Kircheneer Kircheneer added the status: action required This issue requires additional information to be actionable label Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: action required This issue requires additional information to be actionable type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants