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

Fix #13162: Add schema and introduce new approach for saving multiple images in suggestion handler. #14063

Merged
merged 186 commits into from Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
186 commits
Select commit Hold shift + click to select a range
601bb98
Add schema validation for suggestion handler
Oct 14, 2021
e662578
Add cmd constants of diffetent domain types
Oct 14, 2021
be889c1
Add validate_change method
Oct 14, 2021
2e07409
Remove suggestion handler name
Oct 14, 2021
f7ea2f5
Add skill_difficulty attribute
Oct 14, 2021
0d898ce
Add changes after Linter checks
Oct 14, 2021
09e2a69
Add changes after Linter checks
Oct 14, 2021
96bc10e
Remove Trailing whitespace
Oct 14, 2021
6ff7c73
Remove quotes from a string
Oct 14, 2021
eb9a4f1
Remove skill_difficulty attribute
Oct 15, 2021
901a1ae
Add QuestionSuggestionChange class in validate_change method
Oct 15, 2021
d8584bc
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 15, 2021
092e575
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 16, 2021
a18acf4
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 17, 2021
532ac78
Add schema for handling images
Oct 17, 2021
de69b61
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Oct 17, 2021
80a86f9
Add whitespace after colon
Oct 17, 2021
ed862b3
Add space before curly braces
Oct 17, 2021
10a9cc5
Change validate_change method name as suggested
Oct 17, 2021
1abcc4b
Rename argument for validation method in suggestion handler
Oct 17, 2021
47529f7
Remove exception handling in suggestion handler
Oct 17, 2021
a154c5f
Line too long lint check
Oct 17, 2021
60be8a4
Add method in next line
Oct 17, 2021
d59e7f3
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 19, 2021
683947f
Create a list of allowed commands in Question and Exp domain
Oct 19, 2021
5ad7307
Add a blank line
Oct 19, 2021
ba02f47
Add comments in a domain validator function
Oct 19, 2021
5ce77c1
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 21, 2021
6d20006
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 22, 2021
5e2c269
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 22, 2021
90e5cf1
Change key to access image blob
Oct 23, 2021
cd178d4
Replace fieldname args in upload_files method from file_name to image
Oct 23, 2021
5f0a97e
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 23, 2021
8e0587d
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Oct 23, 2021
72a94d1
Add whitespace
Oct 23, 2021
74dd86e
Write two statements equivalent to pass a test case
Oct 24, 2021
873b06b
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 24, 2021
bd2dc22
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 26, 2021
1557b61
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 27, 2021
5a53d36
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 28, 2021
911119d
Write validation_method for suggestion images
Oct 28, 2021
3c4e98e
Write schema for suggestion images
Oct 28, 2021
f30d150
Write scripts to fetch upload_suggestion_images
Oct 28, 2021
4eb3d84
Replace file content from upload_files to an object
Oct 28, 2021
597b1b3
Convert image blob from bytes to str
Oct 28, 2021
72353ac
Make changes after link checks
Oct 28, 2021
80de151
Make changes after link checks
Oct 28, 2021
e27d217
Make changes after link checks
Oct 28, 2021
6991138
Replace upload_files param with files object
Oct 29, 2021
ad08df4
Introduce a condition to handle images
Oct 29, 2021
6173625
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Oct 29, 2021
d3df22f
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Oct 29, 2021
97a7585
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 2, 2021
b089ba8
Pass payload object to save images
Nov 2, 2021
78c14f5
Revert "Pass payload object to save images"
Nov 2, 2021
dcb7d78
Pass payload object in upload_suggestion() to obtain images
Nov 2, 2021
4f5c44b
Perform encoding for all images
Nov 2, 2021
01431c3
Add condition for images that are string type
Nov 2, 2021
7b223ce
Add encoding for images
Nov 2, 2021
85b4904
Add condition for images that are of type string or bytes
Nov 2, 2021
38f13ee
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Nov 2, 2021
d4ff698
Decode images of type bytes
Nov 2, 2021
0d95113
Add whitespace
Nov 2, 2021
e011b81
Fix Line too long lint check
Nov 2, 2021
12b7029
Add annotations future import
Nov 2, 2021
7f4616b
Correctly sort imports
Nov 2, 2021
682201a
Write unit tests for ExplorationOrQuestion Change
Nov 2, 2021
861e6ee
Fix Lint check errors
Nov 2, 2021
2b2b2a6
Remove some arguments from domain_dict
Nov 2, 2021
4c797df
Add condition to validate missing cmd key
Nov 2, 2021
bb9de26
Add 'is' for reference equality to fix lint error
Nov 2, 2021
134f298
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 2, 2021
c02c589
Fix bugs - ExplorationOrQuestionsTests Class
Nov 3, 2021
f7074a1
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 3, 2021
0009585
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Nov 3, 2021
ca8ded1
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 3, 2021
97574ac
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 5, 2021
f2b20b3
Create lists to check domain allowed commands
Nov 5, 2021
ff76f69
Shift block of code left
Nov 5, 2021
5e10c46
Pass files dict to save images
Nov 5, 2021
365463f
Encode image data to write on disks
Nov 5, 2021
277cc17
Indent blocks of code and pass images through files dict
Nov 5, 2021
3f45b7f
Encode image data
Nov 5, 2021
f010dd4
Remove allowed commands list
Nov 5, 2021
33491f1
Remove allowed commands list
Nov 5, 2021
8e60118
Encode raw_image to bytes to validate
Nov 5, 2021
2397250
Decode images
Nov 5, 2021
f00513b
Write functions to encode and decode image
Nov 5, 2021
66dee09
Add intendation
Nov 5, 2021
c3c856e
Images are encoded/decoded from utils module
Nov 5, 2021
e8e103d
Make changes after lint checks
Nov 5, 2021
54e3219
Make changes after lint checks
Nov 5, 2021
0c6affd
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 6, 2021
cb0ebaf
Revert "Make changes after lint checks"
Nov 6, 2021
90c12db
Make changes after lint check
Nov 6, 2021
89785b6
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 8, 2021
37e0837
Add function to check image data
Nov 8, 2021
bb073e9
Add conditions to handle image data in suggestion handler
Nov 8, 2021
537b182
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Nov 8, 2021
98600c7
Make corrections after lint checks
Nov 8, 2021
0871c54
Make changes after lint checks
Nov 8, 2021
7c3f1f7
Break line after paranthesis
Nov 8, 2021
7ae03f6
Add 4 space indentation
Nov 8, 2021
bbbbeb9
Fix lint check errors
Nov 8, 2021
0b9d0d4
Fix lint check errors
Nov 8, 2021
150311b
Fix lint check error
Nov 8, 2021
67be327
Fix lint check error
Nov 8, 2021
c24a813
Add 4 space indentation to fix lint check error
Nov 8, 2021
77137c2
Make changes after code review
Nov 8, 2021
d400971
Add test for is_base64_encoded method
Nov 8, 2021
5261ec7
Add an issue reference to handle images in suggestion controller
Nov 8, 2021
adab06a
Fix lint check errors
Nov 8, 2021
98be9db
Fix error in a test case
Nov 8, 2021
d0a4eb4
Fix call to untyped function error
Nov 8, 2021
541eda6
Add no-untyped-call comment
Nov 8, 2021
354efab
Add test for ValidSuggestionImages
Nov 9, 2021
029d9ef
FIx lint check errors
Nov 9, 2021
5b33a80
Put comment in a new line
Nov 9, 2021
6220cb7
Fix errors to pass lint check
Nov 9, 2021
3087e1c
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 9, 2021
64ef728
Add a test case to fix coverage failure
Nov 9, 2021
bf057ad
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Nov 9, 2021
6baa95f
Remove trailing whitespace
Nov 9, 2021
c029d82
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 10, 2021
809a1ed
Make changes after final review
Nov 10, 2021
d86de72
Add encoding args in test
Nov 10, 2021
45c7fa6
Make some changes after code review
Nov 12, 2021
b65f746
Encode image blobs from frontend for suggestion handler
Nov 17, 2021
f93c06e
Declare files as undefined
Nov 17, 2021
e44ce72
Fix typescript check fail
Nov 17, 2021
c3f05ad
Fix lint check issues
Nov 17, 2021
aec86ed
Refactor code to send multiple images to suggestion handler
Nov 19, 2021
d122e30
Fix typescript lint checkts
Nov 19, 2021
da42af6
Merge branch 'develop' into add-schema-suggestion-handler
chiragbaid7 Nov 19, 2021
a24b480
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Nov 19, 2021
ba2fa3f
Remove suggestionhandler from schema handler class list
Nov 22, 2021
9e87910
Make changes to add files as a key in postData
Nov 22, 2021
4164c36
Fix Lint check errors
Nov 22, 2021
0a0fb76
Remove ImagesDict function
Nov 23, 2021
9cb304d
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 24, 2021
275c628
Update translate-text.service.ts
chiragbaid7 Nov 25, 2021
b6c64d5
Update translation-modal.component.ts
chiragbaid7 Nov 25, 2021
00c01a3
Merge branch 'develop' into add-schema-suggestion-handler
chiragbaid7 Nov 27, 2021
6e74707
Add description in suggestion tests
chiragbaid7 Nov 27, 2021
2322936
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 29, 2021
7ec2c60
Make changes after code review
Nov 30, 2021
d6a6415
Add flushMicrotasks() and make some changes in Frontend tests
Nov 30, 2021
f05a205
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Nov 30, 2021
34c605f
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Nov 30, 2021
37188a5
Make changes to fix coverage test issue
Dec 1, 2021
9c5ba71
Write comments in blobtobase64() method
Dec 1, 2021
cc55c92
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Dec 1, 2021
dcfc3b5
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 1, 2021
a1d6ce7
Add interface for posData to support optional key
Dec 2, 2021
cca58d6
Make changes to fix front-end failures
Dec 2, 2021
3f03300
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Dec 2, 2021
980abe0
Fix lint check errors
Dec 2, 2021
449e63f
Make changes to pass coverage tests
Dec 3, 2021
2a6c417
Merge branch 'develop' into add-schema-suggestion-handler
chiragbaid7 Dec 3, 2021
19d7530
Fix lint errors
Dec 3, 2021
7396e74
Merge branch 'develop' into add-schema-suggestion-handler
chiragbaid7 Dec 3, 2021
a7319de
Make changes after review
Dec 5, 2021
880a1b7
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Dec 5, 2021
96a4e3a
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 5, 2021
f8d430e
Make changes in frontend after code review
Dec 8, 2021
ba209d3
Make changes in backend after code review
Dec 8, 2021
29f2137
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Dec 8, 2021
2f49274
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 8, 2021
9f5e58d
Fix lint check issues
Dec 8, 2021
f85bf72
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Dec 8, 2021
8ca1c3c
Fix lint check issues
Dec 8, 2021
6d4f2bb
Return objects after validation
Dec 8, 2021
e24082d
Fix lint check issues
Dec 8, 2021
dbf42be
Fix lint check issues
Dec 8, 2021
12da771
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 8, 2021
e2c2f85
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 8, 2021
61d67bd
Fix errors
Dec 9, 2021
53cbd6b
Fix backend test issues
Dec 9, 2021
a02e7fd
Add whitespace
Dec 9, 2021
d536a4c
Fix backend test failure
Dec 10, 2021
15da453
Add validation and test case for prefix in data url
Dec 10, 2021
118ab53
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 10, 2021
06630ee
Address review comments
Dec 10, 2021
c13841a
Merge branch 'add-schema-suggestion-handler' of https://github.com/ch…
Dec 10, 2021
9a95622
Update image_validation_services_test.py
chiragbaid7 Dec 10, 2021
31ee29f
Merge branch 'oppia:develop' into add-schema-suggestion-handler
chiragbaid7 Dec 13, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions core/controllers/domain_objects_validator.py
Expand Up @@ -21,18 +21,36 @@
from __future__ import absolute_import
from __future__ import unicode_literals

