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

smart_translate_item_data: translate strings automatically #3453

Merged

Conversation

aronasorman
Copy link
Collaborator

Fixes #3413.

Upon reading KA's code we apparently don't translate a list of strings inside an assessment item properly. The symptom of this is the 500 error code which @EdDixon caught. So now, when we see a string (perhaps passed as an argument in item_data[field] = map(smart_translate_item_data, field_data), we now just translate that string directly.

Now also features tests! (tm)

See, in smart_translate_item_data we always assumed that a list inside
assessment items is always a dict. Now, apparently it always isn't,
based on the behaviour we see from KA's code: https://gist.github.com/aronasorman/e8f77ad7444d55c5a5e1#file-models-py-L127

So now we just replicate that behaviour in smart_translate_item_data by
translating strings, and returning immediately.
@jamalex
Copy link
Member

jamalex commented Mar 30, 2015

Looks good to me (pending testing). Will merge now, but @rtibbles please let us know if you see any issues (as this reverts and replaces 82fe841)

jamalex added a commit that referenced this pull request Mar 30, 2015
smart_translate_item_data: translate strings automatically
@jamalex jamalex merged commit 7592c1c into learningequality:0.13.x Mar 30, 2015
@jamalex jamalex removed the has PR label Mar 30, 2015
@rtibbles
Copy link
Member

Should be fine, as long as all the data that gets passed to the function is
only ever a string or a dictionary.

On Mon, 30 Mar 2015 13:00 Jamie Alexandre [email protected] wrote:

Merged #3453 #3453.


Reply to this email directly or view it on GitHub
#3453 (comment).

@aronasorman aronasorman deleted the translate_list_of_strings branch March 31, 2015 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants