Skip to content
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

Questionnaire: make Summary page display more intuitive #589

Open
wants to merge 2 commits into
base: MOODLE_401_STABLE
Choose a base branch
from

Conversation

toanlamt
Copy link
Contributor

@toanlamt toanlamt commented Aug 15, 2024

E.g. In this questionnaire, they are 151 questionnaire responses, but 198 question responses. As only complete questionnaire responses are analyzed (the data is exported and cleaned), showing more question responses than questionnaire responses in the summary is confusing.

E.g. here's a view of a recent case:
image

If we could make the view selectable, this would clean up the summary and make a bit more sense.

Drop-down: "All responses" [198 in this case]; "Full submissions" [151 in this case]; "Responses not submitted" [47],
Consistent language to delineate between question-specific responses and full/complete questionnaire submissions.
Correct the 'total' and percentages on the visual graphs depending on the view.
E.g. here's a view after improvement
image

@mchurchward
Copy link
Contributor

Can you rebase with the latest MOODLE_401_STABLE? I just fixed the CI.

Copy link
Contributor

@mchurchward mchurchward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your PR still includes commits from other changes that this one. The PR needs to just include the one(s) for this PR.
You need to pull the latest MOODLE_401_STABLE locally, then rebase your branch on top of that.

@tailetan
Copy link
Contributor

rebased with MOODLE_401_STABLE, @mchurchward

@mchurchward mchurchward self-requested a review August 29, 2024 18:05
mchurchward
mchurchward previously approved these changes Aug 29, 2024
@mchurchward mchurchward dismissed their stale review August 29, 2024 18:06

Re-reviewing

@@ -198,7 +198,7 @@ public function get_results_tags($weights, $participants, $respondents, $showtot
if ($showtotals) {
$pagetags->total = new \stdClass();
if ($respondents > 0) {
$percent = round((float)$respondents / (float)$participants * 100.0);
$percent = 100;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? This changes a calculation to a hard coded value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mchurchward
Sorry for this change. During the development of this issue (previously handled by our former developer), it seems that updating the "All participants" text to a dropdown list with the content "Full submissions" - Default value, "All responses", "Responses not submitted" led us to assume that the "Total responses to question" row should always be the sum of the response number/percent above. For example:

Response: Yes - 1 -> 50%
Response: No - 1 -> 50%
Total responses to question row: (2) -> 100%

After thoroughly investigating the logic of this row, we now understand that it should not be hard-coded to 100 as it was before. We will update this code to align with the current logic.

Copy link
Contributor

@mchurchward mchurchward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of questions.

@@ -855,15 +857,20 @@ public function get_responses($userid=false, $groupid=0) {
$sql = 'SELECT r.* ' .
'FROM {questionnaire_response} r ' .
$groupsql .
'WHERE r.questionnaireid = :questionnaireid AND r.complete = :status' . $groupcnd;
'WHERE r.questionnaireid = :questionnaireid' . $groupcnd;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the 'complete' check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the new requirement of separating the 3 response viewing modes, we have been updating this SQL query and adding some logic to check the status correctly for each view.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants