-
Notifications
You must be signed in to change notification settings - Fork 79
WIP using JSON responses to AJAX requests #1190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
This sounds like a big change and it is hard to grasp beforehand how much this will require changes. The existing There are also some AJAX exercises that update the points via a JS event handler. AJAX exercise means that the submission is not posted directly to the A+ server. Normally, the submit form posts to the A+ server.
How would a custom encoder work? I didn't understand how the |
I wouldn't be changing all ajax calls right now. I would only change the ones used by active elements for now (submitting and getting a submission). This shouldn't need much changes.
It kind of depends on how it works. In general, it would be best to do the update without rendering on the server, i.e. just pass the data (points in this case) to the JS and let it do the update but I'd have to check how the code works before I can say for sure. Another option is to fully delegate the points rendering to the client JS but I'd prefer an external framework like React for doing that. We could also look into gradually switching to React or some other similar framework but that is outside the scope of this PR as we need to get the current issue fixed.
The PR text is mainly about requests made to A+. It might be applicable to the ajax exercises too but I'm not that familiar with them, so can't comment.
It's basically just a class inheriting the django encoder with an if isinstance statements in the encoding function (and super() call for other types). The output for each object would include fields that are needed by the ajax calls (e.g. the ones for submission in the example code, more can be added later if needed). It is very simple to implement. Git manager already has a simple one https://github.com/apluslms/gitmanager/blob/41e24405aad57efe8a235db05ac3178c0c655e56/util/export.py#L276 With a custom encoder, we could do stuff like
It is about the concrete implementation of this on active elements. The active elements code displays the messages rendered by _messages.html. Earlier it would do DOM introspection to detect them but I changed the messages to be rendered separately on the server and in the JSON. |
|
I thought it would be faster to fix the bug with small changes. You are suggesting large refactoring, but we should fix the active element submission feedback polling quickly. If we at least first finish a quick and small solution, then there is more time later on to think about major refactoring. For example, the a-plus/exercise/static/exercise/chapter.js Line 605 in 564ee09
If data-ready-url were added there similarly, would that lead to a quick and simple fix to the urgent issue (#1188)?
|
|
Oh, and many views already have two versions: one for ajax request and one for normal requests. It makes much more sense to me to just return the data the client wants in a json instead of having them find it in the html. It also seems a bit dubious to serve a different page on an ajax request than a normal request (and more difficult to debug). |
|
#1192 implements a quick fix to the issue as we need it fixed ASAP. Let's have this PR solely about the ajax refactoring |
Use `@accepts_mimes(<list of implemented mime types>)` to decorate view method/function with the accepted types, and then use `request.expected_mime` to get the type the client expects (the one that matches best with the implemented mime types).
Also fixes an issue where the submission isn't visible if messages are present (e.g. "Staff can submit assignments without enrolling.")
|
I now made an implementation of JSON responses to AJAX calls for active elements submitting. The more I think about this, the more I want to try swapping to using React because then most of the rendering happens on the client, simplifying updating of the site considerably. Edit: as an example, the alert messages could be renderer on the client based on a list of messages inside the JSON to allow easily updating the alerts instead of rendering them on the server like how it works currently and in this PR. |
I created this PR to discuss the idea.
I do not think it is a good idea to load pages using ajax and then use DOM manipulation to get the needed information out of the loaded page. This will easily cause issues (that are also hard to notice and debug) if the page HTML changes. There is also no indication in the HTML itself (or whatever creates it) that the JS is dependent on a specific structure. I propose we add JSON versions of the pages that would otherwise need such DOM introspection to move the logic to python where it will be much easier to see if changes have an effect on something else.
Description
What?
[ANSWER HERE]
Why?
[ANSWER HERE]
How?
[ANSWER HERE]
Testing
What type of test did you run?
[ADD A DESCRIPTION ABOUT WHAT YOU TESTED MANUALLY]
Did you test the changes in
Think of what is affected by these changes and could become broken
Translation
Programming style
Have you updated the README or other relevant documentation?
Is it Done?