-
Notifications
You must be signed in to change notification settings - Fork 263
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
App Provided Non Model View -View Without an associated API #3736
App Provided Non Model View -View Without an associated API #3736
Conversation
examples/example_plugin_with_view_override/example_plugin_with_view_override/ui/CustomView.jsx
Outdated
Show resolved
Hide resolved
…ithub.com/nautobot/nautobot into u/timizuoebideri1-3712-app-non-model-view
examples/example_plugin/example_plugin/ui/ModelRetrieveView.jsx
Outdated
Show resolved
Hide resolved
examples/example_plugin/example_plugin/ui/ModelRetrieveView.jsx
Outdated
Show resolved
Hide resolved
...les/example_plugin_with_view_override/example_plugin_with_view_override/ui/FullWidthPage.jsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
…ithub.com/nautobot/nautobot into u/timizuoebideri1-3712-app-non-model-view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is almost perfect but I think we should at least seed an empty app_routes
for the tests instead of a try-catch.
Co-authored-by: Bryan Culver <31187+bryanculver@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as soon as tests pass
.github/workflows/ci_pullrequest.yml
Outdated
- name: "Setup base app_imports for mock" | ||
working-directory: ./nautobot/ui | ||
run: "echo 'export const NautobotApps = {}; export default NautobotApps;' > src/app_imports.js" | ||
- name: "Setup base app_routes.json for test" | ||
working-directory: ./nautobot/ui | ||
run: "echo '{}' > src/app_routes.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these steps that a developer needs to do before running invoke unittest-ui
locally? If so let's make sure we have that well documented.
Are these steps that a user needs to do when installing Nautobot from pypi? Same concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glennmatthews What if instead we added said file in the repo bit .gitignore
and ignored all changes?
So:
echo "{}" > nautobot/ui/src/app_routes.json
git add -f nautobot/ui/src/app_routes.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably fine, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Womp womp...
Editing .gitignore will only ignore files so they will not be added to the git repository. However files that are tracked already will not be ignored.
Testing locally it still picked up future changes to the file. I'll update the PR to create the file if it doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion about how we do this, just want to make sure that if it's something a developer needs to do, they are informed about it. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They shouldn't have to do that anymore. Exception handling FTW!
let app_routes = {}; | ||
try { | ||
app_routes = require("../app_routes.json"); | ||
} catch {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because JavaScript I can't do:
try {
const app_routes = require("../app_routes.json");
} catch {
const app_routes = {};
}
Closes: #3712
What's Changed
example_plugin
' plugin.build_ui.py
' file to retrieve all Apps routes (urlspatterns) and create a react router representation of these routes, which will then be registered innautobot-ui
.Limitation
It is currently not able to convert nested App urls.
TODO