Translate evaluation state names in log display

[DieKautz]

This closes #1693.

[richardebeling]

Code looks good to me -- it would be nice if you could add a small test.

[janno42]

It seems that the label "State" is not yet translated in the log, although I don't know why. Can you take a look at this?

[DieKautz]

Ah. I forgot to add that a pipeline fail caused by this would be expected, as we already discovered that "State" (capitalized) would still have to be (fuzzy) translated by you(?).
Also "editor_approved" would need translation

[DieKautz]

I cannot attend in-person today. But hopefully it should now be ready for the final review :) @richardebeling

edit: nvm, for some reason overriding the language works different in the ci? (all 575 pass locally)

[richardebeling]

edit: nvm, for some reason overriding the language works different in the ci? (all 575 pass locally)

The issue was with the path that you patched -- see the Where to patch? part of the documentation for more details. In short, if a name is already imported in some location, patching the source-location afterwards doesn't work.

For your test, it should work if you patch the name evap.evaluation.models._ -- the following test passes for me:

    @patch("evap.evaluation.models._", new=(lambda string: "vorbereitet" if string == "prepared" else string))
    def test_state_change_log_translated(self):
        log_ul = self.app.get(self.url, user=self.manager).html.find(class_="list-group")
        self.assertNotIn("vorbereitet", str(log_ul))

        self.evaluation.ready_for_editors()
        self.evaluation.save()

        log_ul = self.app.get(self.url, user=self.manager).html.find(class_="list-group")
        self.assertIn("vorbereitet", str(log_ul))

To get the same behavior locally as with the CI run, you can simply rm evap/locale/de/LC_MESSAGES/django.mo

[DieKautz]

For your test, it should work if you patch the name evap.evaluation.models._ -- the following test passes for me:

When executing just this single test, it does not pass for me if self.managers lanaguage is not overwritten.
Could this be because language is not reset in an earlier test, maybe here?

[richardebeling]

Ah, sorry. When we moved the _(...) call out of the body of state_to_str, we changed the time when the _ method is evaluated. The call inside the function is evaluated during test runtime. The class variable is evaluated before the test runs, so by the time we arrive at the test, the value is already a string marked to be lazily translated. Thus, at that point, patching evap.evaluation.models._ does not change anything anymore, the strings will still be translated by the default django mechanics. These then use the language of self.manager, which is english by default. (The whole idea behind the patching was that the language of self.manager doesn't matter, because we overwrite the translation result for all languages.)

Moving the _() call to the values of the STATE_STR_CONVERSION certainly is the right thing to do here. However, I don't see a nice way to overwrite the translation result for the test with this setup. Django has an override_settings helper to dynamically change setting values for tests. Optimally, we'd have an equivalent for translation, but they don't seem to have that.

My concern before, when I asked you to write a test, was that the code could (in future) not call state_to_str, but use STATE_STR_CONVERSION directly and thus fail to translate. Considering that STATE_STRING_CONVERSION is now translated, "failing to translate" can not really be a problem anymore. We could thus just test that the values of STATE_STRING_CONVERSION are used. This test passes for me:

    @patch.dict(Evaluation.STATE_STR_CONVERSION, {Evaluation.State.PREPARED: "mock-translated-prepared"})
    def test_state_change_log_translated(self):
        log_ul = self.app.get(self.url, user=self.manager).html.find(class_="list-group")
        self.assertNotIn("mock-translated-prepared", str(log_ul))

        self.evaluation.ready_for_editors()
        self.evaluation.save()

        log_ul = self.app.get(self.url, user=self.manager).html.find(class_="list-group")
        self.assertIn("mock-translated-prepared", str(log_ul))

[niklasmohrin]

Nice, thanks!

[janno42]

Thanks