Nice form to use

[carsongee]

This adds a form to exercise the edx-platform form. It has a dependency on #11, so that is why it looks extra huge. Once that merges, this should be a bit easier to test:

Start form

Successful

Failed

[coveralls]

Coverage increased (+8.31%) to 97.0% when pulling a3936746d02a1a7e0495814c6045b478a9aec439 on feature/user_interface into f7aba5d on master.

[coveralls]

Coverage increased (+0.37%) to 97.0% when pulling b849e09eee5cbf70a373b946c4f35d15ff0eac66 on feature/user_interface into 0c70bf1 on master.

[coveralls]

Coverage increased (+0.37%) to 97.0% when pulling b849e09eee5cbf70a373b946c4f35d15ff0eac66 on feature/user_interface into 0c70bf1 on master.

[carsongee]

Rebased and ready for review

[pwilkins]

When I click the get sections button I get this error report. Apart from getting the error, the error report message scrolls off the right margin and clicking the Traceback button displays white text on a light grey background.

[carsongee]

Here is the relevant github issue for the underlying bug in pylmod: mitodl/PyLmod#30. As discussed the unfortunately ugly error message is due to having flask in debug mode. If you want to try it with DEBUG=False in the settings (i.e. through uwsgi or gunicorn) and it is still a mess I could definitely fix it up.

[carsongee]

As another comment to @pdpinch suggestion to remove get-sections, I'm not sure I really want to do that with some more thought. I don't know who/what else maybe consuming that API feature since we are basically black box redesigning grades2stellar. So I think we probably want to leave it in there and fix pylmod.

[pdpinch]

Fair point. Does mitodl/PyLmod#30 block this merge? Does it block a release of lmod_proxy?

IMO, no on 1, assuming we add an issue to track it. Yes on 2, for the same reason you outline @carsongee

[bdero]

Any need for these console.logs? It seems that the convenience of network tools in modern browsers should eliminate the need.

[carsongee]

I was logging them for introspection of the object I was getting back (which wasn't what I thought it was at first), not for the actual data coming back. I'll remove them since they are now the expected object and you are correct on the network tools

[bdero]

Looks 🏆 after taking a look at my few minor suggestions.

[Ferdi]

Just curious, why do we need an actual form ? Who is going to use it ?

[bdero]

It's a convenient way of testing the LMOD gradebook API; developers will use it.

[carsongee]

@pdpinch this bug has already been in here since #11 landed, so the issue while identified here isn't caused by this PR. Because of that, I wouldn't think it should block the merge.

[carsongee]

I read your comment more thoroughly @pdpinch and realized I didn't answer the second piece. Yes, I think this blocks both a release of pylmod and lmod_proxy since we would want this resolved before switching to these in production

[carsongee]

@bdero the '' part of .join('') is very important and hard to debug lol. I think I've addressed your comments.

[coveralls]

Coverage increased (+0.37%) to 97.0% when pulling 494975ab33bd089dff9f762e4d7179ae1e95d41a on feature/user_interface into 0c70bf1 on master.

[coveralls]

Coverage increased (+0.37%) to 97.0% when pulling fefe069 on feature/user_interface into 0c70bf1 on master.

[bdero]

Sorry about forgetting to include the '' for the join(). I think all is well now, so merging.