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

Jump to bottom

Augment get_route_for_model() to support REST API routes. #2223

Merged
merged 3 commits into from
Sep 14, 2022
Merged

Augment get_route_for_model() to support REST API routes. #2223

merged 3 commits into from
Sep 14, 2022

Conversation

jathanism
Copy link
Contributor

@jathanism jathanism commented Aug 16, 2022 •
edited

Closes: #2243

What's Changed

  • This adds an api=True argument to nautobot.utilities.utils.get_route_for_model()
  • Replaced almost all bespoke implementations of manual route construction for plugins or API routes with calls to get_route_for_model()
  • Expanded the Development Best Practices guide with a section on "Getting URL Routes"

TODO

  • Explanation of Change(s)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@jathanism jathanism force-pushed the jathanism-get_route_for_model-api branch 3 times, most recently from 7215fb0 to 01419af Compare August 17, 2022 00:16
glennmatthews
glennmatthews reviewed Aug 17, 2022
View reviewed changes
Copy link
Contributor

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

I think there's also a case in nautobot.utilities.views.GetReturnURLMixin.get_return_url that could perhaps also use get_route_for_model?

nautobot/docs/development/best-practices.md Outdated Show resolved Hide resolved
nautobot/utilities/utils.py Show resolved Hide resolved
@jathanism jathanism changed the title Augment get_route_for_model() to support REST API routes. WIP: Augment get_route_for_model() to support REST API routes. Aug 17, 2022
@bryanculver bryanculver changed the title WIP: Augment get_route_for_model() to support REST API routes. [WIP] Augment get_route_for_model() to support REST API routes. Aug 18, 2022
@bryanculver bryanculver marked this pull request as draft August 18, 2022 16:52
This was referenced Aug 31, 2022
Merged
Closed
@glennmatthews
Copy link
Contributor

@jathanism What's needed to move this out of draft status?

bryanculver
bryanculver requested changes Sep 9, 2022
View reviewed changes
Copy link
Member

@bryanculver bryanculver left a comment

Choose a reason for hiding this comment

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

Let's get this buttoned up and passing tests to simplify some other PRs.

If we miss a few of the old-style calls that's fine but getting the method available will help continued drift from say #2348 & #2371

nautobot/docs/development/best-practices.md Outdated Show resolved Hide resolved
nautobot/docs/development/best-practices.md Outdated Show resolved Hide resolved
nautobot/docs/development/best-practices.md Outdated Show resolved Hide resolved
nautobot/docs/development/best-practices.md Outdated Show resolved Hide resolved
@bryanculver bryanculver changed the title [WIP] Augment get_route_for_model() to support REST API routes. Augment get_route_for_model() to support REST API routes. Sep 9, 2022
@bryanculver bryanculver added the type: housekeeping Changes to the application which do not directly impact the end user label Sep 9, 2022
@bryanculver bryanculver marked this pull request as ready for review September 9, 2022 16:01
@jathanism jathanism self-assigned this Sep 13, 2022
@jathanism
Copy link
Contributor Author

@jathanism What's needed to move this out of draft status?

Get the tests passing? 😩

@jathanism jathanism force-pushed the jathanism-get_route_for_model-api branch 2 times, most recently from 6fc4f8e to 435bc8e Compare September 13, 2022 17:03
@jathanism
Augment get_route_for_model() to support REST API routes.
5c0ad97 
- This adds an `api=True` argument to `nautobot.utilities.utils.get_route_for_model`
- Replaced almost all bespoke implementations of manual route construction for plugins or API routes with calls to `get_route_for_model()`
- Expanded the Development Best Practices guide with a section on "Getting URL Routes"
- Eliminated `view_namespace` attribute from `nautobot.utilities.testing.api.APITestCase` as its only legit use-case was for the Django built-in `Group` model. So this route is now just hard-coded on `nautobot.users.tests.test_api.GroupTest` now by overloading `_get_detail_url()` and `_get_list_url()`
@jathanism jathanism force-pushed the jathanism-get_route_for_model-api branch from 435bc8e to 5c0ad97 Compare September 13, 2022 17:13
bryanculver
bryanculver approved these changes Sep 14, 2022
View reviewed changes
Copy link
Member

@bryanculver bryanculver left a comment

Choose a reason for hiding this comment

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

:shipit:

@jathanism jathanism merged commit 8bfea65 into develop Sep 14, 2022
@jathanism jathanism deleted the jathanism-get_route_for_model-api branch September 14, 2022 19:15
@jathanism jathanism mentioned this pull request Oct 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryanculver bryanculver bryanculver approved these changes

@glennmatthews glennmatthews glennmatthews approved these changes

Assignees

@jathanism jathanism

Labels
type: housekeeping Changes to the application which do not directly impact the end user
Projects
No open projects
Nautobot Core ⚙️
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement best practice for using get_route_for_model() for fetching URL routes
3 participants
@jathanism @glennmatthews @bryanculver