Skip to content

Issues/issue 766 #794

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

Merged
merged 2 commits into from
Jul 13, 2025
Merged

Issues/issue 766 #794

merged 2 commits into from
Jul 13, 2025

Conversation

last-partizan
Copy link
Collaborator

Supersedes #766

Copy link

@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

Adds validation to ensure the fields attribute on translation options isn’t accidentally provided as a single string and includes a test to cover that error case.

  • Introduce a check in FieldsAggregationMetaClass to raise ImproperlyConfigured if fields is a str
  • Add a user-friendly error message with a code hint via textwrap.dedent
  • Add a test verifying that passing a string to fields raises ImproperlyConfigured

Reviewed Changes

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

File Description
modeltranslation/translator.py Add str-type check for fields and raise formatted exception
modeltranslation/tests/tests.py Add test_str_instead_of_tuple to verify the new error path
Comments suppressed due to low confidence (2)

modeltranslation/translator.py:3

  • The exception class ImproperlyConfigured is not imported in this file. Add an import, for example: from django.core.exceptions import ImproperlyConfigured to avoid a NameError when raising the exception.
from textwrap import dedent

modeltranslation/tests/tests.py:8

  • You reference ImproperlyConfigured in the test, but it isn’t imported. Add from django.core.exceptions import ImproperlyConfigured (or the correct module) so pytest.raises(ImproperlyConfigured) works.
from modeltranslation.translator import TranslationOptions

@last-partizan last-partizan merged commit d44a6b8 into master Jul 13, 2025
52 checks passed
@last-partizan last-partizan deleted the issues/issue-766 branch July 13, 2025 09:01
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.

1 participant