S24 recim bracket v2 by amos-cha · Pull Request #1057 · gordon-cs/gordon-360-api

amos-cha · 2024-07-01T20:34:36Z

Mirror UI component: gordon-cs/gordon-360-ui#2347

Rec-IM's old bracket component is no longer being updated and has quickly become an update bottleneck when we attempted to upgrade to react 18. This API PR is a has a UI mirror to handle the change to a new bracket component and its specifications.

The main key pieces required to update packages was

Generating bye matches where no teams exist. My previous implementation would simply not create the matches at all and as I wanted to keep it that way, logic is now used when getting match bracket data to fill out the empty spots on the bracket.
Next Match needs to be known, once again without modifying my old code, I was able to simply organize my list of matches and find the appropriate next match algorithmically.

Gordon360/Properties/launchSettings.json

amos-cha · 2024-07-02T21:13:43Z

TODO:
Next Match ID

…ordon-360-api into s24-recim-bracket-v2

Gordon360/Models/ViewModels/RecIM/MatchBracketExportViewModel.cs

Gordon360/Models/ViewModels/RecIM/TeamBracketExportViewModel.cs

Gordon360/Models/ViewModels/RecIM/MatchBracketViewModel.cs

Gordon360/Models/ViewModels/RecIM/MatchBracketExtendedViewModel.cs

Gordon360/Models/ViewModels/RecIM/TeamBracketViewModel.cs

russtuck

This is good work, and much of it is quite clear. But the heart of the code is complex and hard to decipher, which means maintenance in the future will be hard. Please make it clearer, so it can stay working for years to come. That might mean high-level comments explaining what it's doing, or refactoring into smaller methods with clear names and functions. The latter might be better, but either is fine.

Gordon360/Models/ViewModels/RecIM/MatchBracketExportViewModel.cs

russtuck · 2024-07-09T13:50:49Z

Gordon360/Models/ViewModels/RecIM/MatchBracketExtendedViewModel.cs

+using System.Collections.Generic;
+
+namespace Gordon360.Models.ViewModels.RecIM;
+//UI SHAPE


This example doesn't match the definition of MatchBracketExtendedViewModel below. Why is that? It needs an explanation (unless it just needs an update to match the class).

russtuck · 2024-07-09T14:20:23Z

Gordon360/Models/ViewModels/RecIM/MatchBracketExtendedViewModel.cs

+//        "nextMatchId": null, // Id for the nextMatch in the bracket, if it's final match it must be null OR undefined
+//        "tournamentRoundText": "4", // Text for Round Header
+//        "startTime": "2021-05-30",
+//        "state": "DONE", // 'NO_SHOW' | 'WALK_OVER' | 'NO_PARTY' | 'DONE' | 'SCORE_DONE' Only needed to decide walkovers and if teamNames are TBD (to be decided)


Are these values explained somewhere?

I don't know what WALK_OVER and NO_PARTY mean, and a search of the repo doesn't show them elsewhere. And I'm wondering if DONE implies there is no score (since it's different from SCORE_DONE).

Those are definitions that are explained in the UI package docs. I don't think it is necessary to explain in the API

Gordon360/Models/ViewModels/RecIM/TeamBracketExportViewModel.cs

russtuck · 2024-07-09T14:35:20Z

Gordon360/Models/ViewModels/RecIM/MatchBracketExportViewModel.cs

+    public string? state { get; set; }
+    public DateTime startTime { get; set;}
+    public IEnumerable<TeamBracketExportViewModel> participants { get; set; }
+    public int seedIndex { get; set; }


It might be helpful to add a comment that seedIndex is a numbering of teams from 0 to n-1, essentially a tournament seeding. NOT the seed for a randomized algorithm for assigning position in the tournament. (Assuming I've guessed correctly based on reading some of the other code.)

not exactly, the seed index is the index of where that match exists in each round. its purely a calculation necessity

russtuck · 2024-07-09T14:47:47Z

Gordon360/Services/RecIM/SeriesService.cs

+          * 2) need 'next match ID
+          * 3) status needs to be translated to 'state' enum
+         */
+        var match = context.Match


What can we assume about context.Match?

I don't understand what this means, context.Match is how we access our Match models in the DB. The code itself should be relatively self explanatory of why I need to contain Match.

russtuck · 2024-07-09T14:58:20Z

Gordon360/Services/RecIM/SeriesService.cs

+            }
+        }
+
+        //final conversion to exact UI shape (more efficient than letting the UI handle the key/pair changes


This is a good comment: it explains the goal clearly enough that the code below it makes sense.

The code above could use some similar comment(s).