from core import feconf
from core import python_utils
from core.constants import constants
from core.controllers import base
from core.domain import blog_domain
from core.domain import collection_domain
from core.domain import config_domain
from core.domain import exp_domain
from core.domain import question_domain
from core.domain import state_domain

from typing import Dict, Optional, Union


def validate_change(obj):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def validate_change(obj):
def validate_exploration_or_question_change(obj):

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.

"""Validates Exploration and Question domain change.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Validates Exploration and Question domain change.
"""Validates Exploration or Question change.

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.


Args:
obj: dict. Data that needs to be validated.
vojtechjelinek marked this conversation as resolved.
Show resolved Hide resolved
"""
if obj['cmd'] in feconf.LIST_CMD_EXPLORATION_CHANGE:
exp_domain.ExplorationChange(obj)
elif obj['cmd'] in feconf.LIST_CMD_QUESTION_CHANGE:
question_domain.QuestionSuggestionChange(obj)
Copy link
Member

Choose a reason for hiding this comment

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

Should we call validate explicitly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you talking about validation method in the schema for change param?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind. Can you just add a comment that the change is validated when the object is created?

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.

else:
raise base.BaseHandler.InvalidInputException(
'%s cmd is not allowed.' % obj['cmd']
)


def validate_exploration_change(obj):
"""Validates exploration change.

Expand Down
1 change: 0 additions & 1 deletion core/controllers/payload_validator.py
Expand Up @@ -197,7 +197,6 @@ def convert_string_to_bool(param: str) -> Optional[Union[bool, str]]:
'SubtopicPageDataHandler',
'SubtopicViewerPage',
'SuggestionEmailHandler',
'SuggestionHandler',
'SuggestionListHandler',
'SuggestionToExplorationActionHandler',
'SuggestionToSkillActionHandler',
Expand Down
60 changes: 54 additions & 6 deletions core/controllers/suggestion.py
Expand Up @@ -27,6 +27,7 @@
from core.constants import constants
from core.controllers import acl_decorators
from core.controllers import base
from core.controllers import domain_objects_validator
from core.domain import exp_fetchers
from core.domain import fs_services
from core.domain import html_cleaner
Expand All @@ -40,21 +41,68 @@
class SuggestionHandler(base.BaseHandler):
""""Handles operations relating to suggestions."""

URL_PATH_ARGS_SCHEMAS = {}
HANDLER_ARGS_SCHEMAS = {
'POST': {
'suggestion_type': {
'schema': {
'type': 'basestring',
'choices': feconf.SUGGESTION_TYPE_CHOICES
}
},
'target_type': {
'schema': {
'type': 'basestring',
'choices': feconf.SUGGESTION_TARGET_TYPE_CHOICES
}
},
'target_id': {
'schema': {
'type': 'basestring'
}
},
'target_version_at_submission':
{
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'target_version_at_submission':
{
'target_version_at_submission': {

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

'schema': {
'type': 'int',
'validators': [{
'id': 'is_at_least',
'min_value': 1
}]
}
},
'change': {
'schema': {
'type': 'object_dict',
'validation_method': (
domain_objects_validator.validate_change
)
}
},
'description': {
'schema': {
'type': 'basestring'
}
}
}
}

@acl_decorators.can_suggest_changes
def post(self):
"""Handles POST requests."""
if (self.payload.get('suggestion_type') ==
if (self.normalized_payload.get('suggestion_type') ==
feconf.SUGGESTION_TYPE_EDIT_STATE_CONTENT):
raise self.InvalidInputException(
'Content suggestion submissions are no longer supported.')
vojtechjelinek marked this conversation as resolved.
Show resolved Hide resolved

try:
suggestion = suggestion_services.create_suggestion(
self.payload.get('suggestion_type'),
self.payload.get('target_type'), self.payload.get('target_id'),
self.payload.get('target_version_at_submission'),
self.user_id, self.payload.get('change'),
self.payload.get('description'))
self.normalized_payload.get('suggestion_type'),
self.normalized_payload.get('target_type'),
self.normalized_payload.get('target_id'),
self.normalized_payload.get('target_version_at_submission'),
self.user_id, self.normalized_payload.get('change'),
self.normalized_payload.get('description'))
except utils.ValidationError as e:
raise self.InvalidInputException(e)
Copy link
Member

Choose a reason for hiding this comment

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

Can this still happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this can be removed as validation is being performed,


Expand Down
16 changes: 16 additions & 0 deletions core/feconf.py
Expand Up @@ -1479,6 +1479,22 @@ def get_empty_ratings() -> Dict[str, int]:
SUGGESTION_TYPE_ADD_QUESTION
]

# Constants defining various domain commands.
CMD_ADD_WRITTEN_TRANSLATION = 'add_written_translation'
CMD_EDIT_STATE_PROPERTY = 'edit_state_property'
CMD_CREATE_NEW_FULLY_SPECIFIED_QUESTION = 'create_new_fully_specified_question'

# Possible allowed commands for exploration change.
LIST_CMD_EXPLORATION_CHANGE = [
CMD_ADD_WRITTEN_TRANSLATION,
CMD_EDIT_STATE_PROPERTY
]

# Possible allowed commands for question change.
LIST_CMD_QUESTION_CHANGE = [
CMD_CREATE_NEW_FULLY_SPECIFIED_QUESTION
]
Copy link
Member

Choose a reason for hiding this comment

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

Why are we redefining these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above constants are to check which domain the passed change object's parameter cmd belongs,
The object value cmd is then compared with possibles commands in the list and then corresponding domain object is called accordingly.

 if obj['cmd'] in feconf.LIST_CMD_EXPLORATION_CHANGE:
        exp_domain.ExplorationChange(obj)

 elif obj['cmd'] in feconf.LIST_CMD_QUESTION_CHANGE:
        question_domain.QuestionSuggestionChange(obj)

https://github.com/chiragbaid7/oppia/blob/a18acf4a359546845c107093bb9change717f818a9d978/core/controllers/domain_objects_validator.py#L44

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but these commands are already defined in another file, we shouldn't redefine them, we should import them one way or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I have created a list of possible allowed commands in question_domain and exp_domain and removed the redefined constants from feconf.


# Prefix for all access validation handlers.
# The naming scheme for access validation handlers is
# '/access_validation_handler/<handler_name>'
Expand Down