subssn21 commented 6 years ago

Here is my first attempt at handling non standard parameters.

In LaunchParams class I added a valid_params method. At the moment the default implementation just calls the module valid_param function. We could copy and paste the implementation of the valid_param function into the method and add a deprecation warning to the function if you want or we can just leave it as is.

At the moment I don't see a need to deprecate the original other than for cleanliness.

In the ToolProvider I just added a class attribute to define the LaunchParam class so that it can be overridden.

In the tests the create_tp function was hardcoded to use LaunchParams, I just changed that to use the LaunchParams from the ToolProvider Class.

I created a couple of custom test classes to test with

I added a new test that mimics the other tests but with custom parameters added

codecov[bot] commented 6 years ago

Codecov Report

Merging #55 into master will increase coverage by 0.07%. The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   83.64%   83.71%   +0.07%     
==========================================
  Files          16       16              
  Lines         648      651       +3     
  Branches      113      113              
==========================================
+ Hits          542      545       +3     
  Misses         78       78              
  Partials       28       28

Impacted Files	Coverage Δ
src/lti/tool_provider.py	`90% <100%> (+0.12%)`	:arrow_up:
src/lti/launch_params.py	`90.27% <80%> (+0.27%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 51e68a3...a5de20d. Read the comment docs.

ryanhiebert commented 6 years ago

Looks good. Feel free to merge when you're ready.

pylti / lti

Handling Non-Standard LTI Params #55

Codecov Report