Canvas app isn't production code quality

[seanh]

See also

• Canvas is an LMS (Learning Management System) used by schools to deliver courses. (Similar to Blackboard, Moodle, Sakai...)
• Canvas source code on GitHub
• The Hypothesis Canvas app:
  • Production deployment
  • Source code
  • Canvas app Google Docs from @judell:
    • Canvas Architecture Notes
    • Installing the App
    • Using the App
    • Student Guide
  • Screencasts from @jeremydean:
    • Student walkthrough screencast
    • Teacher walkthrough screencast
• Feb 2017 Canvas spike by Nick
• May 2017 Canvas Feature Spec Sheet (google doc)
• Canvas GitHub Issues label in the product-backlog repo, including:
  • Canvas app isn't on production infrastructure Canvas app isn't on production infrastructure #339
  • Canvas users have to manually create and activate Hypothesis accounts Canvas users have to manually create and activate Hypothesis accounts #342
  • Canvas users have to manually create and join Hypothesis groups Canvas users have to manually create and join Hypothesis groups #341
• #feat-canvas Slack channel
• Canvas folder on Google Drive (we should move all Canvas-related Google Docs into this folder)
• Our Instructure-hosted Canvas instance, we can use this to try out Canvas
• This issue is also tightly coupled with getting the Canvas app onto our production infrastructure.

Problem

There is a Canvas app for Hypothesis currently deployed at https://lti.hypothesislabs.com (git repo). I believe the server behind this is a Digital Ocean droplet run by @judell, it's not on the main Hypothesis infrastructure. This app is currently being used by more than 20 schools for classroom annotation assignments.

TODO: Insert short feature-list of app.

Although it's now in production use, the app was originally intended as a prototype and followed a minimalistic development strategy convenient for rapid iteration, deferring code organization decisions and expecting to be rewritten and hardened before becoming a standard Hypothesis product. For example the server code, HTML templates and JavaScript mostly all exist in a single file. The app:

• Isn't well known to or understood by the Hypothesis development team (it has so far been a personal project of @judell)
• Doesn't have any tests
• Lacks structure (it's mostly all code in one file)
• Doesn't follow the dev team's usual coding standards

In its current state this app can't be maintained by the dev team - we can't help if the app or server goes down, fix bugs in the app, make changes to it, or add new features to it.

Without adding any new features or changing the app from the user's point of view, the app needs to be brought to similar coding practices as other Hypothesis production software, such as h and bouncer, so that the dev team can maintain and extend the app in the future.

By having developers from the dev team do this work they'll also learn a lot about Canvas and the Canvas app, and so be in a good position to work on it in the future, or to be a resource for other developers who're working on it.

Solution

More detail can be added once we get started (and have had a good look through the code) but broadly speaking I think the strategy will be:

• Get development environments set up for the developers who're going to be working on this. This means getting a local Canvas dev install working and getting the Hypothesis Canvas app running locally against this Canvas dev install (and against dev installs of h, client, bouncer, via, and any other Hypothesis things that the Canvas app works with)

• If possible split the main lti.py file into at least two separate files to start with, to make it easier for two developers to change the code simultaneously.

• Add functional (i.e. end-to-end) tests so that we can refactor the code with some confidence that we aren't unintentionally breaking it (because our functional tests will still be passing). We should try to minimise the number of changes we need to make to the code to get this basic test harness into place.

• Get the tests running on Travis CI with GitHub integration

• Get Codecov integration for the git repo

• Get code linting (pylint or flake8 and eslint) in place

• Refactor and write unit tests for the code. The main aim will be to introduce clear modular structure to the code: small, simple, independent modules, in separate files, each with its own unit tests.

There'll likely be other issues that we notice as we work on the code that we should fix.

We should prioritise which parts of the code to concentrate on first. For example working on parts that seem crucial to the app's functionality first, and working on parts that we expect to want to change in the future (e.g. to add Canvas -> Hypothesis accounts integration and groups integration) first.

Other solutions we considered and rejected

1. Write a new Canvas app from scratch. Starting from a list of problems that we want to solve, design and implement solutions to those problems, using the existing Canvas app only as a reference.

   We rejected this approach because it would take too long. In particular, it won't be possible to match the existing Canvas app's feature set within a single six week "feature teams" delivery cycle. At the end of the delivery cycle the old Canvas app would probably remain in use in production, the new app lacking the full set of features needed to replace it, and the new app would be "dead code" until we get more time to finish it.

2. Copy the design of the Canvas app (from the user's point of view) but rewrite the code from scratch.

   We rejected this for the same reason as (1) above. If we choose to iterate on and improve the existing Canvas app, rather than rewriting it, then we avoid the problem of having dead (un-shipped) code at the end of the delivery cycle. However far we get with improving the existing app we should be able to ship that improved version.

[judell]

Sounds good, Sean, thank you. Note that while I did put up a local install of Canvas in the early going, and used it a few times to insert debugging statements into Canvas while working out the OAuth and assignment submission flows, it wasn't really necessary and I haven't needed to use it in a long time, I do all testing against an instance that Instructure provided for us at hypothesis.instructure.com, to which you'll have access.

[seanh]

Update: we've made good progress on this but I think we're still working on it. The remaining functions in app.py still need to be split out into separate files and to have unit tests written for them. The biggest of these is the lti_setup() function that needs to broken up into several separate functions. It looks to me like probably another week of solid work to finish this.