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.

Sign up for GitHub

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

Fix part of #6550: Write tests for controllers/editor #6849

Merged
merged 12 commits into from
Jun 13, 2019
Merged

Fix part of #6550: Write tests for controllers/editor #6849

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ab8d5a2
Merge stash
Jun 2, 2019
b317870
Write tests for controllers/editor
Jun 5, 2019
8c1b30b
Merge branch 'develop' into new_editor_test
Jun 5, 2019
73a5688
package-lock
Jun 5, 2019
d417645
Fix lint
Jun 5, 2019
8df5a37
Fix tests
Jun 5, 2019
e406e09
Merge develop
Jun 10, 2019
e645367
Addressed comments
Jun 10, 2019
b74388c
Addressed comments
Jun 10, 2019
c5b6344
Merge branch 'develop' into new_editor_test
Jun 11, 2019
8c31ca0
Added comment
Jun 11, 2019
3fb9f3c
Merge develop
Jun 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Merge stash
  • Loading branch information
Rishav Chakraborty committed Jun 2, 2019
commit ab8d5a24368a8f3fea4041690e764ca7d589d1db
25 changes: 5 additions & 20 deletions core/controllers/editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,6 @@ class ExplorationPage(EditorHandler):
@acl_decorators.can_play_exploration
def get(self, exploration_id):
"""Handles GET requests."""
if exploration_id in constants.DISABLED_EXPLORATION_IDS:
self.render_template(
'pages/error/disabled_exploration.html',
iframe_restriction=None)
return

exploration_rights = rights_manager.get_exploration_rights(
exploration_id)
Expand Down Expand Up @@ -271,11 +266,7 @@ def put(self, exploration_id):

elif make_community_owned:
exploration = exp_services.get_exploration_by_id(exploration_id)
try:
exploration.validate(strict=True)
except utils.ValidationError as e:
Copy link
Member

Choose a reason for hiding this comment

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

This needs to stay, don't assume old production data is necessarily valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the line above, we are using exploration.validate(strict=True). Strict means that it will raise an exception from the validate function and hence this line becomes unreachable.

Copy link
Member

Choose a reason for hiding this comment

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

Why is it unreachable? The whole point of try/except is to catch an exception, and if a ValidationError is raised then the except clause will be triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

raise self.InvalidInputException(e)

exploration.validate(strict=True)
rights_manager.release_ownership_of_exploration(
self.user, exploration_id)

Expand Down Expand Up @@ -416,10 +407,7 @@ class ExplorationFileDownloader(EditorHandler):
@acl_decorators.can_download_exploration
def get(self, exploration_id):
"""Handles GET requests."""
try:
exploration = exp_services.get_exploration_by_id(exploration_id)
except:
raise self.PageNotFoundException
exploration = exp_services.get_exploration_by_id(exploration_id)

version_str = self.request.get('v', default_value=exploration.version)
output_format = self.request.get('output_format', default_value='zip')
Expand Down Expand Up @@ -456,11 +444,8 @@ class StateYamlHandler(EditorHandler):
@acl_decorators.can_play_exploration
def post(self, unused_exploration_id):
"""Handles POST requests."""
try:
state_dict = self.payload.get('state_dict')
width = self.payload.get('width')
except Exception:
raise self.PageNotFoundException
state_dict = self.payload.get('state_dict')
width = self.payload.get('width')

self.render_json({
'yaml': state_domain.State.convert_state_dict_to_yaml(
Expand All @@ -477,7 +462,7 @@ class ExplorationResourcesHandler(EditorHandler):
def get(self, exploration_id):
"""Handles GET requests."""
fs = fs_domain.AbstractFileSystem(
fs_domain.ExplorationFileSystem(exploration_id))
fs_domain.ExplorationFileSystem('exploration/%s' % exploration_id))
dir_list = fs.listdir('')

self.render_json({'filepaths': dir_list})
Expand Down
Loading