Teammates: Student: feedback submission: distribution points question: explain if equal points are not allowed

[damithc]

v6.5.1

Scenario:
A session has a 'distribute points' question. It requires all points to be different

Current: When a respondent is submitting responses, the submission is rejected if the student gives equal points to all recipients, which is the correct behavior. However, the question description doesn't mention that points need to be different. The restriction is revealed only when the student tries to submit.

Suggested: When the question is presented to the respondent, also explain that equal points are not acceptable. That way, the respondent doesn't have to discover the restriction by trial-and-error.

[jimmy51997]

@damithc ..... I would like to work on this issue.... anywhere I can get help from?

[sukanta-27]

@jimmy51997 As you are a first time contributor, I would advise you to start by taking one of our d.FirstTimers issues to get familiar with the codebase and workflow. But if you strongly feel that you want to take this issue, then you can also do that.

[jimmy51997]

@sukanta-27 Every first timer issue seems already to be taken up....what should i do?? Say if i necessarily want to work on this issue, How to start?

[sukanta-27]

@jimmy51997 Figuring out what to do is a part of the task.

I hope your work environment is set and you have run the app in dev server. If you haven't, do that first.
Then find out which files needs to be modified to solve this issue, modify them, update html documents with GodMode, run tests, make sure all tests are passing. Then make a PR against the TEAMMATES master branch for review.

[jimmy51997]

@sukanta-27 I have set up the environment.........Thanks for the input.

[sukanta-27]

@jimmy51997

Every first timer issue seems already to be taken up

Also, #8589 doesn't seem to have an open PR, if you want you can take it.

[jimmy51997]

@sukanta-27 ....Are u sure it doesn't have an open PR?? If not how to know if it has an open PR or not? one more thing...does this project have a chat channel?

[crphang]

@jimmy51997. Yes, no open PR is opened for it. You can look at the issue to see if a referenced PR is created to address it. A PR that fixes the issue should have '#issue_id' in the PR title and description.

[jimmy51997]

@Crphang thanks...I think i got it now.

[sukanta-27]

does this project have a chat channel?

@jimmy51997 For TEAMMATES, the issue tracker acts as a chat channel. If you have any problem or want to reach out to other developers (Like a help request, feature request etc), then you can open an issue for it. Do remember to follow the appropriate issue template while creating an issue.

[mg14777]

I would like to work on this issue

[mg14777]

@damithc Sir, is this what you had in mind ? I made use of the instructions parameter in FeedbackSubmissionEditQuestion and modify it when the checkbox you highlighted above is ticked.
Another thing worth mentioning is that once a user starts allocating points the status does mention that same number of points shouldn't be given multiple times.

[damithc]

hmm... this is a neat way to get the job done but I'm a bit reluctant to piggy-back of visibility information for two reasons:

• It feels like a 'trick' or a 'clever shortcut'. It is better to avoid such tricks to maintain the simplicity of the code.
• Some users may automatically disregard text in gray thinking that it is a visibility explanation.

From a UIX point of view, it feels like the new instruction belongs together with the existing instruction shown in blue in your screenshot.

[mg14777]

@damithc I have changed it. I agree the visibility is poor. I was trying to make minimal changes to code to accomplish this. My bad. I have clubbed the instruction with the status display. The initial view is shown below:

On halfway allocation of points, I display an error if any two allocations are same.

In the end, if all points have been distributed, the error is still shown until all points are different.

[sukanta-27]

@mg14777 Just a thought, maybe Error: is not a good word to use here? The red font-color is enough to grab the attention of the user IMO.

[mg14777]

@sukanta-27 Thanks for your suggestion. The reason why I put Error was to differentiate it from the first statement "All points distributed" since the second one is specifically an error not a generic statement. But maybe that's just me :sweat_smile: I can change that sure.

[sukanta-27]

@mg14777 I would suggest you to wait for Prof. @damithc's suggestion before making any changes. :smile:

[damithc]