There is a comment. All the code above does, is as I described:
It fills out all the invisible bye matches and injects nextMatchID into each bracket object

amos-cha · 2024-07-10T14:14:47Z

Gordon360/Models/ViewModels/RecIM/MatchBracketExportViewModel.cs

+    public string? state { get; set; }
+    public DateTime startTime { get; set;}
+    public IEnumerable<TeamBracketExportViewModel> participants { get; set; }
+    public int seedIndex { get; set; }


not exactly, the seed index is the index of where that match exists in each round. its purely a calculation necessity

Gordon360/Models/ViewModels/RecIM/MatchBracketExportViewModel.cs

amos-cha · 2024-07-10T14:15:44Z

Gordon360/Models/ViewModels/RecIM/MatchBracketExtendedViewModel.cs

+//        "nextMatchId": null, // Id for the nextMatch in the bracket, if it's final match it must be null OR undefined
+//        "tournamentRoundText": "4", // Text for Round Header
+//        "startTime": "2021-05-30",
+//        "state": "DONE", // 'NO_SHOW' | 'WALK_OVER' | 'NO_PARTY' | 'DONE' | 'SCORE_DONE' Only needed to decide walkovers and if teamNames are TBD (to be decided)


Those are definitions that are explained in the UI package docs. I don't think it is necessary to explain in the API

Gordon360/Models/ViewModels/RecIM/TeamBracketExportViewModel.cs

Gordon360/Services/RecIM/SeriesService.cs

amos-cha · 2024-07-10T14:18:27Z

Gordon360/Services/RecIM/SeriesService.cs

+          * 2) need 'next match ID
+          * 3) status needs to be translated to 'state' enum
+         */
+        var match = context.Match


I don't understand what this means, context.Match is how we access our Match models in the DB. The code itself should be relatively self explanatory of why I need to contain Match.

amos-cha · 2024-07-10T14:19:39Z

Gordon360/Services/RecIM/SeriesService.cs

+            }
+        }
+
+        //final conversion to exact UI shape (more efficient than letting the UI handle the key/pair changes


There is a comment. All the code above does, is as I described:
It fills out all the invisible bye matches and injects nextMatchID into each bracket object

amos-cha added 3 commits July 1, 2024 14:51

Create TODO

be9a962

Create MatchBracketExtendedViewModel

8dc4e8e

Finalize Datastructure

f34f425

amos-cha added Feature A new feature Broken Feature Issues tagged with this label require a more rapid fix than normal bugs. s24 Summer Practicum 2024 labels Jul 1, 2024

amos-cha self-assigned this Jul 1, 2024

amos-cha marked this pull request as draft July 1, 2024 20:34

amos-cha commented Jul 1, 2024

View reviewed changes

Gordon360/Properties/launchSettings.json Outdated Show resolved Hide resolved

amos-cha and others added 5 commits July 2, 2024 16:10

Complete fill out algorithm

871a799

Fix seedindex error

972a701

add ui detail on viewmodel

53a96e7

Update to include status/state changes

bc32359

revert launch settings

ab8bc8d

amos-cha added 7 commits July 3, 2024 10:23

Add next match id calculation

5529cfa

Merge branch 's24-recim-bracket-v2' of https://github.com/gordon-cs/g…

af777bb

…ordon-360-api into s24-recim-bracket-v2

Update viewmodel types

ecaa92d

Convert to exact UI shape

2e2e9de

Visual update

87c2132

Fix viewmodel naming error

3095b3d

Fix "missing match" bug

60f514f

amos-cha requested a review from EjPlatzer July 8, 2024 13:04

amos-cha marked this pull request as ready for review July 8, 2024 13:04

amos-cha mentioned this pull request Jul 8, 2024

[Rec-IM] bracket v2 gordon-cs/gordon-360-ui#2347

Open

2 tasks

amos-cha requested review from jsenning and russtuck July 8, 2024 20:41

amos-cha commented Jul 8, 2024

View reviewed changes

Add definitions to viewmodels

7b0828a

russtuck requested changes Jul 9, 2024

View reviewed changes

amos-cha commented Jul 10, 2024

View reviewed changes

Update comments

34a7630

amos-cha requested a review from russtuck July 10, 2024 14:59

amos-cha added 6 commits July 11, 2024 09:33

Add code explanation.

5f2c4dc

Ammending more explanation

b84d749

Merge branch

02ee2c6

Add double bye

2b89426

Add bye calculation on match bracket

8185fd8

Update comment

7218dfc

Conversation

amos-cha commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

amos-cha commented Jul 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

russtuck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

amos-cha commented Jul 1, 2024 •

edited

Loading