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

🪲 Don't crash the "view program" page if the program has a syntax error #5985

Merged
merged 11 commits into from
Dec 2, 2024
45 changes: 28 additions & 17 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1882,23 +1882,34 @@ def view_program(user, id):
result['username']) else None

code = result['code']
# if the snippet has blanks, it'll fail in the translation process
# so we just dont translate it if it has any
has_blanks = hedy.location_of_first_blank(code) > 0
if not has_blanks and result.get("lang") != "en" and result.get("lang") in ALL_KEYWORD_LANGUAGES.keys():
code = hedy_translation.translate_keywords(code, from_lang=result.get(
'lang'), to_lang="en", level=int(result.get('level', 1)))
# If the keyword language is non-English -> parse again to guarantee
# completely localized keywords
if not has_blanks and g.keyword_lang != "en":
code = hedy_translation.translate_keywords(
code,
from_lang="en",
to_lang=g.keyword_lang,
level=int(
result.get(
'level',
1)))
level = int(result.get('level', 1))

# Try to translate the program from the language of the program to the language of the viewer
#
# - We do this in 2 steps: first translate from the source language to English,
# then from English to the target language. There must be a reason for this, but I don't
# know what it is.
# - In the past we used to do this "normalization via English" step for every program,
# even if we would end up translating nl -> nl. I don't know if that was for a particular
# reason but I've changed it to only do the translation if the source and target languages
# are different.
# - Translation may fail, because it requires parsing the program and the program
# may be syntactically invalid (missing indentation, programming mistakes, etc). Or it
# may contain blanks, which will always yield an unparseable program.
# - We don't currently have a way to surface this error to the user. I don't know if
# that matters.
source_language = result.get("lang")
target_language = g.keyword_lang
if source_language != target_language and source_language in ALL_KEYWORD_LANGUAGES.keys():
try:
english_code = hedy_translation.translate_keywords(
code, from_lang=source_language, to_lang="en", level=level)
code = hedy_translation.translate_keywords(
english_code, from_lang="en", to_lang=target_language, level=level)
except Exception as e:
# Not really a good place to leave this error, but at least we don't
# want it crashing the page load. Log it as a warning then.
logger.warning(f"Error translating program {id} from {source_language} to {target_language}: {e}")

