Skip to content

Conversation

blag
Copy link
Contributor

@blag blag commented May 27, 2025

As discussed in PR #54, make model and form field arguments slightly more explicit with what they are: allowlists or denylists.

I'll rebase this once #54 is merged.

@blag blag force-pushed the tweak-field-argument-names branch from e00ef63 to 74cb16a Compare July 22, 2025 00:47
@marksweb marksweb requested a review from Copilot July 25, 2025 22:46
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds clarity to model and form field arguments by prefixing certain parameters with "allowed_" to explicitly indicate they are allowlists rather than denylists.

  • Renames ambiguous parameters to be more explicit about their allowlist nature
  • Updates template tag parameter for consistency
  • Maintains backward compatibility while improving API clarity

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/django_nh3/templatetags/nh3_tags.py Renames tags parameter to allowed_tags in template filter
src/django_nh3/models.py Updates model field parameters with allowed_ prefix and reorganizes initialization
src/django_nh3/forms.py Updates form field parameters with allowed_ prefix and reorganizes initialization
src/django_nh3/utils.py Adds utility functions and normalizes options handling
tests/test_settings.py Adds comprehensive tests for the new option normalization functionality
Comments suppressed due to low confidence (1)

tests/test_settings.py:90

  • This test duplicates the logic from lines 79-80. The second test should use the string import path as intended in the test setup, not the function reference again.
        result = normalize_nh3_options({"attribute_filter": set_test_flag_true})

}


def normalize_nh3_options( # noqa: C901, PLR0912
Copy link
Preview

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

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

The function has disabled complexity warnings (C901, PLR0912) indicating it may be too complex. Consider breaking this function into smaller, more focused functions to improve maintainability.

Copilot uses AI. Check for mistakes.

@marksweb marksweb merged commit ae5e7db into marksweb:main Jul 25, 2025
7 checks passed
@blag blag deleted the tweak-field-argument-names branch July 25, 2025 23:07
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.

3 participants