i_900: add support for multiple groups per team #909
Merged
+7,206
−5,266
Conversation
This file contains 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
johnbrvc
added
enhancement
troy2914
requested changes
Jan 7, 2024
test/edu/csus/ecs/pc2/core/imports/clics/CLICSAwardUtilitiesTest.java
Outdated
Show resolved
Hide resolved
troy2914
approved these changes
Jan 8, 2024
troy2914
approved these changes
Jan 11, 2024
johnbrvc
force-pushed
the
i_900_multiple_groups_per_team
branch
from
48cb057 to
cc2542f
Compare
troy2914
reviewed
Jan 21, 2024
|
just lookin at the results_D1.xml, I think we need to add a attribute to the standingsHeader to show what the xml is showing.. like group_D1 or something. |
Adding support for teams in multiple groups.
The CMS currently allows only 1 group (site) to be defined per use. We remember which group was the one provided by the CMS. This is used for display format substitution for team names that want to include the group. TODO: When the CMS supports multiple groups (sites) per user, the display format substitution routine will have to be modified, or, the ability to add a group to the display name will have to be removed.
Huge commit finishing off implementing multiple groups / team. Fixed several spelling errors in Unit Tests. Fixed bug in InternalContestTest.java (wrong variable string2 used) A couple of efficiency CI's: HashSet's instead of Vectors.
First round of test - viewing, editing and saving account groups from GUI. Works.
The self-signed API certificate should not be in the repo.
Still needs a bit of cleanup on the gui, but nominally works. Need to fix HTML standings as well.
Fixed some copyright comments. Removed extra spaces at ends of lines Removed dunsel code Consolidated some scoring methods to ScoreboardUtilities Note: in DSA and NSA, groupRank is only computed for the primary group (CMS group). This should be changed to compute the rank for each group the team is a member and save it in the standingsRecord. Then, in the XML create a groupinfo element for each group the team is in contain it's group rank for that group. See addGroupRow in NSA.
Allow showing standings on StandingsHTMLPane based on group (checkboxes). Code cleaup on StandingsTablePane.
During the PR workflow, a couple of junit errors were found as a result of the new group code. These have been fixed.
Update copyright date notices. Delete unused code. Delete $Id strings
… groups Generate scoreboards for all groups that are marked for "show on scoreboard" by appending "_GROUP" to the base name. eg. index.html -> index_D1.html.
Groupnames were not generated properly if used in a substitute var. Allow all group names to be used as substitute vars even if not explicitly shown on scoreboard.
Unit testing generates lots of files that should not be included by git. Add these exceptions to .gitignore.
Add canView(gouprlist) to Problem class. Checks if the problem is visibil3 by any group in the list. In scoring algorithms, call canView(grouplist) using wanted groups for the scoreboard. Note that null for wanted groups means "everything" is shown (all problems).
Fix ambiguity with using 'null' as argument to Problem.canView(obj). Add unit tests for test testing problem visibility for a List<> of groups.
Add group filtering to the Filter class. This allows reports to be filtered by group. Particularly useful on the event feed json report (for resolver). Note that if a Filter has group filtering enabled, this will apply group filtering to accounts and problems, even if the filter is not explicitly filtering by account or problem. This is logical.
There was a longstanding bug where if a team was not a member of a group that ended in D#, the scoreboard (DSA) would cause a NPE and never generate a scoreboard. This was because a null is returned as the 'div' if the group (or now groups) the team is in is not a "Division" group (*D1, *D2, ...) Fixed bug with wrong variable check in loop for groupname.
Update some copyright dates. Fix some cut&paste comment errors.
Previously, only the since group in the filter for the event feed was included in the event feed "groups". In reality, any groups that the filtered teams belong to should also be included in the event feed. Consider a team that is a member of groups D1, Oregon, Oregon-D1 and we filter the Eventfeed by group D1. There were no entries in the eventfeed for Oregon nor Oregon-D1, even thought the team was a member of these. This fix includes those "groups" entries in the eventfeed.
johnbrvc
force-pushed
the
i_900_multiple_groups_per_team
branch
from
75cc40b to
6a85192
Compare
… in standings xml There needed to be a way to indicate which group(s) a particular standings XML tree was generated for. The dumpGroupList() method now adds an attribute, included, indicating if the particular group is included in the XML standings.
we discussed this and opted the included=1 on the grouplist/group |
Adding multiple group support necessitates added support for a remote client requesting the event feed to specify which group(s) they want the feed for. Added support in the (already existing) EventFeedFilter to support multiple group filtering, in addition to the already existing starting event id (token) and record types. Added query param to event-feed caleld "groupids" which is a comma-separated list of cms group id's to filter the EF by. CI: Change System.out.println's to 'info()' calls which also logs the messages. CI: print more descriptive message when a client connects (like who they are) CI: improved efficiency of filtering the event feed by avoiding (as much as possible Regular Expression usage. CI; Fixed spelling error for a unit test (testgetEventFeedEequence->testgetEventFeedSequence
Unit tests found some oversights (ie bugs) in the code. Fixed what it found so far.
troy2914
approved these changes
Jan 28, 2024
Incorrect clarifications were being returned on the EventFeed since the test for clars from specific teams was backwards.
|
From @SamanwaySadhu on slack: 9:50 PM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of what the PR does
This PR provides built-in support for an account to belong to multiple groups, instead of just one. The allows per-group display of standings, scoreboard generation and report generation.
The following changes were made to support multiple groups:
HashSet<ElementId> groupIdstoAccountobject - this is a hash table containing all groups the account is a member ofprimaryGroupIdwhich is now the single CMS supplied group ID. These is currently used by several unit tests, and the DSA/NSA for group ranking. (Note: Group ranking has to be fixed so it ranks the team for each group the team is a member of. This is a non-trivial change).Accountto support adding a group to an account, fetching the groups for an account, clearing the groups.ITeam,ITeamImplementationto support multiple groups for the PC2 api (WTI) interface.PacketFormattershows all groups for an account.ContestXML, when generating contest xml, add all groups for each accountExportAccounts, include all groups for each accountICPCAccountto support multiple groups, although currently, only one is ever present in an ICPC "tab" file. Change made for consistency.ICPCImportDatato pass all groups when creating anICPCAccountAccountreport generation to include all groups the team is a member of in the reportScoreboardUtilitiesto support multiple groups for routines that previously only checked one group per account.getGroupFilteredRuns()toScoreboardUtilitiesto return runs appropriate for selected groups only. (used by DSA/NSA)JSONToolto generate multiple groups per accountPacketHandler, change the routine that calculates which problems a team can see (it's now based on multiple groups, not just one)LoadAccountsto accept a CSV list of groups on importing accounts instead of just a single group.CLICSAwardUtilities, change getGroupForTeam to getGroupsForTeamInternalContest, changeaddProblemto account for all groups the team belongs toProblemGroupAssignmentReportwas changed to check all groups for a team.EventFeedXML,EventFeedXML2013andResolverEventFeedXMLadd all groups a team is a member of to XML feed as separate group tagsTeamData, use renamedprimaryGroupId- it's ok to only support one group here since this is a legacy feed objectICPCTSVLoader, when processingteams.tsv,teams2.tsv, accept CSV list of groups in the group field.AccountsPaneandAccountsTablePaneto show all groups a team is a member of in the Groups column.EditAccountPaneto allow the selection of multiple groups for an account using checkboxes (still dome code that is commented out that needs to be deleted before approval)ReviewAccountLoadto show multiple groups correctly, and to detect changes properly in groups.divisionNumberas well for WTI.StandingsTablePaneandStandingsHTMLPaneto allow selection of which groups to show the standings for on thick client standings panes.ScoreboardView,ScoreboardModuleand related classes to deal with multiple groups. Scoreboards are genereated for every group marked as "show on scoreboard". Files are created with the group name embedded in the file name.cacerts.pc2to.gitignore@overridesadded on inherited methodsEventFeedXMLTODO:
Issue which the PR addresses
Partial implementation #900
Environment in which the PR was developed (OS,IDE, Java version, etc.)
Windows 11
Java 1.8.0_321
Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)
Where to start?
Everything that deals with accounts has to be tested:
AccountTablePane, Standings, HTML Standings, Editing accounts, Editing groups, scoreboards, reports, problems, ...More to come on this once complete.