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

feat(machines): Add new IP address field to Edit Physical form MAASENG-3453 #5511

Merged

Conversation

ndv99
Copy link
Contributor

@ndv99 ndv99 commented Jul 31, 2024

Done

  • Added PrefixedIpInput to the "Edit Physical" form
  • Added subnet-based IP validation to the form schema

QA steps

  • Go to a "Ready" machine
  • Go to the network tab for that machine
  • Click on the dropdown for an interface and click "Edit Physical"
  • Select "Static (Client configured)" IP assignment
  • Ensure the IP address field is shown, and the help text is correct for the subnet
  • Test the validation (non-numeric characters, out-of-range address for subnet)

Fixes

Resolves MAASENG-3453

Screenshots

Before

image

After

image

@webteam-app
Copy link

@ndv99 ndv99 marked this pull request as draft July 31, 2024 15:49
@ndv99 ndv99 changed the title feat(machines): Add new IP address field to Edit Physical form MAASENG-3453 feat(machines): Add new IP address field to Edit Physical form MAASENG-3453 WIP Jul 31, 2024
@ndv99 ndv99 marked this pull request as ready for review August 13, 2024 09:56
@ndv99 ndv99 removed the Don't merge label Aug 13, 2024
@ndv99 ndv99 changed the title feat(machines): Add new IP address field to Edit Physical form MAASENG-3453 WIP feat(machines): Add new IP address field to Edit Physical form MAASENG-3453 Aug 13, 2024
Copy link
Contributor

@amylily1011 amylily1011 left a comment

Choose a reason for hiding this comment

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

LGTM, well defined types and the logic looks quite straightforward.

import { useFormikContext } from "formik";
import { isIPv4 } from "is-ip";
import { useSelector } from "react-redux";
import * as Yup from "yup";
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the code imports Yup here for validation, but there is no validation schema provided below. (I might be missing something.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah - you'll need to expand line 40 in that file to see it, it is there though!

@ndv99 ndv99 merged commit 1375164 into canonical:main Aug 20, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants