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

[REVIEW] Next UI Hackathon #3651

Merged
merged 153 commits into from
May 9, 2023
Merged

[REVIEW] Next UI Hackathon #3651

merged 153 commits into from
May 9, 2023

Conversation

bryanculver
Copy link
Member

@bryanculver bryanculver commented Apr 25, 2023

Closes #3691

Hanlin Miao and others added 30 commits April 25, 2023 14:21
- Generic create view is now working againn and will correctly redirect on create
- `uiSchema` is now being enforced on Form compponents for create view
- API endpoints no longer include `serializer/formData` keys and is back to the default endpoint responses from v1
- `schema/uiSchema keys now emit on `OPTIONS` queries only
- This is currently a POC hack. The correct solution should replace the metadata class and not overload `finalize_response()` on the viewsets.
- A number of extra `FIXME` comments put into place circle back to for correctness.
- Only the Metadata class emits JSON schema now when an `OPTIONS` request is made
- Legacy `NautobotMetadata` renamed to `NautobotMetadataV1` for now
- All other code that was in `nautobot.core.api.views` has been ripped out and there is no longer a need to overload or modify anything with REST API views
- Made `eslint` happy
- Caveat: `ImageField` of type "image" is not happy from `@rjsf/core` and reports it as "Unsupported schema type"; so it is hard-coded as "string" type for the moment.
* Restructure nav menu

* Change nav menu API return, update context when switching object list views, only show menu for current context

* Prettier and lint

* Get sub-group menus working

* ESLint

---------

Co-authored-by: Bryan Culver <me@bryanculver.com>
Hanlin Miao and others added 9 commits May 3, 2023 15:02
…cts with depth 0 (#3679)

* Change API to use HyperlinkedModelSerializer

* flake8 failure that crept in during merge

* pylint ftw

* Test fix and change fragments

* Update return_nested_serializer_data_based_on_depth to use URLs instead of PKs with depth 0

* Make an attempt at updating the REST API docs - definitely not comprehensive

* Address review feedback
# TODO: allow for recursive (more than two-level) nesting of groups?
for group in nav_tab.groups:
if not isinstance(group, NavMenuGroup):
raise TypeError(f"Expected a NavMenuGroup, but got {group}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise TypeError(f"Expected a NavMenuGroup, but got {group}")
raise TypeError(f"Expected a NavMenuGroup, but got {type(group)}")

f"{nav_tab.name} -> {group.name} -> {item.link}",
)
if not isinstance(nav_tab, NavMenuTab):
raise TypeError(f"Top level objects need to be an instance of NavMenuTab: {nav_tab}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise TypeError(f"Top level objects need to be an instance of NavMenuTab: {nav_tab}")
raise TypeError(f"Expected a NavMenuTab, but got {type(nav_tab)}")

group_perms = registry_groups[group.name]["permissions"]
for item in group.items:
if not isinstance(item, (NavMenuGroup, NavMenuItem)):
raise TypeError(f"Expected NavMenuItem or NavMenuGroup, but found {item}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise TypeError(f"Expected NavMenuItem or NavMenuGroup, but found {item}")
raise TypeError(f"Expected NavMenuItem or NavMenuGroup, but got {type(item)}")


master_device = Device.objects.get(pk=virtual_chassis_1["master"])
# The `master` key will be a URL now, but it contains the PK
master_device = Device.objects.get(pk=virtual_chassis_1["master"].split("/")[-2])
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: helper function to extract a pk from an api url or even return the instance of the object?

jathanism and others added 2 commits May 8, 2023 15:07
* Implemented list_display_fields for IPAM serializers.
* Renamed `Meta.list_display` to `list_display_fields` for improved readability.
* Fixed a bug in UI `ObjectCreate` where uiSchema was `undefined`.
* Enable OPTIONS in the browsable API.
* Updated `WritableSerializerMixin` to support URLs as pk
* Introduced new `is_url()` helper that uses Django's `URLValidator` under the hood.
* PK will be extracted from the URL before passed onto PK validation.
* Reverted a change to `readOnly` and `format` JSON schema hints that was breaking validation
* Fix a bug in getting the list display fields.
* bump for nautobot-ui
* add default headers to dcim and tenancy
* fix react warning
* fix initial column state
* add support for boolean table items

---------

Co-authored-by: Bryan Culver <bryan.culver@networktocode.com>
Co-authored-by: Gary Snider <gary.snider@networktocode.com>
@@ -62,6 +63,12 @@ def get_queryset_filter_params(self, data, queryset):
if isinstance(data, dict):
params = dict_to_filter_params(data)
return self.remove_non_filter_fields(params)

# Account for the fact that HyperlinkedIdentityFields might pass in URLs.
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: Let's account for this in the docstrings

# "NautobotHyperlinkedRelatedField": {"type": "string", "readOnly": True},
# type=string results in a free text field; also not what we want. For now,
# however, this will keep things moving so the unit tests pass.
"NautobotHyperlinkedRelatedField": {"type": "string", "format": "uuid"},
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: Should this be "format": "url"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it would be uri but we should probably hold off on making any changes that assume we're going to keep using rjsf

Copy link
Contributor

Choose a reason for hiding this comment

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

It can be a UUID on write.

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.

Let's merge and iterate!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI 2.0: Update Nav Menu for Context-Centered Navigation
6 participants