Access denied upon completing a quiz when Store results is set to The best

Adding the whole result to the SESSION doesn't really feel like a good solution, its quite a lot of information that you would need to place inside of it.

Saving these results to the database seems to be the best solution. All it would take is an extra field to store the status, and some mechanisme to clean it up. Getting a 404 on a page refresh isn't that big of a deal, we could always add a message to the initial result page stating that its a temporary page. Should the user then get a 404 its at least his "own" fault.

Fixing it this way might also be the best way to solve the problems with the current attempt count, which is somewhat related to this issue. In the above scenario your second attempt never gets saved, so you can keep on trying even though the quiz is set to a max of 2 attempts.

I've made a quick patch based on your solution. Meaning we don't directly delete the result, but instead mark it and delete it upon viewing the entity. The patch should not be committed as is, but is mainly created to test the current test cases as well as a check to see if I'm on the right track with this.

The patch only does two things right now. First of all in the function _quiz_maintain_results($quiz, $result_id) I've added a check to see if the current result_id is going to be deleted. If it is we undo the upcoming delete action and instead run a db_query to mark the result as "invalid". I've used the "is_invalid" field for now as it seems to be a deprecated field that so happens to sort of match the field we need.

Then in the function quiz_user_results($result_id) I've added a check to see if the result is marked as invalid. If it is we delete the result and show a message to the user warning him.

On my local environment this seems to do the trick of showing the result to the user while also directly removing the result from the database.

Deling the results based on a cron would have my preference as well. Make everything more manageable without "random" delete action like in the patch, however I'm not quite sure how much work it would cost to do this. Since we'll be storing invalid results to the database we'll need to update all the reports to match the new behavior.

I'll need to look how much work updating all the reports will cost, but might be an idee to push the fix in two steps. Starting off with the current workaround by deleting the invalid result on view. Which we'll phase out later in favor of a cron once all the reports are reworked with the new state.

Added a patch that forms the groundwork for when we move everything to a cron based cleanup. Would still need a lot of work though.

I've added a setting to the settings form to set the time after which invalid records need to be deleted (with a default of 1 day). Not sure what the best default / available intervals would be, but giving adding some flexibility for advanced users seems a better idee than to simply delete everything on the first cron.

For the cron run I'm not entirely sure if its better to retrieve all the results and put them through a entity_delete() or if its better to delete them directly with a db_delete().

I've yet to look at all the code that needs to be updated, but to pick up on the list from the old issue:
- Attempt numbering
- Build on last
- Quiz resume
- Reports featuring results (for you personal report it might be nice to include in_valid results, but for the admin overview you want to exclude them)
- Writing testscript

The test result that failed seems to check if the store mechanisms are working by checking the around of results it can find in the database. So as excepted those will fail without properly rewriting them first (something that's yet to be done).

I'll rework the test cases later, as I've also found another issue with the current patch as well.

There is one major thing that could be worth mentioning. Once the results are reworked to be deleted on cron run we also have the possibility to fix #2687741: Allowed number of attempts fails when store results is different from all by using the query below for the cron delete mechanism. This way we prevent deleting quiz results for quizzes where the user has a limited amount of takes, thus directly fixing the count query that prevents the user from making more attempts.

  // Remove invalid quiz results.
  $rm_time = variable_get('quiz_remove_invalid_quiz_record', 86400);
  if ($rm_time) { // $time = 0 for never.
    $query = db_select('quiz_node_results', 'qnr');
    $query->fields('qnr', array('result_id'));
    $query->join('quiz_node_properties', 'qnp', 'qnr.vid = qnp.vid');
    // If the user has a limited amount of takes we don't delete invalid results
    $query->condition('qnp.takes', 0, '=');
    $query->condition('qnr.is_invalid', 1);
    $query->condition('qnr.time_end', REQUEST_TIME - $rm_time, '<=');
    $res = $query->execute();
    while ($result_id = $res->fetchField()) {
      $result_ids[$result_id] = $result_id;
    }
  }

However if we also hide the invalid results on the reports we end up with "hidden" records in the database. All I can think of to solve this issue is to add filters to the current result views to show/hide invalid results?

The following things have been done in the patch:

1) The function _quiz_maintain_results has been changed in the way that it only marks quiz_results as invalid. This goes for both QUIZ_KEEP_BEST and QUIZ_KEEP_LATEST.

2) quiz_cron() has been extended to remove all invalid quiz_results older than a certain amount of time (with the default being 1 day). As mentioned in the comment above I've also added the condition that the quiz must have unlimited takes. If it has a limited amount of takes we'll keep the results as to fix issue #2687741: Allowed number of attempts fails when store results is different from all.

3) I've attempted to update all reports/statistics to work with the is_invalid status.

4) The testcase that checks the purging mechanisme has been updated. Right now it will check both the amount of valid as invalid results, ending with a cron run to test if purging the results will work.

@webservant316 Currently I'm not running this patch on any production site. The project that required this functionality had a strict deadline, so alongside creating this patch we also used a different "fix" for the production site.

In the end we simply changed back the storage settings from "best" to "all" and used some custom views that would only show the best results to the user. This has some minor downsides (mainly being that end-users can still see his other results by manually typing the URLs), but for our project this wasn't a big issue.

Theoretically speaking the only way that the code in patch #11 couldn't return a result page is when the results couldn't get stored into the database. Seeming how that would be a critical failure somewhere else in the code I personally don't think we'll need a fail save mechanism.

As far as the status of patch #11. Its been quite a while since it was made, but I'm pretty confident its as good as finished and mainly just needs to be reviewed. I had already updated the test scripts to match the new purging mechanism and as far as I could see all db_queries and default views where updated to use the new "Invalid" status.

The only main issue would be custom views made by people themselves as there is no real way to create an upgrade path for those. Not including the "invalid" check on a view doesn't break anything, but it could end up showing quiz results that shouldn't be shown.

Changes compared to the patch from #11

1. I've added an additional case in the test script that will check if the purging only happens after the given purge time period.
2. All queries within the functions that we touch have been updated to Dynamic queries (db_select & db_update).
3. For all the functions that we touch I've updated the coding standards to match the Drupal standard.
4. Found an additional query which needed to be updated with the "new" is_invalid status.

Same patch as #31, but now without the typo in the test script.