Teammates: Instructor: edit question: do not warn about deleting responses if destructive changes are cancelled

[leeyimin]

Steps to replicate the bug:

1. Click 'Edit' for a question with existing responses.
2. Change the visibility option to some other listed option.
3. Change the visibility option to the original option (or discard changes).
4. Attempt to save the question.

A modal warning the user that responses will be deleted is shown although the visibility is not changed.

Edit: After #6124 is solved, the issue will not affect (most?) visibility changes, but is still present for team contribution question and when feedback path is edited and then reverted.

[whipermr5]

Noticed this too. In fact, steps 2-3 are not even needed. Edit: This is true for some question types. E.g. team contrib questions, for which merely clicking edit followed by save results in the modal warning.

[VamsiSangam]

There are multiple bugs in this issue. As @leeyimin said, discard changes and try to save, delete existing responses shows up. It happens because when we restore original version of a question, we only restore the .panel-body contents, whereas editstatus attribute is changed in form tag to mustDeleteResponses but it is not reverted. This causes the warning modal.

And about what @whipermr5 said,

E.g. team contrib questions, for which merely clicking edit followed by save results in the modal warning.

It happens because, when edit button is clicked for contribution questions, fixContribQnGiverRecipient() and setContribQnVisibilityFormat() are called which change the visibility options (as observed in developer tools), so upon saving the warning modal shows up.

And if I may add another bug. When editing visibility option, simply selecting the same visibility option again (don't even change it to another) will also cause the warning modal to show up! This is because, there's a flaw in checking if the visibility option is changed. Current logic compares the text of .visibility-options-dropdown button and the selected option's text. It will always fail due to space issues -

Also upon saving, we need to check if the question is same before and after editing, i.e. cases where visibility option is changed but reverted back to normal, in which case mustDeleteResponses attribute value should be removed if present. This too will prevent the warning dialog.

On second thoughts, maybe we should stop instructor from saving a question if it is same before and after editing. We could save a few requests. :stuck_out_tongue_winking_eye:

[VamsiSangam]

I'd like to work on this issue. Also to confirm with you @leeyimin , can I assume that the bug I stated above and the one stated by @whipermr5 is added to the scope of this issue as well?

[whipermr5]

whereas editstatus attribute is changed in form tag to mustDeleteResponses but it is not reverted

This means the backend actually deletes the responses if the user confirms, right? It's not just an issue of a false warning appearing. I believe this is quite a bad bug, as it means that users cannot edit any part of team contrib questions that already have responses without losing all responses.

[leeyimin]

@whipermr5 The deletion mechanism works independently from the front end I believe, see #7642.

[damithc]

Does #7642 invalidate this issue?

[leeyimin]

@damithc I have tested and the team contribution issue is still present.

[leeyimin]

It is still an issue if feedback path is edited once and then reverted to original.

[whipermr5]

It happens because, when edit button is clicked for contribution questions, fixContribQnGiverRecipient() and setContribQnVisibilityFormat() are called which change the visibility options (as observed in developer tools), so upon saving the warning modal shows up.

This is actually due to code I wrote a long time ago 😅 #6669

[VamsiSangam]

This is actually due to code I wrote a long time ago :sweat_smile: #6669

IMO the real culprit is this -

Whenever the page is loaded, somehow the input fields showresponsesto, showgiverto, showrecipientto seem to set to RECEIVER,INSTRUCTORS for all the questions. But when submitting the question form (even without any edits), the correct values are being sent.

I think we should look into this too. Maybe if we loaded the values of showresponsesto, showgiverto, showrecipientto fields during HTML templating itself, then even if fixContribQnGiverRecipient() or setContribQnVisibilityFormat() are called for contrib questions, it probably wouldn't lead to a destructive change. Just a hunch :sweat_smile:

[HirdayGupta]

@VamsiSangam You are working on this?

[VamsiSangam]

Yes I am.