Skip to content

Conversation

@ezstarr
Copy link
Collaborator

@ezstarr ezstarr commented Nov 11, 2024

Hi Kroden and @EvieePy, looking to merge this branch. It's main purpose: create and edit team info and membership. Let me know if you have questions or if there's anything I need to fix! There's a small mismatch between backend and frontend in the TeamEdit file, but it still runs. Let me know if I should create a separate model for the teamMember in the frontend. The conflict is that teamMember property names are based on the team_member table fields, but the data is gathered from the users table fields.

EvieePy and others added 30 commits June 25, 2024 11:07
Co-authored-by: Mysty <evieepy@gmail.com>
@KrodenDev
Copy link
Collaborator

You'll want to resolve those conflicts first. Probably just a missed a merge from main.

Copy link
Collaborator

@KrodenDev KrodenDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you'd prefer, we can make the security issues separate tasks to address if you'd prefer to just merge this in now and fix those later.

Availability: item.Availability,
Description: item.Description,
CreatedOn: item.CreatedOn,
InviteCode: item.InviteCode,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't send InviteCode back for all teams as anybody could look at the JSON responses and see the invite code for any other team.

@@ -0,0 +1,139 @@
package database
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a mandatory change, but I would consider splitting the UI and DB structs into different files. With the DB structs under the database package and the UI structs under the "server" package.

for _, team := range teamMap {
result = append(result, *team)
}
fmt.Println(result)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove debug statements for a final build. Or at least make them logger debug statements if they might be helpful to still have locally while testing.

VALUES ($1, $2, $3)
RETURNING user_id, team_id, team_role`, userId, teamUUID, role)
if err != nil {
fmt.Println(err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any error logging should use logger.Error() so it's captured correctly.

if err == nil {
ctx.JSON(http.StatusOK, teams)
} else {
fmt.Println("ERROR: ", err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logger.Error

teamResponse, err := server.GetTeamInfo(id)

if err != nil {
ctx.Status(http.StatusBadRequest)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be StatusInternalServerError, and there should also be a logger.Error()

uuidTeamId := convert.StringToUUID(MemberPayload.TeamId)
uuidMemberId := convert.StringToUUID(MemberPayload.MemberId)

removedMember, err := database.RemoveTeamMember(uuidTeamId, uuidMemberId)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For security we'll want to make sure the current user is the team owner and has permission to remove the user. Otherwise anybody could call this and remove users.

}
team, err = database.UpdateTeam(team)

updatedTeam, err := database.UpdateTeam(team)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should verify that user is owner of team and is allowed to make updates

let teamId: string = '';
$: currUserId = $userStore?.Id;

interface ErrorResponse {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

$: currUserId = $userStore?.Id;
console.log('currUserId (teamsbrowse.svelte):', currUserId);

interface ErrorResponse {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this (again)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants