From fc9a6249bbb2dc77d3b7cb13459ec7834cf1f849 Mon Sep 17 00:00:00 2001 From: Wasif ur Rehman Date: Mon, 5 Oct 2020 11:18:48 +0500 Subject: [PATCH 1/3] MCKIN-24468 McKA - Update Scorm xblock to Python 3.5 --- .travis.yml | 11 + Makefile | 5 + manage.py | 2 + pylintrc | 782 +++++++++++++++++++++++++++++ requirements.txt | 10 + scormxblock/scorm_file_uploader.py | 5 +- scormxblock/scormxblock.py | 69 +-- scormxblock/settings.py | 3 + setup.py | 5 +- 9 files changed, 855 insertions(+), 37 deletions(-) create mode 100644 .travis.yml create mode 100644 pylintrc create mode 100644 requirements.txt diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..05454c8 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,11 @@ +language: python + +python: + - "2.7" + - "3.5" + - "3.8" +virtualenv: + system_site_packages: true + +script: + - make quality diff --git a/Makefile b/Makefile index 88a306a..77c5f44 100644 --- a/Makefile +++ b/Makefile @@ -35,3 +35,8 @@ dummy_translations: ## generate dummy translation (.po) files build_dummy_translations: dummy_translations compile_translations ## generate and compile dummy translation files validate_translations: build_dummy_translations detect_changed_source_translations ## validate translations + +quality: ## run quality checkers on the codebase + pycodestyle scormxblock --max-line-length=120 + pylint scormxblock + diff --git a/manage.py b/manage.py index a4151a8..d03e1aa 100644 --- a/manage.py +++ b/manage.py @@ -1,4 +1,6 @@ #!/usr/bin/env python +from __future__ import absolute_import + import os import sys diff --git a/pylintrc b/pylintrc new file mode 100644 index 0000000..43093ac --- /dev/null +++ b/pylintrc @@ -0,0 +1,782 @@ +[MASTER] + +# A comma-separated list of package or module names from where C extensions may +# be loaded. Extensions are loading into the active Python interpreter and may +# run arbitrary code +extension-pkg-whitelist= + +# Add files or directories to the blacklist. They should be base names, not +# paths. +ignore=scormxblock/migrations + +# Add files or directories matching the regex patterns to the blacklist. The +# regex matches against base names, not paths. +ignore-patterns= + +# Python code to execute, usually for sys.path manipulation such as +# pygtk.require(). +#init-hook= + +# Use multiple processes to speed up Pylint. +jobs=1 + +# List of plugins (as comma separated values of python modules names) to load, +# usually to register additional checkers. +load-plugins= + +# Pickle collected data for later comparisons. +persistent=yes + +# Specify a configuration file. +#rcfile= + +# When enabled, pylint would attempt to guess common misconfiguration and emit +# user-friendly hints instead of false-positive error messages +suggestion-mode=yes + +# Allow loading of arbitrary C extensions. Extensions are imported into the +# active Python interpreter and may run arbitrary code. +unsafe-load-any-extension=no + + +[MESSAGES CONTROL] + +# Only show warnings with the listed confidence levels. Leave empty to show +# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED +confidence= + +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifiers separated by comma (,) or put this +# option multiple times (only on the command line, not in the configuration +# file where it should appear only once).You can also use "--disable=all" to +# disable everything first and then reenable specific checks. For example, if +# you want to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use"--disable=all --enable=classes +# --disable=W" +disable=blacklisted-name, + invalid-name, + missing-docstring, + empty-docstring, + unneeded-not, + singleton-comparison, + misplaced-comparison-constant, + unidiomatic-typecheck, + consider-using-enumerate, + consider-iterating-dictionary, + bad-classmethod-argument, + bad-mcs-method-argument, + bad-mcs-classmethod-argument, + single-string-used-for-slots, + line-too-long, + too-many-lines, + trailing-whitespace, + missing-final-newline, + trailing-newlines, + multiple-statements, + superfluous-parens, + bad-whitespace, + mixed-line-endings, + unexpected-line-ending-format, + bad-continuation, + wrong-spelling-in-comment, + wrong-spelling-in-docstring, + invalid-characters-in-docstring, + multiple-imports, + wrong-import-order, + ungrouped-imports, + wrong-import-position, + old-style-class, + len-as-condition, + syntax-error, + unrecognized-inline-option, + bad-option-value, + init-is-generator, + return-in-init, + not-in-loop, + return-outside-function, + yield-outside-function, + return-arg-in-generator, + nonexistent-operator, + duplicate-argument-name, + abstract-class-instantiated, + bad-reversed-sequence, + too-many-star-expressions, + invalid-star-assignment-target, + star-needs-assignment-target, + nonlocal-and-global, + continue-in-finally, + nonlocal-without-binding, + used-prior-global-declaration, + method-hidden, + access-member-before-definition, + no-method-argument, + no-self-argument, + invalid-slots-object, + assigning-non-slot, + invalid-slots, + inherit-non-class, + inconsistent-mro, + duplicate-bases, + non-iterator-returned, + unexpected-special-method-signature, + invalid-length-returned, + import-error, + relative-beyond-top-level, + used-before-assignment, + undefined-all-variable, + invalid-all-object, + no-name-in-module, + unbalanced-tuple-unpacking, + unpacking-non-sequence, + bad-except-order, + raising-bad-type, + bad-exception-context, + misplaced-bare-raise, + raising-non-exception, + notimplemented-raised, + catching-non-exception, + slots-on-old-class, + super-on-old-class, + bad-super-call, + missing-super-argument, + no-member, + not-callable, + assignment-from-no-return, + no-value-for-parameter, + too-many-function-args, + unexpected-keyword-arg, + redundant-keyword-arg, + missing-kwoa, + invalid-sequence-index, + invalid-slice-index, + assignment-from-none, + not-context-manager, + invalid-unary-operand-type, + unsupported-binary-operation, + repeated-keyword, + not-an-iterable, + not-a-mapping, + unsupported-membership-test, + unsubscriptable-object, + unsupported-assignment-operation, + unsupported-delete-operation, + invalid-metaclass, + logging-unsupported-format, + logging-format-truncated, + logging-too-many-args, + logging-too-few-args, + bad-format-character, + truncated-format-string, + mixed-format-string, + format-needs-mapping, + missing-format-string-key, + too-many-format-args, + too-few-format-args, + bad-str-strip-call, + print-statement, + parameter-unpacking, + unpacking-in-except, + old-raise-syntax, + backtick, + long-suffix, + old-ne-operator, + old-octal-literal, + import-star-module-level, + non-ascii-bytes-literal, + invalid-unicode-literal, + yield-inside-async-function, + not-async-context-manager, + fatal, + astroid-error, + parse-error, + method-check-failed, + raw-checker-failed, + bad-inline-option, + locally-disabled, + locally-enabled, + file-ignored, + suppressed-message, + useless-suppression, + deprecated-pragma, + c-extension-no-member, + literal-comparison, + no-self-use, + no-classmethod-decorator, + no-staticmethod-decorator, + cyclic-import, + duplicate-code, + too-many-ancestors, + too-many-instance-attributes, + too-few-public-methods, + too-many-public-methods, + too-many-return-statements, + too-many-branches, + too-many-arguments, + too-many-locals, + too-many-statements, + too-many-boolean-expressions, + consider-merging-isinstance, + too-many-nested-blocks, + simplifiable-if-statement, + redefined-argument-from-local, + no-else-return, + consider-using-ternary, + trailing-comma-tuple, + stop-iteration-return, + simplify-boolean-expression, + inconsistent-return-statements, + unreachable, + dangerous-default-value, + pointless-statement, + pointless-string-statement, + expression-not-assigned, + unnecessary-pass, + unnecessary-lambda, + duplicate-key, + deprecated-lambda, + assign-to-new-keyword, + useless-else-on-loop, + exec-used, + eval-used, + confusing-with-statement, + using-constant-test, + lost-exception, + assert-on-tuple, + attribute-defined-outside-init, + bad-staticmethod-argument, + protected-access, + arguments-differ, + signature-differs, + abstract-method, + super-init-not-called, + no-init, + non-parent-init-called, + useless-super-delegation, + unnecessary-semicolon, + bad-indentation, + mixed-indentation, + lowercase-l-suffix, + wildcard-import, + deprecated-module, + relative-import, + reimported, + import-self, + misplaced-future, + misplaced-futurefixme, + invalid-encoded-data, + global-variable-undefined, + global-variable-not-assigned, + global-statement, + global-at-module-level, + unused-argument, + unused-wildcard-import, + redefined-outer-name, + redefined-builtin, + redefine-in-handler, + undefined-loop-variable, + cell-var-from-loop, + bare-except, + broad-except, + duplicate-except, + nonstandard-exception, + binary-op-exception, + raising-format-tuple, + property-on-old-class, + keyword-arg-before-vararg, + logging-not-lazy, + logging-format-interpolation, + bad-format-string-key, + unused-format-string-key, + bad-format-string, + missing-format-argument-key, + unused-format-string-argument, + format-combined-specification, + missing-format-attribute, + invalid-format-index, + anomalous-backslash-in-string, + anomalous-unicode-escape-in-string, + bad-open-mode, + boolean-datetime, + redundant-unittest-assert, + deprecated-method, + bad-thread-instantiation, + shallow-copy-environ, + apply-builtin, + basestring-builtin, + buffer-builtin, + cmp-builtin, + coerce-builtin, + execfile-builtin, + file-builtin, + long-builtin, + raw_input-builtin, + reduce-builtin, + standarderror-builtin, + unicode-builtin, + xrange-builtin, + coerce-method, + delslice-method, + getslice-method, + setslice-method, + no-absolute-import, + old-division, + dict-iter-method, + dict-view-method, + next-method-called, + metaclass-assignment, + indexing-exception, + raising-string, + reload-builtin, + oct-method, + hex-method, + nonzero-method, + cmp-method, + input-builtin, + round-builtin, + intern-builtin, + unichr-builtin, + map-builtin-not-iterating, + zip-builtin-not-iterating, + range-builtin-not-iterating, + filter-builtin-not-iterating, + using-cmp-argument, + eq-without-hash, + div-method, + idiv-method, + rdiv-method, + exception-message-attribute, + invalid-str-codec, + sys-max-int, + bad-python3-import, + deprecated-string-function, + deprecated-str-translate-call, + deprecated-itertools-function, + deprecated-types-field, + next-method-defined, + dict-items-not-iterating, + dict-keys-not-iterating, + dict-values-not-iterating, + deprecated-operator-function, + deprecated-urllib-function, + xreadlines-attribute, + deprecated-sys-function, + exception-escape, + comprehension-escape, + import-outside-toplevel + +# Enable the message, report, category or checker with the given id(s). You can +# either give multiple identifier separated by comma (,) or put this option +# multiple time (only on the command line, not in the configuration file where +# it should appear only once). See also the "--disable" option for examples. +enable=function-redefined, + undefined-variable, + unused-import, + unused-variable + + +[REPORTS] + +# Python expression which should return a note less than 10 (10 is the highest +# note). You have access to the variables errors warning, statement which +# respectively contain the number of errors / warnings messages and the total +# number of statements analyzed. This is used by the global evaluation report +# (RP0004). +evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) + +# Template used to display messages. This is a python new-style format string +# used to format the message information. See doc for all details +#msg-template= + +# Set the output format. Available formats are text, parseable, colorized, json +# and msvs (visual studio).You can also give a reporter class, eg +# mypackage.mymodule.MyReporterClass. +output-format=text + +# Tells whether to display a full report or only the messages +reports=no + +# Activate the evaluation score. +score=yes + + +[REFACTORING] + +# Maximum number of nested blocks for function / method body +max-nested-blocks=5 + +# Complete name of functions that never returns. When checking for +# inconsistent-return-statements if a never returning function is called then +# it will be considered as an explicit return statement and no message will be +# printed. +never-returning-functions=optparse.Values,sys.exit + + +[LOGGING] + +# Logging modules to check that the string format arguments are in logging +# function parameter format +logging-modules=logging + + +[SPELLING] + +# Limits count of emitted suggestions for spelling mistakes +max-spelling-suggestions=4 + +# Spelling dictionary name. Available dictionaries: none. To make it working +# install python-enchant package. +spelling-dict= + +# List of comma separated words that should not be checked. +spelling-ignore-words= + +# A path to a file that contains private dictionary; one word per line. +spelling-private-dict-file= + +# Tells whether to store unknown words to indicated private dictionary in +# --spelling-private-dict-file option instead of raising a message. +spelling-store-unknown-words=no + + +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +notes=FIXME, + XXX, + TODO + + +[SIMILARITIES] + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + +# Ignore imports when computing similarities. +ignore-imports=no + +# Minimum lines number of a similarity. +min-similarity-lines=4 + + +[TYPECHECK] + +# List of decorators that produce context managers, such as +# contextlib.contextmanager. Add to this list to register other decorators that +# produce valid context managers. +contextmanager-decorators=contextlib.contextmanager + +# List of members which are set dynamically and missed by pylint inference +# system, and so shouldn't trigger E1101 when accessed. Python regular +# expressions are accepted. +generated-members= + +# Tells whether missing members accessed in mixin class should be ignored. A +# mixin class is detected if its name ends with "mixin" (case insensitive). +ignore-mixin-members=yes + +# This flag controls whether pylint should warn about no-member and similar +# checks whenever an opaque object is returned when inferring. The inference +# can return multiple potential results while evaluating a Python object, but +# some branches might not be evaluated, which results in partial inference. In +# that case, it might be useful to still emit no-member and other checks for +# the rest of the inferred objects. +ignore-on-opaque-inference=yes + +# List of class names for which member attributes should not be checked (useful +# for classes with dynamically set attributes). This supports the use of +# qualified names. +ignored-classes=optparse.Values,thread._local,_thread._local + +# List of module names for which member attributes should not be checked +# (useful for modules/projects where namespaces are manipulated during runtime +# and thus existing member attributes cannot be deduced by static analysis. It +# supports qualified module names, as well as Unix pattern matching. +ignored-modules= + +# Show a hint with possible names when a member name was not found. The aspect +# of finding the hint is based on edit distance. +missing-member-hint=yes + +# The minimum edit distance a name should have in order to be considered a +# similar match for a missing member name. +missing-member-hint-distance=1 + +# The total number of similar names that should be taken in consideration when +# showing a hint for a missing member. +missing-member-max-choices=1 + + +[VARIABLES] + +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= + +# Tells whether unused global variables should be treated as a violation. +allow-global-unused-variables=yes + +# List of strings which can identify a callback function by name. A callback +# name must start or end with one of those strings. +callbacks=cb_, + _cb + +# A regular expression matching the name of dummy variables (i.e. expectedly +# not used). +dummy-variables-rgx=_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)|dummy|^ignored_|^unused_ + +# Argument names that match this expression will be ignored. Default to name +# with leading underscore +ignored-argument-names=_.*|^ignored_|^unused_ + +# Tells whether we should check for unused import in __init__ files. +init-import=no + +# List of qualified module names which can have objects that can redefine +# builtins. +redefining-builtins-modules=six.moves,past.builtins,future.builtins,io,builtins + + +[FORMAT] + +# Expected format of line ending, e.g. empty (any line ending), LF or CRLF. +expected-line-ending-format= + +# Regexp for a line that is allowed to be longer than the limit. +ignore-long-lines=^\s*(# )??$ + +# Number of spaces of indent required inside a hanging or continued line. +indent-after-paren=4 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +indent-string=' ' + +# Maximum number of characters on a single line. +max-line-length=120 + +# Maximum number of lines in a module +max-module-lines=1000 + +# List of optional constructs for which whitespace checking is disabled. `dict- +# separator` is used to allow tabulation in dicts, etc.: {1 : 1,\n222: 2}. +# `trailing-comma` allows a space between comma and closing bracket: (a, ). +# `empty-line` allows space-only lines. +no-space-check=trailing-comma, + dict-separator + +# Allow the body of a class to be on the same line as the declaration if body +# contains single statement. +single-line-class-stmt=no + +# Allow the body of an if to be on the same line as the test if there is no +# else. +single-line-if-stmt=no + + +[BASIC] + +# Naming style matching correct argument names +argument-naming-style=snake_case + +# Regular expression matching correct argument names. Overrides argument- +# naming-style +#argument-rgx= + +# Naming style matching correct attribute names +attr-naming-style=snake_case + +# Regular expression matching correct attribute names. Overrides attr-naming- +# style +#attr-rgx= + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo, + bar, + baz, + toto, + tutu, + tata + +# Naming style matching correct class attribute names +class-attribute-naming-style=any + +# Regular expression matching correct class attribute names. Overrides class- +# attribute-naming-style +#class-attribute-rgx= + +# Naming style matching correct class names +class-naming-style=PascalCase + +# Regular expression matching correct class names. Overrides class-naming-style +#class-rgx= + +# Naming style matching correct constant names +const-naming-style=UPPER_CASE + +# Regular expression matching correct constant names. Overrides const-naming- +# style +#const-rgx= + +# Minimum line length for functions/classes that require docstrings, shorter +# ones are exempt. +docstring-min-length=-1 + +# Naming style matching correct function names +function-naming-style=snake_case + +# Regular expression matching correct function names. Overrides function- +# naming-style +#function-rgx= + +# Good variable names which should always be accepted, separated by a comma +good-names=i, + j, + k, + ex, + Run, + _ + +# Include a hint for the correct naming format with invalid-name +include-naming-hint=no + +# Naming style matching correct inline iteration names +inlinevar-naming-style=any + +# Regular expression matching correct inline iteration names. Overrides +# inlinevar-naming-style +#inlinevar-rgx= + +# Naming style matching correct method names +method-naming-style=snake_case + +# Regular expression matching correct method names. Overrides method-naming- +# style +#method-rgx= + +# Naming style matching correct module names +module-naming-style=snake_case + +# Regular expression matching correct module names. Overrides module-naming- +# style +#module-rgx= + +# Colon-delimited sets of names that determine each other's naming style when +# the name regexes allow several styles. +name-group= + +# Regular expression which should only match function or class names that do +# not require a docstring. +no-docstring-rgx=^_ + +# List of decorators that produce properties, such as abc.abstractproperty. Add +# to this list to register other decorators that produce valid properties. +property-classes=abc.abstractproperty + +# Naming style matching correct variable names +variable-naming-style=snake_case + +# Regular expression matching correct variable names. Overrides variable- +# naming-style +#variable-rgx= + + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=5 + +# Maximum number of attributes for a class (see R0902). +max-attributes=7 + +# Maximum number of boolean expressions in a if statement +max-bool-expr=5 + +# Maximum number of branch for function / method body +max-branches=12 + +# Maximum number of locals for function / method body +max-locals=15 + +# Maximum number of parents for a class (see R0901). +max-parents=7 + +# Maximum number of public methods for a class (see R0904). +max-public-methods=20 + +# Maximum number of return / yield for function / method body +max-returns=6 + +# Maximum number of statements in function / method body +max-statements=50 + +# Minimum number of public methods for a class (see R0903). +min-public-methods=2 + + +[CLASSES] + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__, + __new__, + setUp + +# List of member names, which should be excluded from the protected access +# warning. +exclude-protected=_asdict, + _fields, + _replace, + _source, + _make + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + +# List of valid names for the first argument in a metaclass class method. +valid-metaclass-classmethod-first-arg=mcs + + +[IMPORTS] + +# Allow wildcard imports from modules that define __all__. +allow-wildcard-with-all=no + +# Analyse import fallback blocks. This can be used to support both Python 2 and +# 3 compatible code, which means that the block might have code that exists +# only in one or another interpreter, leading to false positives when analysed. +analyse-fallback-blocks=no + +# Deprecated modules which should not be used, separated by a comma +deprecated-modules=regsub, + TERMIOS, + Bastion, + rexec + +# Create a graph of external dependencies in the given file (report RP0402 must +# not be disabled) +ext-import-graph= + +# Create a graph of every (i.e. internal and external) dependencies in the +# given file (report RP0402 must not be disabled) +import-graph= + +# Create a graph of internal dependencies in the given file (report RP0402 must +# not be disabled) +int-import-graph= + +# Force import order to recognize a module as part of the standard +# compatibility libraries. +known-standard-library= + +# Force import order to recognize a module as part of a third party library. +known-third-party=enchant + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..e78ac98 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,10 @@ +XBlock>=1.2 + +edx-opaque-keys>=0.4 +Django~=1.11; python_version == '2.7' +Django~=2.2; python_version > '2.7' +-e git+https://github.com/edx/xblock-utils.git@2.0.0#egg=xblock-utils +-e . + +pylint +pycodestyle==2.4.0 diff --git a/scormxblock/scorm_file_uploader.py b/scormxblock/scorm_file_uploader.py index aec1419..29b63a8 100644 --- a/scormxblock/scorm_file_uploader.py +++ b/scormxblock/scorm_file_uploader.py @@ -1,3 +1,4 @@ +from __future__ import absolute_import import os import tempfile import re @@ -117,7 +118,7 @@ def _save_to_storage(self, tempdir): logger.info('File `{}` stored.'.format(file_relative_path)) uploaded_size += file_to_store['size'] self._set_upload_progress(uploaded_size, total_files_size) - except encoding.DjangoUnicodeDecodeError, e: + except encoding.DjangoUnicodeDecodeError as e: logger.warn('SCORM XBlock Couldn\'t store file {} to storage. {}'.format(file_to_store, e)) self._post_upload_cleanup(tempdir) @@ -151,7 +152,7 @@ def _files_to_store(self, tempdir): files_to_store = [] total_files_size = 0 - for (dirpath, dirnames, files) in os.walk(tempdir): + for (dirpath, _, files) in os.walk(tempdir): for f in files: file_path = os.path.join(os.path.abspath(dirpath), f) size = os.path.getsize(file_path) diff --git a/scormxblock/scormxblock.py b/scormxblock/scormxblock.py index b8e1481..70fe270 100644 --- a/scormxblock/scormxblock.py +++ b/scormxblock/scormxblock.py @@ -1,29 +1,31 @@ +from __future__ import absolute_import +import encodings import json -import os -import pkg_resources import logging -import encodings import mimetypes -import pytz +import os +from datetime import datetime +import pkg_resources +import pytz +import six from django.conf import settings from django.core.files.storage import default_storage from django.http import QueryDict +from mako.template import Template as MakoTemplate +from six.moves import range from webob import Response -from datetime import datetime - from xblock.core import XBlock -from xblock.fields import Scope, String, Integer, Boolean, Float, DateTime +from xblock.fields import Boolean, DateTime, Float, Integer, Scope, String from xblock.fragment import Fragment -from openedx.core.lib.xblock_utils import add_staff_markup from microsite_configuration import microsite +from openedx.core.lib.xblock_utils import add_staff_markup from util.date_utils import get_default_time_display -from mako.template import Template as MakoTemplate - -from .scorm_file_uploader import ScormPackageUploader, STATE as UPLOAD_STATE -import constants +from . import constants +from .scorm_file_uploader import STATE as UPLOAD_STATE +from .scorm_file_uploader import ScormPackageUploader # Make '_' a no-op so we can scrape strings _ = lambda text: text @@ -110,33 +112,33 @@ class ScormXBlock(XBlock): scope=Scope.settings, ) display_type = String( - display_name =_("Display Type"), + display_name=_("Display Type"), values=["iframe", "popup"], default="iframe", help=_("Open in a new popup window, or an iframe. This setting may be overridden by player-specific configuration."), scope=Scope.settings ) popup_launch_type = String( - display_name =_("Popup Launch Type"), + display_name=_("Popup Launch Type"), values=["auto", "manual"], default="auto", help=_("Open in a new popup through button or automatically."), scope=Scope.settings ) launch_button_text = String( - display_name =_("Launch Button Text"), + display_name=_("Launch Button Text"), help=_("Display text for Launch Button"), default="Launch", scope=Scope.settings ) display_width = Integer( - display_name =_("Display Width (px)"), + display_name=_("Display Width (px)"), help=_('Width of iframe or popup window'), default=820, scope=Scope.settings ) display_height = Integer( - display_name =_("Display Height (px)"), + display_name=_("Display Height (px)"), help=_('Height of iframe or popup window'), default=450, scope=Scope.settings @@ -149,13 +151,13 @@ class ScormXBlock(XBlock): scope=Scope.settings ) player_configuration = String( - display_name =_("Player Configuration"), + display_name=_("Player Configuration"), default='', help=_("JSON object string with overrides to be passed to selected SCORM player. These will be exposed as data attributes on the host iframe and sent in a window.postMessage to the iframe's content window. Attributes can be any. 'Internal player' will always check this field for an 'initial_html' attribute to override index.html as the initial page."), scope=Scope.settings ) scorm_file_name = String( - display_name =_("Scorm File Name"), + display_name=_("Scorm File Name"), help=_("Scorm Package Uploaded File Name"), default="", scope=Scope.settings @@ -164,6 +166,7 @@ class ScormXBlock(XBlock): default=None, scope=Scope.settings, help="Scorm File Last Uploaded Date" ) + @property def student_id(self): if hasattr(self, "scope_ids"): @@ -198,7 +201,7 @@ def _serialize_opaque_key(self, key): if hasattr(key, 'to_deprecated_string'): return key.to_deprecated_string() else: - return unicode(key) + return six.text_type(key) def resource_string(self, path): """Handy helper for getting resources from our kit.""" @@ -243,9 +246,9 @@ def student_view(self, context=None, authoring=False): get_url = set_url = get_completion_url = '#' # if display type is popup, don't use the full window width for the host iframe - iframe_width = self.display_type=='popup' and DEFAULT_IFRAME_WIDTH or self.display_width; - iframe_height = self.display_type=='popup' and DEFAULT_IFRAME_HEIGHT or self.display_height; - show_popup_manually = True if self.display_type=='popup' and self.popup_launch_type=='manual' else False; + iframe_width = self.display_type == 'popup' and DEFAULT_IFRAME_WIDTH or self.display_width + iframe_height = self.display_type == 'popup' and DEFAULT_IFRAME_HEIGHT or self.display_height + show_popup_manually = True if self.display_type == 'popup' and self.popup_launch_type == 'manual' else False lock_next_module = self.is_next_module_locked and self.scorm_progress < constants.MAX_PROGRESS_VALUE try: player_config = json.loads(self.player_configuration) @@ -255,13 +258,13 @@ def student_view(self, context=None, authoring=False): frag = Fragment() frag.add_content(MakoTemplate(text=html.format(self=self, scorm_player_url=scorm_player_url, get_url=get_url, set_url=set_url, - get_completion_url = get_completion_url, + get_completion_url=get_completion_url, iframe_width=iframe_width, iframe_height=iframe_height, player_config=player_config, show_popup_manually=show_popup_manually, scorm_file=course_directory, is_next_module_locked=lock_next_module) - ).render_unicode()) + ).render_unicode()) frag.add_css(self.resource_string("static/css/scormxblock.css")) context['block_id'] = self.url_name @@ -269,7 +272,6 @@ def student_view(self, context=None, authoring=False): jsfrag = MakoTemplate(js).render_unicode(**context) frag.add_javascript(jsfrag) - # TODO: this will only work to display staff debug info if 'scormxblock' is one of the # categories of blocks that are specified in lms/templates/staff_problem_info.html so this will # for now have to be overridden in theme or directly in edx-platform @@ -299,7 +301,7 @@ def studio_view(self, context=None): html = self.resource_string("static/html/studio.html") frag = Fragment() file_uploaded_date = get_default_time_display(self.file_uploaded_date) if self.file_uploaded_date else '' - context = {'block': self, 'file_uploaded_date':file_uploaded_date} + context = {'block': self, 'file_uploaded_date': file_uploaded_date} frag.add_content(MakoTemplate(text=html).render_unicode(**context)) frag.add_css(self.resource_string("static/css/scormxblock.css")) frag.add_javascript(self.resource_string("static/js/src/studio.js")) @@ -369,7 +371,7 @@ def studio_submit(self, request, suffix=''): try: json.loads(request.params['player_configuration']) # just validation self.player_configuration = request.params['player_configuration'] - except ValueError, e: + except ValueError as e: return Response(json.dumps({'result': 'failure', 'error': 'Invalid JSON in Player Configuration'.format(e)}), content_type='application/json') return Response(json.dumps({'result': 'success'}), content_type='application/json') @@ -392,7 +394,7 @@ def scorm_set_value(self, data, suffix=''): self._publish_grade() context.update({"lesson_score": self.lesson_score}) if name == 'cmi.core.score.raw': - self._set_lesson_score(data.get('value',0)) + self._set_lesson_score(data.get('value', 0)) return context def _get_all_scos(self): @@ -477,7 +479,7 @@ def get_scorm_completion(self, request, suffix=''): completion = {'completion': self.scorm_progress or 0} return Response( json.dumps(completion), - content_type = 'application/json' + content_type='application/json' ) @XBlock.handler @@ -494,7 +496,7 @@ def proxy_content(self, request, suffix=''): if ext in mimetypes.types_map: content_type = mimetypes.types_map[ext] else: - return Response('Did not exist in storage: ' + path_to_file , status=404, content_type='text/html', charset='UTF-8') + return Response('Did not exist in storage: ' + path_to_file, status=404, content_type='text/html', charset='UTF-8') return Response(contents, content_type=content_type) def generate_report_data(self, user_state_iterator, limit_responses=None): @@ -571,11 +573,10 @@ def _set_lesson_score(self, scos): for sco in scos.keys(): sco = scos[sco]['data'] total_score += int(self._get_value_from_sco(sco, 'cmi.core.score.raw', 0)) - score_rollup = float(total_score) / float(len(scos.keys())) + score_rollup = float(total_score) / float(len(list(scos.keys()))) self.lesson_score = score_rollup return score_rollup - def _publish_grade(self, status, score): """ publish the grade in the LMS. @@ -597,7 +598,7 @@ def _publish_grade(self, status, score): self, 'grade', { - 'value': (float(score)/float(DEFAULT_SCO_MAX_SCORE)) * self.weight, + 'value': (float(score) / float(DEFAULT_SCO_MAX_SCORE)) * self.weight, 'max_value': self.weight, }) diff --git a/scormxblock/settings.py b/scormxblock/settings.py index 7ae83fc..cafbae8 100644 --- a/scormxblock/settings.py +++ b/scormxblock/settings.py @@ -7,7 +7,10 @@ """ # Build paths inside the project like this: os.path.join(BASE_DIR, ...) +from __future__ import absolute_import + import os + import yaml BASE_DIR = os.path.dirname(os.path.dirname(__file__)) diff --git a/setup.py b/setup.py index 39fab7f..f97f3fd 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,9 @@ """Setup for scormxblock XBlock.""" +from __future__ import absolute_import + import os + from setuptools import setup @@ -22,7 +25,7 @@ def package_data(pkg, roots): setup( name='scormxblock-xblock', - version='2.3', + version='3.0', description='XBlock to integrate SCORM content packages', packages=[ 'scormxblock', From 12ffc948e22d0345985cbcf0c7cbd00dba321cde Mon Sep 17 00:00:00 2001 From: Wasif ur Rehman Date: Mon, 5 Oct 2020 11:54:27 +0500 Subject: [PATCH 2/3] Fix pylint and pycode suggestions --- .travis.yml | 2 +- openedx.yaml | 5 +++++ pylintrc | 10 ++------- scormxblock/scorm_file_uploader.py | 21 +++++++++-------- scormxblock/scormxblock.py | 36 +++++++++++++++++++++--------- 5 files changed, 44 insertions(+), 30 deletions(-) create mode 100644 openedx.yaml diff --git a/.travis.yml b/.travis.yml index 05454c8..96e66bd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ language: python python: - "2.7" - "3.5" - - "3.8" + virtualenv: system_site_packages: true diff --git a/openedx.yaml b/openedx.yaml new file mode 100644 index 0000000..085305d --- /dev/null +++ b/openedx.yaml @@ -0,0 +1,5 @@ +# This file describes this Open edX repo, as described in OEP-7: + +oeps: + oep-7: true + diff --git a/pylintrc b/pylintrc index 43093ac..5ef7fb0 100644 --- a/pylintrc +++ b/pylintrc @@ -263,6 +263,8 @@ disable=blacklisted-name, reimported, import-self, misplaced-future, + fixme, + try-except-raise, misplaced-futurefixme, invalid-encoded-data, global-variable-undefined, @@ -439,14 +441,6 @@ spelling-private-dict-file= spelling-store-unknown-words=no -[MISCELLANEOUS] - -# List of note tags to take in consideration, separated by a comma. -notes=FIXME, - XXX, - TODO - - [SIMILARITIES] # Ignore comments when computing similarities. diff --git a/scormxblock/scorm_file_uploader.py b/scormxblock/scorm_file_uploader.py index 29b63a8..529e3c3 100644 --- a/scormxblock/scorm_file_uploader.py +++ b/scormxblock/scorm_file_uploader.py @@ -1,17 +1,16 @@ from __future__ import absolute_import + +import logging import os -import tempfile import re -import zipfile import shutil -import logging +import tempfile +import zipfile from django.conf import settings -from django.core.files.storage import default_storage -from django.core.files.storage import get_storage_class -from django.utils import encoding from django.core.cache import cache - +from django.core.files.storage import default_storage, get_storage_class +from django.utils import encoding logger = logging.getLogger(__name__) @@ -23,13 +22,13 @@ PROGRESS_CACHE_EXPIRY = 1 * 60 * 60 # 1 Hour -class FileAccessMode(object): +class FileAccessMode: WRITE = "wb+" APPEND = "ab+" READ_WRITE = "rb+" -class STATE(object): +class STATE: """ enum for upload state """ @@ -37,7 +36,7 @@ class STATE(object): COMPLETE = 'complete' -class ScormPackageUploader(object): +class ScormPackageUploader: """ Handles scorm package uploading """ @@ -145,7 +144,7 @@ def _cleanup_storage_dir(self, storage): try: for key in storage.bucket.list(prefix=self.scorm_storage_location): key.delete() - except AttributeError: + except AttributeError: # pylint: disable=try-except-raise raise def _files_to_store(self, tempdir): diff --git a/scormxblock/scormxblock.py b/scormxblock/scormxblock.py index 70fe270..955826a 100644 --- a/scormxblock/scormxblock.py +++ b/scormxblock/scormxblock.py @@ -1,4 +1,5 @@ from __future__ import absolute_import + import encodings import json import logging @@ -27,8 +28,11 @@ from .scorm_file_uploader import STATE as UPLOAD_STATE from .scorm_file_uploader import ScormPackageUploader + # Make '_' a no-op so we can scrape strings -_ = lambda text: text +def _(text): + return text + logger = logging.getLogger(__name__) @@ -69,7 +73,8 @@ class ScormXBlock(XBlock): scope=Scope.settings ) scorm_player = String( - values=[{"value": key, "display_name": DEFINED_PLAYERS[key]['name']} for key in DEFINED_PLAYERS.keys()] + [SCORM_PKG_INTERNAL, ], + values=[{"value": key, "display_name": DEFINED_PLAYERS[key]['name']} for key in DEFINED_PLAYERS.keys()] + + [SCORM_PKG_INTERNAL, ], display_name=_("SCORM player"), help=_("SCORM player configured in Django settings, or index.html file contained in SCORM package"), scope=Scope.settings @@ -115,7 +120,8 @@ class ScormXBlock(XBlock): display_name=_("Display Type"), values=["iframe", "popup"], default="iframe", - help=_("Open in a new popup window, or an iframe. This setting may be overridden by player-specific configuration."), + help=_("Open in a new popup window, or an iframe. This setting may be overridden by " + "player-specific configuration."), scope=Scope.settings ) popup_launch_type = String( @@ -146,14 +152,19 @@ class ScormXBlock(XBlock): encoding = String( display_name=_("SCORM Package text encoding"), default='cp850', - help=_("Character set used in SCORM package. Defaults to cp850 (or IBM850), for Latin-1: Western European languages)"), + help=_("Character set used in SCORM package. Defaults to cp850 (or IBM850), " + "for Latin-1: Western European languages)"), values=[{"value": AVAIL_ENCODINGS[key], "display_name": key} for key in sorted(AVAIL_ENCODINGS.keys())], scope=Scope.settings ) player_configuration = String( display_name=_("Player Configuration"), default='', - help=_("JSON object string with overrides to be passed to selected SCORM player. These will be exposed as data attributes on the host iframe and sent in a window.postMessage to the iframe's content window. Attributes can be any. 'Internal player' will always check this field for an 'initial_html' attribute to override index.html as the initial page."), + help=_("JSON object string with overrides to be passed to selected SCORM player. " + "These will be exposed as data attributes on the host iframe and sent in a window.postMessage " + "to the iframe's content window. Attributes can be any. " + "'Internal player' will always check this field for an 'initial_html' attribute " + "to override index.html as the initial page."), scope=Scope.settings ) scorm_file_name = String( @@ -239,7 +250,8 @@ def student_view(self, context=None, authoring=False): if not authoring: get_url = '{}://{}{}'.format(scheme, lms_base, self.runtime.handler_url(self, "get_raw_scorm_status")) set_url = '{}://{}{}'.format(scheme, lms_base, self.runtime.handler_url(self, "set_raw_scorm_status")) - get_completion_url = '{}://{}{}'.format(scheme, lms_base, self.runtime.handler_url(self, "get_scorm_completion")) + get_completion_url = '{}://{}{}'.format(scheme, lms_base, + self.runtime.handler_url(self, "get_scorm_completion")) # PreviewModuleSystem (runtime Mixin from Studio) won't have a hostname else: # we don't want to get/set SCORM status from preview @@ -248,7 +260,7 @@ def student_view(self, context=None, authoring=False): # if display type is popup, don't use the full window width for the host iframe iframe_width = self.display_type == 'popup' and DEFAULT_IFRAME_WIDTH or self.display_width iframe_height = self.display_type == 'popup' and DEFAULT_IFRAME_HEIGHT or self.display_height - show_popup_manually = True if self.display_type == 'popup' and self.popup_launch_type == 'manual' else False + show_popup_manually = self.display_type == 'popup' and self.popup_launch_type == 'manual' lock_next_module = self.is_next_module_locked and self.scorm_progress < constants.MAX_PROGRESS_VALUE try: player_config = json.loads(self.player_configuration) @@ -289,7 +301,8 @@ def student_view(self, context=None, authoring=False): disable_staff_debug_info = settings.FEATURES.get('DISPLAY_DEBUG_INFO_TO_STAFF', True) and False or True block = self view = 'student_view' - frag = add_staff_markup(dj_user, has_instructor_access, disable_staff_debug_info, block, view, frag, context) + frag = add_staff_markup(dj_user, has_instructor_access, disable_staff_debug_info, + block, view, frag, context) frag.initialize_js('ScormXBlock_{0}'.format(context['block_id'])) return frag @@ -372,7 +385,9 @@ def studio_submit(self, request, suffix=''): json.loads(request.params['player_configuration']) # just validation self.player_configuration = request.params['player_configuration'] except ValueError as e: - return Response(json.dumps({'result': 'failure', 'error': 'Invalid JSON in Player Configuration'.format(e)}), content_type='application/json') + return Response(json.dumps({'result': 'failure', + 'error': 'Invalid JSON in Player Configuration'.format(e)}), + content_type='application/json') return Response(json.dumps({'result': 'success'}), content_type='application/json') @@ -496,7 +511,8 @@ def proxy_content(self, request, suffix=''): if ext in mimetypes.types_map: content_type = mimetypes.types_map[ext] else: - return Response('Did not exist in storage: ' + path_to_file, status=404, content_type='text/html', charset='UTF-8') + return Response('Did not exist in storage: ' + path_to_file, status=404, + content_type='text/html', charset='UTF-8') return Response(contents, content_type=content_type) def generate_report_data(self, user_state_iterator, limit_responses=None): From 577a2df2847292effd300acf49d6dc0e5d6a4d7c Mon Sep 17 00:00:00 2001 From: Wasif ur Rehman Date: Mon, 5 Oct 2020 23:57:51 +0500 Subject: [PATCH 3/3] Fix Travis and add requirements-dev.txt --- .travis.yml | 4 ++-- Makefile | 3 +++ requirements-dev.txt | 9 +++++++++ requirements.txt | 8 +------- 4 files changed, 15 insertions(+), 9 deletions(-) create mode 100644 requirements-dev.txt diff --git a/.travis.yml b/.travis.yml index 96e66bd..66b6859 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,8 +4,8 @@ python: - "2.7" - "3.5" -virtualenv: - system_site_packages: true +install: + - make requirements script: - make quality diff --git a/Makefile b/Makefile index 77c5f44..56eef22 100644 --- a/Makefile +++ b/Makefile @@ -40,3 +40,6 @@ quality: ## run quality checkers on the codebase pycodestyle scormxblock --max-line-length=120 pylint scormxblock +requirements: ## install development environment requirements + pip install -r requirements.txt --exists-action w + pip install -r requirements-dev.txt --exists-action w diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..fc549ff --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,9 @@ +XBlock>=1.2 + +edx-opaque-keys>=0.4 +Django~=1.11; python_version == '2.7' +Django~=2.2; python_version > '2.7' +-e git://github.com/edx/xblock-utils.git#egg=xblock-utils + +pylint +pycodestyle==2.4.0 diff --git a/requirements.txt b/requirements.txt index e78ac98..b7d082f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,10 +1,4 @@ -XBlock>=1.2 -edx-opaque-keys>=0.4 -Django~=1.11; python_version == '2.7' -Django~=2.2; python_version > '2.7' --e git+https://github.com/edx/xblock-utils.git@2.0.0#egg=xblock-utils +-e git://github.com/edx/xblock-utils.git#egg=xblock-utils -e . -pylint -pycodestyle==2.4.0