Whenever Quiz questions are updated, existing quiz_question_relationship entities are being deleted rather than updated, and new relationship entities are created in its place. This appears to occur even if only minor things like weights are being changed.

function quiz_set_questions($quiz, $questions, $set_new_revision = FALSE) {
  if ($set_new_revision) {
    // Create a new Quiz VID, even if nothing changed.
    $quiz->revision = 1;

    node_save($quiz);
  }

  // Clear the existing relationships for this version.
  db_delete('quiz_node_relationship')
    ->condition('parent_nid', $quiz->nid)
    ->condition('parent_vid', $quiz->vid)
    ->execute();

It would be more logical if the overview on node/%node/quiz/questions operated directly on the relationship entities, rather than the question entities. This would allow for targetted updating of only relationships which have actually changed and would update those relationships rather than creating new ones.

I'm having some trouble deciphering the logic in quiz_set_questions and the related quiz_update_quiz_question_relationship, the latter of which seems to aim to simply copy over existing relationships to new entities connected to a new node->vid, but first creates new entities, then right after updates them again with values that could've been inserted in the first place.

Comments

FreekVR’s picture

Issue summary: View changes
djdevin’s picture

Version: 7.x-5.0-alpha7 » 7.x-5.x-dev

Yeah there was a reason for this, with the complexity of 1) only updating the changed relationships and 2) deleting any old relationships and 3) maintaining versions, it was easier to just delete all the relationships for an nid/vid and re-add them. Because this function only takes in a set of questions, #2 was more difficult because the set of questions did not include deleted questions. I think it's from 6.x-4.x 7.x-4.x and was left as is.

When a new revision of a Quiz is created, brand new relationships are created anyway so we don't need to worry about that one. #1 and #2 we should fix. We should also add some tests to verify that the IDs don't change and that questions are removed properly.

I see no reason not to change this now as the current way is sort of bad practice, especially because of #2397863: Add uuid to quiz_question_relationship entity where deleting the relationships would defeat the point of UUID'ing them.

FreekVR’s picture

Yeah, that's kindof why I mentioned it, haha :)

It shouldn't be too hard to prune old/removed relationships - any relationships which are not in the submitted form AND are coupled to the quiz node current VID should be removed, the rest should be updated. I think #1 (only updating changed relationships) is a bit harder and would probably add too much complexity to the code.

djdevin’s picture

Bump for me