Skip to content

Refactor: #19: Rename "White-XY" to "Allow-XY" and "Black-XY" to "Block-XY"#60

Open
christoph-teichmeister wants to merge 37 commits intoambient-innovation:masterfrom
christoph-teichmeister:refactor/rename-whitelist-to-allowlist
Open

Refactor: #19: Rename "White-XY" to "Allow-XY" and "Black-XY" to "Block-XY"#60
christoph-teichmeister wants to merge 37 commits intoambient-innovation:masterfrom
christoph-teichmeister:refactor/rename-whitelist-to-allowlist

Conversation

@christoph-teichmeister
Copy link
Contributor

@christoph-teichmeister christoph-teichmeister commented May 17, 2025

Hey @GitRon,

I stumbled upon the TEST_STRUCTURE_VALIDATOR_FILE_WHITELIST setting in my private project and took the liberty to refactor any "White-" and "Black-" occurrences to their modern counterparts 🙂

I've introduced a Deprecation Warning for all "old" names.

I'd suggest, that we remove all old variables / methods / etc in 12.7 (or 13?).


Let me know what you think 🙂


relates to: #19

@GitRon GitRon changed the title Refactor: Rename "White-XY" to "Allow-XY" and "Black-XY" to "Block-XY" Refactor: #19: Rename "White-XY" to "Allow-XY" and "Black-XY" to "Block-XY" May 18, 2025
return _EMAIL_BACKEND_DOMAIN_WHITELIST


@EMAIL_BACKEND_DOMAIN_WHITELIST.setter
Copy link
Contributor

Choose a reason for hiding this comment

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

question: whats that magic here? 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method is called, when trying to set the EMAIL_BACKEND_DOMAIN_WHITELIST variable


