[WIP] fix(gaq): support localized error messages #24457
Conversation
| @@ -96,15 +101,13 @@ def load_chart_data_into_cache( | |||
| else str(ex) | |||
| ) | |||
| errors = [{"message": error}] | |||
There was a problem hiding this comment.
I wonder if this code is repeated; feels like I've seen errors reported as {"message": "(some exception message)"} fairly frequently so maybe it could be more DRY and be made safe against lazy messaging for good by reusing any logic we need surrounding that.
Presumably even with locale here, we still need to be using gettext instead of gettext_lazy when raising the error we've caught here; having the locale context will let us translate the message correctly but won't fix the fact that the json library doesn't know how to serialise LazyString.
There was a problem hiding this comment.
Good idea. I'm still contemplating different solutions to this problem, as there's multiple things that are wrong right now:
- The absence of a locale in celery tasks (which this is primarily trying to address)
- Incorrect use of
lazy_gettextwheregettextis sufficient - serializing/deserializing using
json.
Thinking more about this I'm not so sure anymore if we should really be using flask.json as I've done here (bullet nr 3). So maybe that part should be reverted, and also that error message logic DRY'd up like you are proposing.
There was a problem hiding this comment.
Ah I didn't spot the import change to use flask.json. So flask's json implementation most likely will be able to deal with LazyString, actually.
There was a problem hiding this comment.
It does. However, similar logic may exist elsewhere, so we may run into this same issue again. So at any rate, we should ensure prudent use of gettext/lazy_gettext everywhere.
SUMMARY
Use the user's locale to correctly handle
flask-babeltranslated strings, both regulargettextandlazy_gettext.Related PR which started discussions that led to this PR: #24447
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION