Closed the-maldridge closed 5 years ago
I've scattered some comments through the files for things I'm not sure of. I can turn around the API in a day or two once the models are approved. I've eschewed security for the moment in order to get to a point we can more easily decide what we do and don't like.
Yes that would be the right way. Greg
On Tue, Jan 29, 2019, 10:32 PM Michael Aldridge <notifications@github.com wrote:
@the-maldridge commented on this pull request.
In internal/models/hub.go https://github.com/BESTRobotics/registry/pull/6#discussion_r252113462:
@@ -0,0 +1,42 @@ +package models + +import (
- "time" +)
+// The Hub is an organization that may run events on behalf of BEST. +type Hub struct {
I haven't yet figured out a good way to attach hubs to seasons. It seems like it may be best to do this with another model which is created by an administrator on verification of license fees for a season. Input is appreciated.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/BESTRobotics/registry/pull/6#pullrequestreview-197900550, or mute the thread https://github.com/notifications/unsubscribe-auth/Aswa0KCIOEz1BwWm9qZUBSavCe0PKNPJks5vISBHgaJpZM4aZby9 .
Agree.
On Tue, Jan 29, 2019, 10:32 PM Michael Aldridge <notifications@github.com wrote:
@the-maldridge commented on this pull request.
In internal/models/hub.go https://github.com/BESTRobotics/registry/pull/6#discussion_r252113538:
+
- // The hub director is the primary point of contact for the
- // hub and is responsible for its smooth running. Changing
- // this field requires the approval of BRI.
- Director User
- // A director can't do it alone, they have people that help
- // them. These are the admins and can act as the director for
- // the hub in many actions. They can be added and removed by
- // the director.
- Admins []User
- // Location stores the City and State of a hub. Since the hub
- // itself is usually only rooted to a city, this is stored as
- // a string.
- Location string
My intent here is that this is a rough location. Events themselves will have precise geo-references that allow them to be stuck on a map, but this is enough to generate a map of "what cities have events".
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/BESTRobotics/registry/pull/6#pullrequestreview-197900640, or mute the thread https://github.com/notifications/unsubscribe-auth/Aswa0OmReJJJ82wGFeTIB6SUCLT219viks5vISBugaJpZM4aZby9 .
I only limited this because it was a description used on bri webpage for the Hub Spotlight section. Greg
On Jan 29, 2019 10:33 PM, "Michael Aldridge" notifications@github.com wrote:
In internal/models/hub.go https://github.com/BESTRobotics/registry/pull/6#discussion_r252113608:
+
- // A director can't do it alone, they have people that help
- // them. These are the admins and can act as the director for
- // the hub in many actions. They can be added and removed by
- // the director.
- Admins []User
- // Location stores the City and State of a hub. Since the hub
- // itself is usually only rooted to a city, this is stored as
- // a string.
- Location string
- // The Description is the blurb for the hub. This is a good
- // place to add a history for the hub or talk about the goals
- // of the specific hub.
- Description string
How long should this field be? Right now its a VARCHAR(255) so it obviously needs to be longer, but by how much.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/BESTRobotics/registry/pull/6#pullrequestreview-197900723, or mute the thread https://github.com/notifications/unsubscribe-auth/Aswa0HVykcVZehKP-OS0eRxg_2R9cnjOks5vISCOgaJpZM4aZby9 .
You should link Team to a school. I always have this trouble with Coach as well. I prefer Team Sponsor or Teacher vs. Coach. We often have a 2nd POC (adult) for the team as well.
Greg
On Tue, Jan 29, 2019, 10:34 PM Michael Aldridge <notifications@github.com wrote:
@the-maldridge commented on this pull request.
In internal/models/team.go https://github.com/BESTRobotics/registry/pull/6#discussion_r252113770:
+type Team struct {
- // A team has exactly one coach. This is the person who the
- // school has designated as responsible for the team's
- // actions, and is the point of contact for delivering
- // paperwork for the team and other key functions.
- Coach User
- // Like the hub director, its rare the coach does it on their
- // own. They have Mentors that help them out to run the team
- // and keep things moving along. Mentors have similar powers
- // in the system to the Coach.
- Mentor []User
- // As stated above, the team must be associated with a school,
- // so that is stored here as a string.
- SchoolName string
I was debating making this a reference to a School in a Schools table, which would allow us to conveniently do things with having multiple teams all parented to the same school. I'm not sure if this is needed, but it deduplicates things like mailing addresses if we need such fields.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/BESTRobotics/registry/pull/6#pullrequestreview-197900921, or mute the thread https://github.com/notifications/unsubscribe-auth/Aswa0O8clpYMeJv525XxsYbPIhe6sXsiks5vISDagaJpZM4aZby9 .
Did I answer these in enough detail? Greg
On Tue, Jan 29, 2019, 10:35 PM Michael Aldridge <notifications@github.com wrote:
I've scattered some comments through the files for things I'm not sure of. I can turn around the API in a day or two once the models are approved.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/BESTRobotics/registry/pull/6#issuecomment-458808689, or mute the thread https://github.com/notifications/unsubscribe-auth/Aswa0DYKjeJNoyyslEPbESFCZocBnagXks5vISD-gaJpZM4aZby9 .
Mostly. Since you're replying via email it hasn't attached your comments to a thread, but I think I've got them matched up. If you use the web interface you can reply directly to the thread that is attached to a particular line of code.
Since it sounds like we should have a school table with records for the school, what besides name and address should be in there?
I was answering by phone so web interface not available. G
On Wed, Jan 30, 2019, 12:17 PM Michael Aldridge <notifications@github.com wrote:
Mostly. Since you're replying via email it hasn't attached your comments to a thread, but I think I've got them matched up. If you use the web interface you can reply directly to the thread that is attached to a particular line of code.
Since it sounds like we should have a school table with records for the school, what besides name and address should be in there?
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/BESTRobotics/registry/pull/6#issuecomment-459051036, or mute the thread https://github.com/notifications/unsubscribe-auth/Aswa0AZdb1R6_EDU7S-m-AikZ9-aDlm6ks5vIeG8gaJpZM4aZby9 .
I am reasonably satisfied with the user handling at the moment, so now its time to dig into the meat of the application.
I'm starting with the team, the season, and the hub since these are fairly loosely coupled. The events themselves and the team roster will come later.