@mg14777 Looks like we have two independent conditions but treat both of them as one. e.g., Even when all points are distributed, All points distributed! appear in red. Is it possible to make the two operate independently?

Good point @sukanta-27 but I guess no harm using the word Error as it will make it even more clearer. Note that some users can be color blind.

[mg14777]

@damithc The way it currently works is that the font color of both statements is dependent on whether there is an error or not. Once the user fixes the error All points distributed! turns green:

We could completely separate them by having two different <p> elements for both messages and applying different formatting to them. Currently they are part of a single <p> element and the formatting is overridden by the error statement

[damithc]

We could completely separate them by having two different <p> elements for both messages and applying different formatting to them.

Let's give it a try

[mg14777]

@damithc I have separated the conditions now. Error statements are displayed separately whenever needed.

If all points have been distributed but some are same we display a green and red flag separately

If some points are left to be distributed it is simply stated in blue

If there is an overallocation two separate error statements are displayed for clarity

[damithc]

Nice work!
While we are touching this code, see if you can tweak these things too.

• I think it will look nicer if the messages are left-aligned.
• It may be easier for the user to understand if we said something like Expected total is 300 but your total is 302! Please adjust numbers to make the total 300

[mg14777]

@damithc Does this look intuitive

[damithc]

Looks better.
Alignment: indent a bit?

The current color choice is a bit fuzzy (e.g., total<400 is shown in blue). Shall we do the following?

The total points should add up to 400. and Multiple options should not be given the same number of points. can always be in blue and remain static as they are simply the instructions.

In addition, and based on the actual points, we can also show these:
✅ Actual total is 400. (green)
❌ Actual total is 396! Please adjust points to add up to 400. (red)
❌ Actual total is 401! Please adjust points to add up to 400. (red)
❌ Multiple options are given 22!. Please ensure that multiple options are not given the same number of points. (red)

[mg14777]

I agree with most of them.
I was thinking if instead of Actual total is 396! Please adjust points to add up to 400. we can instead state the pending amount (or extra in case of overallocation) like Actual total is 396! Please distribute the remaining 4 points. because we are already specifying the constraint/instruction in the beginning as you suggested.

Another thing to clarify, should I replace Error: with icons then ?

In the last statement, what should happen if there are multiple sets of repetitions ? For example, if 2 users have been allocated 22 and other 2 have been allocated 23. Should we display all such repetitions. Not sure if this is applicable here, but from a scalability perspective that might lead to a lot of repetitions to display?

[damithc]

I was thinking if instead of Actual total is 396! Please adjust points to add up to 400. we can instead state the pending amount (or extra in case of overallocation) like Actual total is 396! Please distribute the remaining 4 points. because we are already specifying the constraint/instruction in the beginning as you suggested.

Yes, that's a good idea.

Another thing to clarify, should I replace Error: with icons then ?

We can try that. Use the standard bootstrap glyphicons we use in other places.

In the last statement, what should happen if there are multiple sets of repetitions ? For example, if 2 users have been allocated 22 and other 2 have been allocated 23. Should we display all such repetitions. Not sure if this is applicable here, but from a scalability perspective that might lead to a lot of repetitions to display?

Perhaps we limit to first duplication of points we detected? We can phrase it like this Multiple options are given the same number of points! e.g., 22 Please ensure that multiple options are not given the same number of points.

[mg14777]

@damithc I have made the following changes

Instructions are listed in blue and other notifications/messages follow

Since currently the functionality is implemented commonly for both Distribute points among recipients and options type of questions, I have added this feature for among Options type of questions as well. This includes conditions where at least one of the options must be different etc. On a side note, it seems like the recipients and options are indented differently atm. Maybe this is something that can be fixed ?

Also I was wondering if I can create a PR and commit some of this now.

[damithc]

Looks good. Yes, can start a PR to seek prelim comments about the implementation approach.
One thing though: as there are two instructions, we should also have two 'status' messages too? i.e., there should be a ✅ or ❌ for each ℹ️