Skip to content

Conversation

alehaa
Copy link

@alehaa alehaa commented Aug 17, 2025

Pull Request

Related Issue

None.

New Behavior

None

Contrast to Current Behavior

Instead of using a custom RegexValidator, the name of an access list can be validated using Django's slug validator. This resolves the following issue involving an invalid escape sequence in the related test case:

/mnt/netbox/plugins/netbox-acls/netbox_acls/tests/models/test_accesslists.py:160: SyntaxWarning: invalid escape sequence '\|'
  non_alphanumeric_plus_chars = " !@#$%^&*()[]{};:,./<>?\|~=+"

Discussion: Benefits and Drawbacks

This PR reduces the overhead of a custom implementation and its testing.

Changes to the Documentation

None

Proposed Release Note Entry

Address a syntax warning in the AccessList test cases.

Double Check

  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR targets the dev branch.

Instead of using a custom RegexValidator, the name of an access list can
be validated using Django's slug validator. This reduces the overhead of
a custom implementation and its testing. Additionally, this resolves an
issue involving an invalid escape sequence in the related test case.
@pheus
Copy link
Contributor

pheus commented Aug 18, 2025

Hi @alehaa - thanks for the PR and for tackling the warning!

I’m all for dropping the custom regex. To avoid any UI changes and an extra migration, could we keep AccessList.name as a CharField and use Django’s validate_slug as the validator instead of switching the field type? That gives us the same validation without changing the field.

@alehaa
Copy link
Author

alehaa commented Aug 18, 2025

I considered that, but since changing the validation also required a migration, it should be the same. However, I agree that this would be cleaner and will make the change.

Although slug validation works for an access list name, the field should
remain a CharField to clearly indicate its purpose. Using slug
validation explicitly ensures that existing validation rules are used.
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.

2 participants