search

mitodl / rapid-response-xblock

a django app plug-in for edx-platform
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

edX Koa release compatibility #87

Closed arslanashraf7 closed 3 years ago

arslanashraf7 commented 3 years ago

What are the relevant tickets?

NOTE: This PR is rebased on other dependent PR https://github.com/mitodl/rapid-response-xblock/pull/86 (This rebase was required to pass all the tests for events)

What's this PR do?

How should this be manually tested?

Majorly it should just be tested by having a look at the CI logs and making sure that the build passes all the checks. Since it's rebased on https://github.com/mitodl/rapid-response-xblock/pull/86 and if that PR is already tested with local rapid response integration with edx-platform we should be good to go on this part.

Where should the reviewer start?

Any background context you want to provide?

Tests were failing on this repo over the time when edx probably made the transition to juniper. This PR makes the required refactoring w.r.t edx-platform Koa and gets the tests to pass.

codecov-io commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@dbc6bfb). Click here to learn what that means. The diff coverage is 89.39%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #87   +/-   ##
=========================================
  Coverage          ?   90.77%           
=========================================
  Files             ?       16           
  Lines             ?      607           
  Branches          ?       42           
=========================================
  Hits              ?      551           
  Misses            ?       51           
  Partials          ?        5
Impacted Files Coverage Δ
rapid_response_xblock/apps.py 100.00% <ø> (ø)
rapid_response_xblock/migrations/0001_initial.py 0.00% <ø> (ø)
...id_response_xblock/migrations/0002_block_status.py 0.00% <ø> (ø)
...d_response_xblock/migrations/0003_rename_fields.py 0.00% <0.00%> (ø)
rapid_response_xblock/migrations/0004_run.py 0.00% <ø> (ø)
...response_xblock/migrations/0005_remove_run_name.py 0.00% <0.00%> (ø)
rapid_response_xblock/models.py 92.30% <ø> (ø)
setup.py 0.00% <ø> (ø)
rapid_response_xblock/logger.py 94.73% <71.42%> (ø)
rapid_response_xblock/block.py 99.12% <100.00%> (ø)
... and 5 more

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 dbc6bfb...0b47c8b. Read the comment docs.

arslanashraf7 commented 3 years ago

@pdpinch @noisecapella This is the PR for all the required refactoring for the tests. Checking out mitodl/mitx/koa also fixed the pylint issue.

Please note that it is rebased on https://github.com/mitodl/rapid-response-xblock/pull/86 (Bugfix PR) to pass all the tests.

gsidebo commented 3 years ago

@arslanashraf7 I'm actually seeing that rapid response works end-to-end with this PR, but it doesn't work with the other PR (https://github.com/mitodl/rapid-response-xblock/pull/86). In light of that, I think it might be better to close that other PR in favor of this one and squash the commits. Let me know if you see any issues with that

arslanashraf7 commented 3 years ago

@gsidebo This approach totally looks okay to me since we created that PR just to address the bug the we were facing with koa and seeing the priority to fix that, And this PR was created saperately since we didn't know how much it will take to fix all the test and refactoring, Also this PR is rebased on that one so technically it correct that this PR is working end-to-end with tests passing as well. We can close that PR if these changes looks good to you and we decide to merge this PR, We can squash the commits during the merge.

@pdpinch what are your thoughts on this?