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

Add CalculationSummary Titles to translations #19

Merged
merged 1 commit into from Aug 3, 2018

Conversation

dwyeradam
Copy link
Contributor

Titles should be translatable for CalculationSummary blocks.
These are the main titles of the block but also the titles with the calculation object.

},
"metadata": {
"user_id": {
"validator": "string"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

@jonnyshaw89 We missed the metadata changes here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've edited this in my PR anyway.

benhowes
benhowes previously approved these changes Aug 1, 2018
@dwyeradam
Copy link
Contributor Author

Possible I've only done half a job here. Need to check the translation script works.

dcdarrell9
dcdarrell9 previously approved these changes Aug 2, 2018
@@ -79,6 +79,9 @@ def get_non_question_translatable_text(container):
extracted_text.extend(get_introduction_translatable_text(container))
elif container['type'] == 'Interstitial':
extracted_text.extend(get_interstitial_translatable_text(container))
elif container['type'] == 'CalculatedSummary':
extracted_text.extend(get_titles_text(container['titles'], container['id']))
extracted_text.extend(get_titles_text(container['calculation']['titles'], container['id']))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could possibly change this to get_calculated_summary_text or something like that.

But works fine and probably not worth changing 👍

@dwyeradam dwyeradam dismissed stale reviews from benhowes and dcdarrell9 August 2, 2018 16:01

New commit. Had only done extract. Not the translate.

Copy link
Contributor

@dcdarrell9 dcdarrell9 left a comment

Choose a reason for hiding this comment

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

Extracts and translates everything in the test schema for me and passing tests... Might be worth improving the tests as they do the bare bare minimum

Copy link
Contributor

@benhowes benhowes left a comment

Choose a reason for hiding this comment

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

LGTM

get_calculated_summary_translatable_text method
@dwyeradam dwyeradam force-pushed the eq-1882-calculate-total-playback branch from 63d94f6 to b2d4266 Compare August 3, 2018 09:57
@jonnyshaw89 jonnyshaw89 merged commit 1f8dcf3 into master Aug 3, 2018
@jonnyshaw89 jonnyshaw89 deleted the eq-1882-calculate-total-playback branch August 3, 2018 09:59
iwootten pushed a commit that referenced this pull request Feb 21, 2019
# This is the 1st commit message:

Add support for translation of schemas using Pot Files

# This is the commit message #2:

Fix broken tests

# This is the commit message #3:

Correct spacing

# This is the commit message #4:

Remove test output from repo

# This is the commit message #5:

Label schema appropriately

# This is the commit message #6:

Tidy up naming

# This is the commit message #7:

Move main method into extractor class

# This is the commit message #8:

Use static methods where appropriate

# This is the commit message #9:

Add csv tranform test

# This is the commit message #10:

Use argparse throughout

# This is the commit message #11:

Correct spacing

# This is the commit message #12:

Remove common methods and put them in one place

# This is the commit message #13:

Remove common methods and put them in one place

# This is the commit message #14:

Correct output path

# This is the commit message #15:

Remove output from test

# This is the commit message #16:

Create paths when they don't exist

# This is the commit message #17:

Move translator class into separate file

# This is the commit message #18:

Correct Spacing

# This is the commit message #19:

Remove ability to transform to xls

# This is the commit message #20:

Remove unused import

# This is the commit message #21:

Remove unneccessary translation object call

# This is the commit message #22:

Add missing method

# This is the commit message #23:

Correct spacing

# This is the commit message #24:

Remove all xls transforms from repo

# This is the commit message #25:

Add newline to header to be importable

# This is the commit message #26:

Remove all shell scripts and replace with python

# This is the commit message #27:

Workaround for travis numpy weirdness

# This is the commit message #28:

Block empty legal_basis

# This is the commit message #29:

Block empty legal_basis

# This is the commit message #30:

Block missin legal basis

# This is the commit message #31:

Remove csv conversion from extractor

# This is the commit message #32:

Update readme

# This is the commit message #33:

Remove TODO's

# This is the commit message #34:

Only allow po files

# This is the commit message #35:

Remove condition

# This is the commit message #36:

Only ask for output path on save

# This is the commit message #37:

Remove context to meet Adam's original method calls

# This is the commit message #38:

Correct argument description:

# This is the commit message #39:

Use cwd for source of parent

# This is the commit message #40:

Remove '_translate_' from naming

# This is the commit message #41:

Remove duplicate file

# This is the commit message #42:

Don't attempt to translate if template but no schema

# This is the commit message #43:

Add better description

# This is the commit message #44:

Address comment

# This is the commit message #45:

Remove context from everything in extract template except answer options. Only print answer id's as template comments
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.

None yet

5 participants