Issues with reading tutorial/submission information from server support

[jgat]

I believe this issue occurs in the master branch, but the surrounding code has been modified in the admin_interface branch (see #111) to the point that resolving this issue in master would lead to further merge conflicts when completing the branch. Hence, all code links in this issue are links to that branch.

One of the support methods required for the user progress page is to retrieve a list of all submissions the user has made to the current set of problems, specified in order of the problems. The current solution does not take into account the "tutorial hash mappings" file which describes the relation between old and new hashes of a tutorial.

The parse_tutorial_hashes method does not have this issue, but that method does not retain the original order of the problems, nor does it retain information about the current set of problems. As a result, get_submissions_for_user (which has been moved from mpt_cgi.py into support.py in this branch) does not retain either of these necessary pieces of information (and it certainly doesn't return "A list of two-element tuples" as the docstring indicates).

The desired solution is a method which returns a list of TutorialInfo and TutorialSubmission objects for each current tutorial problem, ordered according to the hashes file (with submission date possibly None). The method should also respect submissions to previous versions of a tutorial, but not include duplicate entries in the returned list -- i.e. old and/or deleted hashes will not be included, but submissions to those old hashes will be included paired with the new hash - if both the old and new hash have a submission (which should't occur in normal use), then the one with the earlier submission timestamp should be taken.

To see this bug in effect, observe the differences between reporting marks in http://csse1001.uqcloud.net/cgi-bin/_tests/admin.py vs. the individual "view progress" pages (in particular, s4206155, uqjgaten, uqprobin). This includes both marks for updated tutorials and deleted tutorials. The admin.py reported mark uses get_submisisons_for_user, while the progress.py reported mark uses this method.

[sapi]

Just quick comments as it's late; I'll have a closer look tomorrow.

The get_submissions_for_user function is intended to be tutorial package agnostic, which is why it doesn't attempt to preserve order, take care of deletions, or anything like that. I'd view those as client responsibilities, or at least as something for a different function.

If it's including marks for two versions of the same tutorial (old and updated), that's definitely a bug. It should only consider the latest version. But I don't think it should try to narrow things down to a given tutorial set, or even be aware that such a set exists.

Also, would it be possible to limit support.py to functions which touch the file system? I don't have a problem with refactoring code into different files, but I'd like to keep support.py (renamed if necessary) as an abstraction layer for what will later be a database.

[jgat]

Briefly: On 23 Feb 2015 23:20, "Sean Purdon" notifications@github.com wrote:

Just quick comments as it's late; I'll have a closer look tomorrow.

The get_submissions_for_user function is intended to be tutorial package agnostic, which is why it doesn't attempt to preserve order, take care of deletions, or anything like that. I'd view those as client responsibilities, or at least as something for a different function.

Right, I probably got the wrong impression of the role of that function.

If it's including marks for two versions of the same tutorial (old and updated), that's definitely a bug.

I don't know if it is or not; I didn't test it, I just read over the code while being not entirely awake. The ideal scenario is that it looks for submissions from both old and new tutorials, and consolidates them nicely.

It should only consider the latest version. But I don't think it should try to narrow things down to a given tutorial set, or even be aware that such a set exists.

Also, would it be possible to limit support.py to functions which touch the file system? I don't have a problem with refactoring code into different files, but I'd like to keep support.py (renamed if necessary) as an abstraction layer for what will later be a database.

Sure. I probably had some vision that get_submissions_for_user, or a variant thereof, might form some high-level component of the DB interface, rather than being a helper function for the CGI interface.

[sapi]

After looking at the code, this is the basic process atm:

• parse_tutorial_hashes will give a mapping of all known hashes to their current iteration (except where a tutorial has been removed, in which case it won't be in the dictionary)
  • note that this dictionary (should be) unique on values, not keys, as multiple hashes can map to the same current tutorial if it was updated more than once
• get_submissions_for_user will return the submission status of all current tutorials
  • if a user submitted under an older incarnation of a tutorial, that will be counted
• the client then narrows that down to submissions that they're interested in (eg, only for the CSSE1001Tutorials package)
  • I would prefer ordering to happen at this step, as it's only really meaningful on a UI level

In other words, I don't think there's a bug in either function, and I'd prefer for them to stay as they are.

However, I've got no objection to adding an additional function, which would structure the submission statuses according to some tutorial package. If so, this should probably make use of the known path to the CSSE1001Tutorials.zip file on the server (open it using the zipfile module and extract the relevant data).

[jgat]

I haven't looked at it too much since then, but yes, I believe those are correct, and the parsing which is done in cgi-bin/progress.py is not.

[sapi]

Yeah, that looks a bit too barebones to me. Probably best to consolidate it to use the existing get_submissions_for_user call.

[jgat]

That would be ideal, except that my use case requires an ordered list of submissions, as well as the timestamp of the submission (though that second requirement isn't absolutely necessary)

[sapi]

What about adding another call that wraps get_submissions_for_user and orders the list? That was what I was trying to hint at.