[#13646] Rename default Instructor team and section constants#13683
Open
Nsohko wants to merge 19 commits intoTEAMMATES:masterfrom
Open
[#13646] Rename default Instructor team and section constants#13683Nsohko wants to merge 19 commits intoTEAMMATES:masterfrom
Nsohko wants to merge 19 commits intoTEAMMATES:masterfrom
Conversation
| return this.section.isEmpty() ? Const.DEFAULT_SECTION : this.section; | ||
| } | ||
|
|
||
| /** Section as entered (may be empty); used for reserved-name validation. */ |
Member
There was a problem hiding this comment.
Moving the check to validate method would be sufficient. Check should be done on the FE before they submit the form as well.
Comment on lines
+61
to
+69
| for (StudentsEnrollRequest.StudentEnrollRequest studentEnrollRequest : studentEnrollRequests) { | ||
| try { | ||
| sqlLogic.validateReservedTeamAndSectionForEnrollment( | ||
| studentEnrollRequest.getTeam(), | ||
| studentEnrollRequest.getSectionInput()); | ||
| } catch (EnrollException e) { | ||
| throw new InvalidOperationException(e); | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Don't think this is necessary. Just validate the inputs should be fine.
I would like the mental model to be "Users cannot set Team and Section name equivalent to the special constants", not "Team and section name cannot be the special constants".
This makes it an input validation concern, not a system one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13646
Reserved team and section names
The system identifiers are fixed to TEAMMATES_RESERVED_INSTRUCTORS (synthetic instructor team in roster/results) and TEAMMATES_RESERVED_DEFAULT_SECTION (default / “no specific section” section).
The backend now blocks anyone trying to manually create teams / sections with these names
The old 'reserved' names (Instructors / None) are now allowed
generateTypes and the frontend
Running ./gradlew generateTypes (via generateApiConstants) refreshes src/web/types/api-const.ts, so the Angular app gets ApiStringConst.DEFAULT_SECTION and ApiStringConst.USER_TEAM_FOR_INSTRUCTOR with the same values as teammates.common.util.Const / ApiStringConst on the server. The UI uses those generated constants for comparisons and labels (e.g. showing “No specific section” when the section key is the default) without duplicating magic strings by hand.
On the frontend, the unique identifiers are converted to human readable 'No Specific Section' and 'Instructors' respectively.
If a section exists with the name 'No Specific Section', it is displayed as 'No Specific Section (Named Section)' to disambiguate
Similarly, a team called 'Instructors' is displayed as 'Instructors (Named Team)'
Finished a todo in student-update validation: it now sets the existing student’s id on the Student used for validation before validateSectionsAndTeams, removing the need for temporary email swap that existed only to make batch logic line up, and extends isInEnrollList so a roster row matches the batch by persisted id as well as email—covering single-student updates (including email changes) where the old and new addresses no longer match.