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

MVC, simplify list field types #3252

Closed
AdSchellevis opened this issue Feb 26, 2019 · 0 comments
Assignees
Labels
Projects
Milestone

Comments

@AdSchellevis
Copy link
Member

@AdSchellevis AdSchellevis commented Feb 26, 2019

The are multiple field types serving lists, it might be a good idea to add a new base class to abstract shared functionality.

The following field types are affected by this change:

  • AuthGroupField
  • AuthenticationServerField
  • CertificateField
  • ConfigdActionsField
  • CountryField
  • InterfaceField
  • JsonKeyValueStoreField
  • OptionField

EDIT: skipped ModelRelationField since it has some additional logic.

@AdSchellevis AdSchellevis self-assigned this Feb 26, 2019
AdSchellevis added a commit that referenced this issue Feb 26, 2019
We should refactor some code later, but there's no rush. issue in #3252
@fichtner fichtner added this to the 19.7 milestone Feb 27, 2019
fichtner added a commit that referenced this issue Mar 3, 2019
We should refactor some code later, but there's no rush. issue in #3252

(cherry picked from commit 560a6cb)
(cherry picked from commit 46de064)
@fichtner fichtner modified the milestones: 19.7, 20.1 Jul 1, 2019
AdSchellevis added a commit that referenced this issue Jul 2, 2019
…wed. eventually we need to restructure this a bit more (#3252), found while working on #3250
@fichtner fichtner added this to To Do in 20.1 Jul 18, 2019
EugenMayer added a commit to EugenMayer/core that referenced this issue Jul 22, 2019
…wed. eventually we need to restructure this a bit more (opnsense#3252), found while working on opnsense#3250
EugenMayer added a commit to EugenMayer/core that referenced this issue Jul 22, 2019
…wed. eventually we need to restructure this a bit more (opnsense#3252), found while working on opnsense#3250
AdSchellevis added a commit that referenced this issue Nov 8, 2019
AdSchellevis added a commit that referenced this issue Nov 8, 2019
AdSchellevis added a commit that referenced this issue Nov 8, 2019
AdSchellevis added a commit that referenced this issue Nov 8, 2019
InclusionIn() validation only works when using string, php seems to convert keys automatically, in which case the following input:

$data = ["101" => "abc", "102" => "cde"];

would lead to these keys:

array_keys($data) ==> [101, 102]

when validating if 101,100 is a valid item in the list InclusionIn() seems to think so....
AdSchellevis added a commit that referenced this issue Nov 8, 2019
20.1 automation moved this from To Do to Done Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
20.1
  
Done
2 participants
You can’t perform that action at this time.