result['code'] = code
student_adventure_id = f"{result['username']}-{result['adventure_name']}-{result['level']}"
Expand Down
88 changes: 78 additions & 10 deletions data-for-testing.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,31 @@
"username": "user1",
"password": "$2b$09$1lZSLTxPBn1p6sOBpXy6H./9Fho7fXed.ZZJPBOd4lHu1QwRDiKW6",
"email": "[email protected]",
"language": "en",
"keyword_language": "en",
"language": "fr",
"keyword_language": "fr",
"created": 1667487371548,
"teacher_request": null,
"third_party": null,
"verification_pending": "$2b$09$W/lHayOzZttUVDf6nW0OWuXErrXFV3WC.OXAMTHVJ5hZOahFxPzYW",
"last_login": 1667488074388,
"last_login": 1733156219944,
"country": "VE",
"birth_year": 1999,
"gender": "f",
"prog_experience": "no",
"epoch": 1
"epoch": 1,
"program_count": 1
},
{
"username": "user2",
"password": "$2b$09$BlvY7/i5T1/tyGABEFJOx.6W8LcQHz9HaPrwhxXVZ7gILs4GkYyFe",
"email": "[email protected]",
"language": "en",
"keyword_language": "en",
"language": "es",
"keyword_language": "es",
"created": 1667487426140,
"teacher_request": null,
"third_party": null,
"verification_pending": "$2b$09$8ce7rVDk8ZbMASMP1CRDBepWWXNGZLNuXlhz.2yhXkJUyXELuOdGO",
"last_login": 1667488140990,
"last_login": 1733156278063,
"country": "NL",
"birth_year": 2010,
"gender": "m",
Expand Down Expand Up @@ -310,6 +311,11 @@
"id": "$2b$09$EW1c0OGdZTGk.TmEBAHKlO",
"username": "student3",
"ttl": 1728935624
},
{
"id": "$2b$09$hOgREabUhtSYJqYcMOJHYe",
"username": "user2",
"ttl": 1734365878
}
],
"program-stats": [
Expand Down Expand Up @@ -861,6 +867,46 @@
1,
1
]
},
{
"id#level": "user1#9",
"week": "2024-49",
"id": "user1",
"level": 9,
"number_of_lines": 2,
"NoIndentationException": 10,
"chart_history": [
0,
0,
0,
0,
0,
0,
0,
0,
0,
0
]
},
{
"id#level": "@all-logged#9",
"week": "2024-49",
"id": "@all-logged",
"level": 9,
"number_of_lines": 2,
"NoIndentationException": 10,
"chart_history": [
0,
0,
0,
0,
0,
0,
0,
0,
0,
0
]
}
],
"achievements": [
Expand Down Expand Up @@ -1674,6 +1720,22 @@
"id": "48ec04d60a4c4990a60b9e5e81121e71",
"username_level": "student3-1",
"is_modified": true
},
{
"session": "a583433caabc41af9cff634103fc315d",
"date": 1733156126001,
"lang": "fr",
"version": "DEV",
"level": 9,
"code": "r\u00e9p\u00e8te 1 fois\ntourne 3\n avance 50",
"name": "Introduction 9",
"username": "user1",
"error": true,
"adventure_name": "default",
"id": "fb23d0fa90ce48b5bf87c0632969fc28",
"username_level": "user1-9",
"is_modified": true,
"public": 1
}
],
"public_profiles": [
Expand All @@ -1682,7 +1744,8 @@
"image": "1",
"personal_text": "I like Hedy!",
"agree_terms": "on",
"tags": []
"tags": [],
"country": "VE"
},
{
"username": "user3",
Expand Down Expand Up @@ -1879,10 +1942,10 @@
"students": {
"$type": "set",
"elements": [
"student2",
"student3",
"student4",
"student1",
"student3",
"student2",
"student5"
]
},
Expand Down Expand Up @@ -2788,6 +2851,11 @@
"id": "student3-default-1",
"ticked": false,
"program_id": "02930e0a15894bc68331775f08e12a32"
},
{
"id": "user1-default-9",
"ticked": false,
"program_id": "fb23d0fa90ce48b5bf87c0632969fc28"
}
]
}
7 changes: 2 additions & 5 deletions tests/cypress/e2e/hedy_page/editor_box.cy.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const YAML = require('js-yaml')
import { codeMirrorContent } from '../tools/programs/program';

