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

Instructors reminding non-submitters: instructors are considered non-submitters even if there are no questions for instructors #11737

Open
damithc opened this issue Apr 14, 2022 · 8 comments · Fixed by u6656793/teammates#1
Labels
p.Low Very little impact; unlikely to do in the near future s.ToInvestigate The issue sounds complete but needs validation by a core team member

Comments

@damithc
Copy link
Contributor

damithc commented Apr 14, 2022

Environment: v8.13.1 (production)

Steps to reproduce

  1. Instructor clicks the Remind -> all non-submitters of a session from the home page

Problem: Instructors of the course are listed as non-submitters even when the session doesn't have any questions for instructors.

@damithc damithc added the p.Low Very little impact; unlikely to do in the near future label Apr 14, 2022
@hoanglan21
Copy link
Contributor

I want to have a try at this issue

@hoanglan21
Copy link
Contributor

I don't think showing the instructor's name is necessary because the instructor is the one who is sending the reminder emails here, unless can you think of a case where the instructor specially wants to send this survey to another instructor?

@damithc
Copy link
Contributor Author

damithc commented Apr 29, 2022

... unless can you think of a case where the instructor specially wants to send this survey to another instructor?

@hoanglan21 Yes, there can be multiple instructors in a course (e.g., tutors) and an instructor might want to send reminders to other instructors of the course.

@hoanglan21
Copy link
Contributor

So i have looked at how you get the list of non-submitters: you get a list of all instructors & students in the class (1) and then a list of people who have had at least 1 answer (2), whoever is in list (1) but not in list (2) will get sent a reminder email. This makes it hard to change the code because I will have to make a query of all the questions and their respective responders to see if any of the questions require the instructors to answer them or not.
Do you have any advice on how to tackle this? Should i fix this from the BE or the FE?

@damithc
Copy link
Contributor Author

damithc commented May 10, 2022

So i have looked at how you get the list of non-submitters: you get a list of all instructors & students in the class (1) and then a list of people who have had at least 1 answer (2), whoever is in list (1) but not in list (2) will get sent a reminder email. This makes it hard to change the code because I will have to make a query of all the questions and their respective responders to see if any of the questions require the instructors to answer them or not.
Do you have any advice on how to tackle this? Should i fix this from the BE or the FE?

Thanks for looking into this @hoanglan21 If it is hard/costly to achieve, we can leave it be. After all, the instructor using this feature would know if the session contains questions for instructors and reminders need to be sent to instructors as well.

I'll leave this issue open so that a senior dev can confirm the technical difficulty you have mentioned, after which we can close this issue.

@damithc damithc added the s.ToInvestigate The issue sounds complete but needs validation by a core team member label May 10, 2022
@sivayogasubramanian
Copy link
Contributor

sivayogasubramanian commented Jul 7, 2022

@damithc Investigated this issue.

The cause of this issue is the GetStudentsAction::execute method returns a response that includes the instructor (I believe that this is wrong). However, this also suggests that the student's database is polluted with instructor entires. This is a very big issue and therefore I do not know if this behavior for the GetStudentsAction::execute is intended.

[
Student:Alice Betsy[alice.b.tmms@gmail.tmt], 
Student:Benny Charles[benny.c.tmms@gmail.tmt], 
Student:Charlie Davis[charlie.d.tmms@gmail.tmt], 
Student:Danny Engrid[danny.e.tmms@gmail.tmt], 
Student:Emma Farrell[emma.f.tmms@gmail.tmt], 
Student:Francis Gabriel[francis.g.tmms@gmail.tmt], 
Student:Gene Hudson[gene.h.tmms@gmail.tmt], 
Student:Hugh Ivanov[hugh.i.tmms@gmail.tmt], 
Student:John Doran[teammates.instructor@university.edu],
]

Notice the response of the GetStudentsAction::execute method. I believe that the last entry should not be there. However, this is directly coming from the students' database (no appending or merging of data from the instructor database).

@damithc
Copy link
Contributor Author

damithc commented Jul 7, 2022

@sivayogasubramanian note that it is possible for an instructor to be a student. In fact, the demo courses we create for instructors (when they join TEAMMATES) have themselves added as students. Does that affect your finding above?

@u6656793
Copy link

Hi @damithc , Does this issue still open? If so, could I work on it for my first open source study? TY.

u6656793 added a commit to u6656793/teammates that referenced this issue Oct 28, 2022
… is an instructor then not adding to the list.
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 28, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 28, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 28, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 28, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 29, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 29, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 29, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 29, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 29, 2022
u6656793 added a commit to u6656793/teammates that referenced this issue Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment