Skip to content

Conversation

Jathn
Copy link
Contributor

@Jathn Jathn commented Aug 29, 2025

Fixes #19896: Allow decimal number boundaries for custom fields

The changes made allows storing decimal values as boundary values for custom fields. This fixes the previous bug that caused values to be stored as whole numbers.

Changes made:

  • Changes customfields model to use models.DecimalField to store validation_maximum and validation_minimum
  • Changes customfields filterset to use forms.DecimalField for respective fields
  • Database migration 0130 in extras application to allow above changes

@jnovinger jnovinger requested review from a team and arthanson and removed request for a team August 29, 2025 15:23
@jeremystretch jeremystretch added this to the v4.4.1 milestone Aug 29, 2025
Copy link
Collaborator

@arthanson arthanson left a comment

Choose a reason for hiding this comment

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

The min/max is different then the decimal field definition also I think larger then integer could be 12 seems excessive I think 10 (non decimal) places works for max integer 2147483647, wondering if 16 max and 4 decimal would work better here?

           field = forms.DecimalField(
                required=required,
                initial=initial,
                max_digits=12,
                decimal_places=4,
                min_value=self.validation_minimum,
                max_value=self.validation_maximum
            )

@Jathn Jathn requested a review from arthanson September 4, 2025 12:02
@Jathn
Copy link
Contributor Author

Jathn commented Sep 4, 2025

I agree. I originally looked mainly at the storage and that it was a BigIntegerField, however to my understanding it seems only Integers could be entered anyway. Please see new changes:

  • max_digits set to 16 and decimal_fields set to 4 on both min/max and the field. 276dd6c
  • I also noted that the graphql still used IntegerLookup so changed it to FloatLookup in line with other DecimalField implementations. 017b173
  • Fixed history of migrations: 0130 -> 0133 28be6a7

Edit: added note about migrations

@arthanson
Copy link
Collaborator

Thanks @Jathn !

@arthanson arthanson merged commit 309e434 into netbox-community:main Sep 4, 2025
7 checks passed
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.

Min/max values for decimal custom fields must be an integer
3 participants