describe('Is able to type in the editor box', () => {
const LANGUAGES_TO_TEST = ['en', 'ar', 'bg', 'bn', 'ca', 'cs', 'cy', 'da', 'de','el', 'eo', 'es', 'et', 'fa', 'fi', 'fr', 'fy', 'he', 'hi', 'hu', 'id', 'it', 'ja', 'kmr', 'ko', 'nb_NO', 'nl', 'pa_PK', 'pl', 'pt_BR', 'pt_PT', 'ro', 'ru', 'sq', 'sr', 'sv', 'sw', 'te', 'th']
Expand Down Expand Up @@ -95,8 +96,4 @@ describe('Test editor box functionality', () => {
function clearViaBackspace() {
cy.focused().type('{moveToEnd}' + '{backspace}'.repeat(40));
codeMirrorContent().should('have.text', '');
}

function codeMirrorContent() {
return cy.get('#editor > .cm-editor > .cm-scroller > .cm-content');
}
}
11 changes: 6 additions & 5 deletions tests/cypress/e2e/hedy_page/invalid_program.cy.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { goToHedyPage } from "../tools/navigation/nav";
import { codeMirrorContent } from "../tools/programs/program";

describe('Error code gives correct error', () => {
describe('Misspelled Keyword', () => {
Expand All @@ -7,7 +8,7 @@ describe('Error code gives correct error', () => {
const error_message = `We detected that prnt is not a Hedy level 1 command. Can you try using print?`;
goToHedyPage();

cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
cy.focused().type(error_code);

cy.intercept('/parse').as('parse')
Expand All @@ -21,7 +22,7 @@ describe('Error code gives correct error', () => {
const error_message = `We detected that ak is not a Hedy level 1 command. Can you try using ask?`;
goToHedyPage();

cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
cy.focused().type(error_code);

cy.intercept('/parse').as('parse')
Expand All @@ -36,7 +37,7 @@ describe('Error code gives correct error', () => {
const error_message = `We detected that the code seems to be missing a command on line 1. Can you try looking at the exercise section to find which command to use?`;
goToHedyPage();

cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
cy.focused().type(error_code);

cy.intercept('/parse').as('parse')
Expand All @@ -50,7 +51,7 @@ describe('Error code gives correct error', () => {
const error_message = `We detected that forward doesn't work with lalala because it is text. Can you try changing lalala to a number or input from ask?`;
goToHedyPage();

cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
Comment on lines -53 to +54
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

❤️

cy.focused().type(error_code);

cy.intercept('/parse').as('parse')
Expand All @@ -64,7 +65,7 @@ describe('Error code gives correct error', () => {
const error_message = `We detected that turn is not usable with test. Can you try changing test to right or left?`;
goToHedyPage();

cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
cy.focused().type(error_code);

cy.intercept('/parse').as('parse')
Expand Down
3 changes: 2 additions & 1 deletion tests/cypress/e2e/hedy_page/keyword_language.cy.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {goToHedyPageWithEnKeywords} from "../tools/navigation/nav";
import { codeMirrorContent } from "../tools/programs/program";

describe('when the user changes their language to Arabic', () => {
beforeEach(() => {
goToHedyPageWithEnKeywords();
cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
cy.get('#editor').type("print Hallo!'\n");
cy.getDataCy('language_dropdown_button').click();
cy.getDataCy('switch_lang_ar').click();
Expand Down
3 changes: 2 additions & 1 deletion tests/cypress/e2e/hedy_page/to_text_tabs_button.cy.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {goToHedyPage} from "../tools/navigation/nav";
import { codeMirrorContent } from "../tools/programs/program";

describe('Navigating through the tabs with the buttons', () => {
it('should be able to go to the next and previous tab', () => {
Expand All @@ -12,7 +13,7 @@ describe('Navigating through the tabs with the buttons', () => {
// Test when code is changed
goToHedyPage();
cy.wait(500)
cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').click();
codeMirrorContent().click();
cy.focused().type('hello');
cy.get('.next-tab').click();
cy.wait(500)
Expand Down
5 changes: 3 additions & 2 deletions tests/cypress/e2e/hedy_page/try_button.cy.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {goToHedyPage} from "../tools/navigation/nav";
import { codeMirrorContent } from "../tools/programs/program";

describe('Is able to use try pre formatted code', () => {
it('The cheatsheet code is added to the end of the editor', () => {
goToHedyPage();
cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').clear()
cy.get('#editor > .cm-editor > .cm-scroller > .cm-content').type('print hello world')
codeMirrorContent().clear()
codeMirrorContent().type('print hello world')

cy.get('#dropdown_cheatsheet_button').click();
cy.get('#cheatsheet_dropdown').should('be.visible');
Expand Down
10 changes: 10 additions & 0 deletions tests/cypress/e2e/public_programs/view_programs.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { login } from "../tools/login/login"
import { codeMirrorContent } from "../tools/programs/program";

describe('Tests for viewing programs', () => {
it('Seeing a program that contains an error in a different language, shouldnt result in a crash', () => {
login('user2', '123456');
cy.visit('/hedy/fb23d0fa90ce48b5bf87c0632969fc28/view')
codeMirrorContent().should('have.text', 'répète 1 foistourne 3 avance 50');
})
})
6 changes: 5 additions & 1 deletion tests/cypress/e2e/tools/programs/program.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,8 @@ export function deleteProgram(name) {
})
}
})
}
}

export function codeMirrorContent() {
return cy.get('#editor > .cm-editor > .cm-scroller > .cm-content');
}
Loading