if blacklisted_fields:
warnings.warn(
"blacklisted_fields is deprecated and will be removed in a future version. Use blocklisted_fields instead.",
Copy link
Contributor

Choose a reason for hiding this comment

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

question: do we have tests for those temporary things?

Copy link
Contributor Author

@christoph-teichmeister christoph-teichmeister Jun 11, 2025

Choose a reason for hiding this comment

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

nope, do you want to have tests for deprecation warnings?

I am writing tests for this 🙂

CHANGES.md Outdated
# Changelog

**12.1.6** (2025-05-17)
* Refactor all "white-XY" related code to "allow-XY"
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: imagine you're a user of this package. you don't want to proactively search your code for everything that might have changed. please add all changes explicitly and add a breaking change prefix (check the last majors updates, there are examples)

Example:

Breaking change: EmailWhitelistBackend renamed to EmailAllowlistBackend

Copy link
Contributor Author

@christoph-teichmeister christoph-teichmeister Jun 11, 2025

Choose a reason for hiding this comment

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

I adjusted the Changelog 🙂

Resolved inconsistent spacing in changelog entries.

- Adjusted spacing for breaking change entry in version 12.0.0.
Refactored test cases to reflect updated function signature for `object_to_dict`.

- Adjusted argument order in `test_object_to_dict_blocklist`.
- Updated keyword arguments in test cases to match new parameter names.
Added a new combination of Python 3.13 and Django 5.1 to the CI matrix for compatibility testing.

- Included Python 3.13 with Django 5.1 in workflow configurations.
- Added a note to review if this addition is necessary.
@GitRon
Copy link
Contributor

GitRon commented May 29, 2025

There was an issue with gevent, an updated seems to have solved it. Please merge master into your branch.

…telist-to-allowlist

# Conflicts:
#	CHANGES.md
#	README.md
Improved clarity of instructions for running pre-commit hooks on all files.

- Updated example descriptions for single and all hook executions.
- Replaced ambiguous phrasing for better understanding.
Refined the example for running pre-commit hooks to specify the push stage explicitly.

- Updated example description for running hooks during the push stage.
- Improved clarity by adding `--hook-stage push` to the command.
Eliminated an unnecessary Python 3.13 and Django 5.1 configuration in the GitHub Actions workflow file.

- Removed duplicate Python 3.13 and Django 5.1 combination.
- Addressed a TODO comment regarding its necessity.
Introduced `allowlist` to replace `whitelist` for consistency and inclusivity. Added deprecation warning for the `whitelist` usage, maintaining backward compatibility.

- Added `TEST_STRUCTURE_VALIDATOR_FILE_ALLOWLIST` as the new setting.
- Deprecated `TEST_STRUCTURE_VALIDATOR_FILE_WHITELIST` with a warning.
- Ensured support for existing `whitelist` usage temporarily.
Replaced all occurrences of `whitelist` and `blacklist` with `allowlist` and `blocklist` for consistency and inclusivity. Introduced deprecation warnings for the old terms.

- Renamed `WhitelistEmailBackend` to `AllowlistEmailBackend` in `mail.backends`.
- Updated related method/setting names such as `get_domain_whitelist` to `get_domain_allowlist`.
- Renamed `blacklisted_fields` to `blocklisted_fields` in `object_to_dict` and updated related tests.
- Added deprecation warnings to maintain backward compatibility.
Introduced tests to ensure deprecation warnings are raised when using `get_domain_whitelist` and `whitify_mail_addresses`.

- Added tests to validate warnings for deprecated methods.
- Verified calls to their replacements: `get_domain_allowlist` and `allowify_mail_addresses`.
Introduced a test to ensure `WhitelistEmailBackend` raises a deprecation warning and inherits from `AllowlistEmailBackend`.

- Added test to check warning when instantiating `WhitelistEmailBackend`.
- Verified that it inherits from `AllowlistEmailBackend`.
Added tests ensuring deprecation warnings for `file_whitelist` methods and properties in `StructureTestValidator`.

- Verified warnings for deprecated `file_whitelist` property access and assignments.
- Validated `_get_file_whitelist` issues a warning and calls `_get_file_allowlist`.
@christoph-teichmeister christoph-teichmeister marked this pull request as draft June 11, 2025 12:46
Added a blank line in `settings.py` for better readability.

- Ensured consistency with coding standards.
- Minor adjustment with no functional impact.
Added tests to ensure deprecation warnings for the `TEST_STRUCTURE_VALIDATOR_FILE_WHITELIST` setting usage.

- Verified that accessing the deprecated setting issues a warning and returns the expected value.
- Ensured assigning a value to the deprecated setting issues a warning and sets the expected value.
…telist-to-allowlist

# Conflicts:
#	CHANGES.md
#	ambient_toolbox/utils/model.py
#	docs/features/tests.md
#	tests/ambient_toolbox/test_test_structure_validator.py
…o TEST_STRUCTURE_VALIDATOR_MISPLACED_TEST_FILE_ALLOWLIST

- Updated all occurrences of the deprecated "whitelist" terminology to "allowlist" for misplaced test file settings.
- Added deprecation warnings to maintain backward compatibility.
- Updated documentation and tests accordingly.
…` directory

- Relocated `test_test_structure_validator` module to align with updated test directory structure.
- Maintained backward compatibility by preserving legacy tests.
- Updated paths, imports, and references accordingly.
…ormatting

- Consolidated and optimized import statements across relevant files.
- Improved readability of deprecation warnings by reformatting lengthy strings.
- Reformatted `object_to_dict` function definition for better readability.
- Simplified `is_allowlisted` logic in structure validator tests.
…or tests

- Combined logic to handle both whitelist and allowlist settings for better consistency.
- Added safeguards to prevent duplicate entries in merged lists.
- Extended ignored directories to include `.venv`.
CHANGES.md Outdated
@@ -1,5 +1,22 @@
# Changelog

**12.6.6** (2025-12-17)
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: would be 13.0.0 with breaking changes, right? or is the rest still there and it's not being dropped yet? Then I think it'll be a minor release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a WIP, please wait

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GitRon updated

…telist-to-allowlist

# Conflicts:
#	CHANGES.md
…ings

- Added `_StructureValidatorSettingsModule` to warn and synchronize deprecated whitelist settings with new allowlist terminology.
- Deprecated `TEST_STRUCTURE_VALIDATOR_FILE_WHITELIST` and `TEST_STRUCTURE_VALIDATOR_MISPLACED_TEST_FILE_WHITELIST`.
- Updated logic in `StructureTestValidator` to gracefully handle both legacy and new settings.
- Patched tests to ensure deprecation warnings are triggered and compatibility logic works as expected.
- Extended directory exclusion feature to ignore `.venv`.
…re `.venv` in structure validator tests

- Enhanced `_StructureValidatorSettingsModule` to improve clarity and consistency in deprecation mechanisms.
- Updated deprecation warnings for legacy whitelist attributes to ensure synchronization with allowlist settings.
- Added `.venv` directory to ignored paths in validation logic.
- Included new test case to verify `.venv` exclusion behavior.
@christoph-teichmeister christoph-teichmeister marked this pull request as ready for review December 19, 2025 08:57
@christoph-teichmeister
Copy link
Contributor Author

@GitRon this is ready to be reviewed 🙂

TEST_STRUCTURE_VALIDATOR_IGNORED_DIRECTORY_LIST = []
TEST_STRUCTURE_VALIDATOR_MISPLACED_TEST_FILE_WHITELIST = []

class _StructureValidatorSettingsModule(ModuleType):
Copy link
Contributor

Choose a reason for hiding this comment

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

Over-engineered: _StructureValidatorSettingsModule module proxy (High)

Replacing a simple 13-line settings module with a 119-line ModuleType subclass that does sys.modules[name] = ... is significant over-engineering for a deprecation shim. This pattern:

  • Is hard to understand and maintain
  • Overrides getattr and setattr at module level
  • Syncs state into settings._TEST_STRUCTURE_VALIDATOR_FILE_WHITELIST (private attributes on Django settings — fragile)
  • Could cause subtle import-order bugs

Suggestion: A much simpler approach would be to just rename the constants in the settings file and add a small helper or property in the StructureTestValidator class that emits a warning when the old name is accessed. Or simply rename and bump a major version.

def __init__(self, get_response: Callable[["HttpRequest"], "HttpResponse"]):
warnings.warn(
"CurrentUserMiddleware is deprecated. Use CurrentRequestMiddleware instead.",
"CurrentUserMiddleware is deprecated and will be removed in a future version."
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: missing whitespace after the "." - the strings will be concaternated

".", r"\."
def __init__(self, fail_silently=False, **kwargs):
warnings.warn(
"WhitelistEmailBackend is deprecated and will be removed in a future version."
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: missing whitespace after the "." - the strings will be concaternated

# --------------------- deprecated methods START

@staticmethod
def get_domain_whitelist() -> list:
Copy link
Contributor

Choose a reason for hiding this comment

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

Deprecated methods live on the new class (Medium)

The deprecated get_domain_whitelist() and whitify_mail_addresses() methods are defined on AllowlistEmailBackend itself rather than on the legacy WhitelistEmailBackend wrapper. This means the new, clean class carries the baggage of the old API. These deprecated methods should live on WhitelistEmailBackend
(the compat shim), keeping AllowlistEmailBackend clean.

@@ -1,5 +1,21 @@
# Changelog

**12.6.7** (2025-12-17)
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: just write "no release yet" or something similar, this keeps getting outdated

"get_domain_whitelist() is deprecated and will be removed in a future version."
"Use get_domain_allowlist() instead.",
category=DeprecationWarning,
stacklevel=1,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: apparently, it should be level 2. never tried it, though

  • stacklevel=1 (the default) makes the warning point to the line where warnings.warn() is called — i.e., inside the deprecated function itself
  • stacklevel=2 makes the warning point to the caller of the deprecated function — i.e., the user's code that needs to be updated

"whitify_mail_addresses() is deprecated and will be removed in a future version."
"Use allowify_mail_addresses() instead.",
category=DeprecationWarning,
stacklevel=1,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: apparently, it should be level 2. never tried it, though

  • stacklevel=1 (the default) makes the warning point to the line where warnings.warn() is called — i.e., inside the deprecated function itself
  • stacklevel=2 makes the warning point to the caller of the deprecated function — i.e., the user's code that needs to be updated

"WhitelistEmailBackend is deprecated and will be removed in a future version."
"Use AllowlistEmailBackend instead.",
category=DeprecationWarning,
stacklevel=1,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: apparently, it should be level 2. never tried it, though

  • stacklevel=1 (the default) makes the warning point to the line where warnings.warn() is called — i.e., inside the deprecated function itself
  • stacklevel=2 makes the warning point to the caller of the deprecated function — i.e., the user's code that needs to be updated

"CurrentUserMiddleware is deprecated and will be removed in a future version."
"Use CurrentRequestMiddleware instead.",
category=DeprecationWarning,
stacklevel=1,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: apparently, it should be level 2. never tried it, though

  • stacklevel=1 (the default) makes the warning point to the line where warnings.warn() is called — i.e., inside the deprecated function itself
  • stacklevel=2 makes the warning point to the caller of the deprecated function — i.e., the user's code that needs to be updated

@GitRon
Copy link
Contributor

GitRon commented Feb 6, 2026

@christoph-teichmeister We have conflicts and I asked Claude for a review since I find it quite hard to review and not miss anything 😅

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