codecov[bot]
commented
5 years ago Codecov Report
Merging #405 into master will decrease coverage by
0.28%. The diff coverage is100%.
@@ Coverage Diff @@
## master #405 +/- ##
==========================================
- Coverage 81.84% 81.55% -0.29%
==========================================
Files 53 53
Lines 2407 2424 +17
==========================================
+ Hits 1970 1977 +7
- Misses 437 447 +10| Impacted Files | Coverage Δ | |
|---|---|---|
| api/serializers.py | 97.71% <ø> (-0.04%) |
:arrow_down: |
| api/urls.py | 100% <ø> (ø) |
:arrow_up: |
| ui/serializers.py | 95.91% <100%> (+0.51%) |
:arrow_up: |
| ui/views.py | 100% <100%> (ø) |
:arrow_up: |
| api/views.py | 95.33% <100%> (+0.08%) |
:arrow_up: |
| ui/urls.py | 100% <100%> (ø) |
:arrow_up: |
| mediaplatform_jwp/api/delivery.py | 53.74% <100%> (-5.99%) |
:arrow_down: |
| legacysms/views.py | 100% <100%> (ø) |
:arrow_up: |
| mediaplatform_jwp/models.py | 98.27% <0%> (-1.73%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 6adf6a8...ff8e0fd. Read the comment docs.
msb
This PR is big. Sorry. I've spent some time trying to split it down into commits which can be understood individually. Generally there are individual commits which implement various bits of infrastructure and then a commit which makes use of it. If you review better seeing the use of components before looking at their implementation I recommend you review from the last commit back. If you review better by looking at components first and then seeing how they are put together, review from the first commit forward.
Note that GitHub is very poor at ordering commits properly. I recommend using local tooling.
This PR is so large because we change the way we handle embedding. Instead of using traditional Django templates, we render the embed view now like any other UI view. To this end the UI implements a specialised API which can be used to get configuration for the JWPlayer JavaScript API. This API is used by the frontend to configure the JWPlayer API manually rather than relying on JWPlatform to do it for us.
This gives us a nice degree of flexibility in how we structure the UI and lets us add our own playlist UI in the embed view.
Some time has been taken to perform JWPlayer integration "the React way" so that the various bits of the UI are decoupled from each other.
Since the media and playlist embed pages are so similar, a lot of the shared functionality has been broken out into generic components along with specialisations.
I've taken some care to write long commit messages for the trickier commits and tried to document what is going on through comments but ultimately some of this code is quite complex and may take a little time to get one's head around. I'm sorry for this but I don't think the problem can be solved in a simpler way without either a) copying and pasting a lot of code between the playlist and media embed pages or b) having to have everything JWPlayer related in a single "mega component".
If you are having trouble following what a component does, I suggest seeing how it is used in the rest of the code.
I've tested the playlist UI in Firefox on my machine and in IE11 on Windows 7 via browser stack.
It is also worth testing that the legacy embeds have not broken.
Closes #356. Closes #360.