[#10936] Show explicit enroll errors

[daongochieu2810]

Fixes #10936

• Return error messages from backend through EnrollError class to be displayed by frontend.
• Create a new column in the fail to enroll students table to show error messages.
• Fix table layout so text will not overflow.
  

[teammates-bot]

Hi @daongochieu2810, these parts of your pull request do not appear to follow our contributing guidelines:

1. PR Title
   • Issue Reference (#<issue-number>) missing.

[hcwong]

Any reason why the changes were made directly on the StudentsData response directly?

I'm uncomfortable with the idea because StudentsData is used by many Actions. It seems weird to add a failToEnrollStudents field when this would only be used for the EnrollStudentsAction.
EDIT: You can take a look at the rest of the files in ui/output. All of them only have one constructor iirc.

An alternative is to create a custom API response for this action, something like EnrollStudentsData. Do let me know what you think.

[daongochieu2810]

Yes I think having a separate response for this action is better

[hcwong]

Yes I think having a separate response for this action is better

Okay, on that note since EnrollError does not extend ApiOutput, consider making it a nested static class, or a map (not too particular about this). You can look at SessionResultsData for reference. Also consider renaming EnrollError if its still a class since it can be misleading as it is not exactly an Exception.

[hcwong]

Few comments, but nothing major. LGTM, just address the stuff before merging

[moziliar]

One comment here: is it good/possible for the instructor to fold the error messages and have expand all option? In and expanded view, should the # of failed enrolments exceed ~20, it'd be hard for the instructor to view all?

[damithc]

Can we make it easier for the instructor to trace back the error to the spreadsheet? e.g.,

• give the matching row number in this table
• mark the offending rows in some way e.g., turn the bg color to red

[damithc]

One comment here: is it good/possible for the instructor to fold the error messages and have expand all option? In and expanded view, should the # of failed enrolments exceed ~20, it'd be hard for the instructor to view all?

hmm... given that the user is unlikely NOT to want to look at the errors, I think it is fine to show all errors by default. Otherwise it is just one more unnecessary step the user has to perform.

[madanalogy]

Can we make it easier for the instructor to trace back the error to the spreadsheet? e.g.,

• give the matching row number in this table
• mark the offending rows in some way e.g., turn the bg color to red

@daongochieu2810 any response to the comment above? If not something you feel is within the scope of this PR maybe you could help create a feature request issue for it

[daongochieu2810]

@madanalogy the highlighting is done in this PR #10968