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

Custom validators unable to validate tag presence #12731

Closed
stuntguy3000 opened this issue May 26, 2023 · 6 comments · Fixed by #14516
Closed

Custom validators unable to validate tag presence #12731

stuntguy3000 opened this issue May 26, 2023 · 6 comments · Fixed by #14516
Assignees
Labels
severity: medium Results in substantial degraded or broken functionality for specfic workflows status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@stuntguy3000
Copy link
Contributor

stuntguy3000 commented May 26, 2023

NetBox version

v3.5.1

Python version

3.10

Steps to Reproduce

  1. Set the Custom Validators setting to validate tag requirements
CUSTOM_VALIDATORS = {
        "ipam.ipaddress": [{"tags": {"required": True}}],
        "dcim.site": [{"tags": {"required": True}}],
        "dcim.region": [{"tags": {"required": True}}]
}
  1. Create a new object defined in the list above with/without tags defined.

(This error is not exclusive to those object types)

Expected Behavior

I expected NetBox to validate if that particular object has tags set.

Observed Behavior

<class 'ValueError'>

IPAddress objects need to have a primary key value before you can access their tags.

Python version: 3.10.6
NetBox version: 3.5.1

I'm not sure if I'm asking too much from the Custom Validators here, or if another method to enforce the existence of Tags is available.

@stuntguy3000 stuntguy3000 added the type: bug A confirmed report of unexpected behavior in the application label May 26, 2023
@jeremystretch jeremystretch added the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label May 30, 2023
@stuntguy3000
Copy link
Contributor Author

stuntguy3000 commented May 31, 2023

I'm curious if this is actually fixable?

My understanding is that the Model is passed to the custom validator, but the tag information is lost as it sits as it is different from the other fields. Validators can't access the raw form data and the tags input is not passed along down this chain.

The only fix I can even see is to either hack together more custom validation specifically for tags, or to save the object BEFORE the tags are then validated - what could go wrong.

Have to leave this for someone more versed in Django, this isn't a simple fix :(

@jeremystretch jeremystretch added the severity: medium Results in substantial degraded or broken functionality for specfic workflows label Jun 23, 2023
@netopsab
Copy link
Contributor

netopsab commented Jun 24, 2023

Hi,

This is the only way I have found to catch tags (or other m2m fields) with CustomValidator :

from netbox.context import current_request
from extras.models import Tag

request = current_request.get()
tags = Tag.objects.filter(id__in=request.POST.getlist('tags')).values_list('name', flat=True)

Note : this is for the 'standard' edit form. For bulk edit (or import form), you have to dig with other fields (like add_tags, remove_tags) and check the view name before (with object request.resolver_match.view_name).

from extras.models import Tag

tags = set(instance.tags.names()).union(
    Tag.objects.filter(id__in=request.POST.getlist('add_tags')).values_list('name', flat=True)
).difference(
    Tag.objects.filter(id__in=request.POST.getlist('remove_tags')).values_list('name', flat=True)
)

Hope this can help you.

NB : for tracking, this issue occurs for all m2m fields (like tagged_vlans), not just tags.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 27, 2023
@arthanson arthanson self-assigned this Oct 2, 2023
@arthanson arthanson removed the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label Oct 2, 2023
Copy link
Contributor

github-actions bot commented Nov 2, 2023

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2023
@arthanson arthanson removed their assignment Nov 2, 2023
Urth added a commit to Urth/netbox that referenced this issue Nov 2, 2023
…d serializers

This allows for validation of any m2m relation like tags.
The validator receives the cleaned data from the form/serializer.
The model instance is not consistently available in the model
forms/serializers so is ommitted from both.

Note: the available fields can vary per form/serializer for a model.
You should always check if a field is defined in the cleaned data.
Urth added a commit to Urth/netbox that referenced this issue Nov 2, 2023
…d serializers

This allows for validation of any m2m relation like tags.
The validator receives the cleaned data from the form/serializer.
The model instance is not consistently available in the model
forms/serializers so is omitted from both.

Note: the available fields can vary per form/serializer for a model.
You should always check if a field is defined in the cleaned data.
@arthanson
Copy link
Collaborator

@Urth stated he wanted to work on this.

@arthanson arthanson reopened this Nov 3, 2023
@Urth
Copy link
Contributor

Urth commented Nov 3, 2023

yes, please assign it to me 👍

@arthanson arthanson removed the pending closure Requires immediate attention to avoid being closed for inactivity label Nov 3, 2023
@jeremystretch jeremystretch self-assigned this Dec 13, 2023
@jeremystretch jeremystretch added the status: accepted This issue has been accepted for implementation label Dec 13, 2023
jeremystretch added a commit that referenced this issue Dec 22, 2023
* WIP

* Enforce custom validators during bulk edit

* Add bulk edit M2M validation test

* Clean up tests

* Add custom validation test for bulk import

* Misc cleanup
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
severity: medium Results in substantial degraded or broken functionality for specfic workflows status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
5 participants