Project

General

Profile

Actions

Feature #13256

open

Better handling of duplicate IPs in static DHCP assignments

Added by → luckman212 over 1 year ago. Updated 20 days ago.

Status:
Feedback
Priority:
Normal
Assignee:
Category:
DHCP (IPv4)
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
24.03
Release Notes:
Default

Description

summary:

In 2018 code that prevented duplicate IPs from being used as static DHCP mappings was removed. There are some cases where this may be desireable, but this PR updates the feature to bring the warning back and allow it to be overridden. This should satisfy all use cases and prevent accidental misconfiguration.

related: https://redmine.pfsense.org/issues/8220

PR: https://github.com/pfsense/pfsense/pull/4594


Files

Actions #1

Updated by Marcos M over 1 year ago

Thank you for the contributions!

In general, it's best to avoid first/second person perspective. A yellowish warning box would be preferred over the red error box as well.

However, there are many instances in the GUI where the user is able to save a valid configuration that can have unintended behavior. In these cases, details are left to documentation (either online or in field descriptions). I don't believe this situation warrants a checkbox confirmation in this way.

Actions #2

Updated by Jim Pingle over 1 year ago

I don't think we should change the default behavior/add extra steps to reach the current behavior.

Something that might be more palatable is a non-fatal warning handled in JavaScript. Build a list of existing IP addresses in mappings for the interface and if the users enters one that matches, display a JS box under the field that states it's already in use.

That way the user could be warned but not required to jump through hoops to override it.

Actions #3

Updated by → luckman212 over 1 year ago

I wanted to make the warning display in a "Yellow Box" too but I looked through the code and couldn't see an easy way to re-use what we have to make that happen. If it's important I will try again.

As for rewriting the whole thing as a JS/AJAX function, that's not my strongest area so Jim Pingle if you feel strongly that it needs to be done, it might take me more time or I'd leave it up to someone else. Either way thanks for the consideration.

Actions #4

Updated by → luckman212 over 1 year ago

I pushed a revised version, looks like this now

Actions #5

Updated by Marcos M over 1 year ago

Looks good to me.

Actions #6

Updated by Marcos M 20 days ago

  • Status changed from New to Feedback
  • Assignee set to Marcos M
  • Target version set to 2.8.0
  • Plus Target Version set to 24.03
Actions

Also available in: Atom PDF