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

Added available languages for models #678

Open
wants to merge 62 commits into
base: master
Choose a base branch
from

Conversation

text-adi
Copy link

@text-adi text-adi commented Apr 12, 2023

Added the ability to specify language codes during registration of translation fields. This allows for setting the translations needed for each model.

from modeltranslation.translator import translator, TranslationOptions
from .models import  Category

class PostTranslationOptions(TranslationOptions):
    fields = ('name', 'short_story', 'full_story', 'meta_title', 'meta_descr')

translator.register(
    Post, PostTranslationOptions,
    languages=['en','uk']
)

Progress

  • Feature
  • Docs
  • Tests

@last-partizan
Copy link
Collaborator

Please, update documentation accordingly and add test for this feature.

modeltranslation/translator.py Outdated Show resolved Hide resolved
modeltranslation/translator.py Outdated Show resolved Hide resolved
@last-partizan
Copy link
Collaborator

poetry.lock should not be touched, please revert.

@last-partizan last-partizan self-requested a review April 12, 2023 15:02
@last-partizan
Copy link
Collaborator

Take a look at modeltranslation/tests/tests.py, test_fields.

And add similar for your feature.

  • Create model with specified set of fields
  • Check that it has expected fields and does not have unexpected.

Copy link
Collaborator

@last-partizan last-partizan left a comment

Choose a reason for hiding this comment

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

Looks fine, but you need to add tests before we can merge this.

@text-adi
Copy link
Author

text-adi commented May 15, 2023

Added tests.

@text-adi text-adi marked this pull request as ready for review May 16, 2023 16:02
@text-adi
Copy link
Author

text-adi commented May 16, 2023

Resolved all errors that occurred during the tests and updated the documentation.

docs/modeltranslation/registration.rst Outdated Show resolved Hide resolved
modeltranslation/translator.py Outdated Show resolved Hide resolved
Comment on lines +3612 to +3630
def test_class_field(self):
models.SpecificLanguageModelX.objects.create(title_x_de="foo", text="bar")
models.SpecificLanguageModelX.objects.create(title_x_de="foo")

y_foo = models.SpecificLanguageModelX.objects.filter(title_x_de="foo")
assert 2 == y_foo.count()

y_baz = models.SpecificLanguageModelX.objects.filter(title_x_de="baz")
assert 0 == y_baz.count()

def test_register_decorator(self):
models.SpecificLanguageModelY.objects.create(title_y_de="foo", text="bar")
models.SpecificLanguageModelY.objects.create(title_y_de="foo")

y_foo = models.SpecificLanguageModelY.objects.filter(title_y_de="foo")
assert 2 == y_foo.count()

y_baz = models.SpecificLanguageModelY.objects.filter(title_y_de="baz")
assert 0 == y_baz.count()
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two tests are essentially the same, different only model?

Replace it with single test and @pytest.mark.parametrize("model", ...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Btw, what this test supposed to check?

Copy link
Author

Choose a reason for hiding this comment

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

Basically, this test checked whether an error occurs when creating a database record if a model with the languages parameter is registered.
It is expected that if something goes wrong, the model will not be able to write data or retrieve it.
I will think about how to improve the tests to add more checks during the test.

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