drift-org / backend

2 stars 0 forks source link

Added name to group model + fixed plurality issue #41

Closed aidenszeto closed 3 years ago

aidenszeto commented 3 years ago

Contributors

@aidenszeto

Relevant issue

Closes #37

Summary of change

Testing/Verification

N/A

aidenszeto commented 3 years ago

One thing we should discuss is whether or not we want to make name required - if we leave it optional, we could have it default to the names of the users in the group.

teekenzie commented 3 years ago

One thing we should discuss is whether or not we want to make name required - if we leave it optional, we could have it default to the names of the users in the group.

I was just about to ask about the name being required. The issue with having it default to the names of the users is that the group could be created without any users and thus the name would be an empty string? My take is that it should be required because people might create multiple new groups without users and they are all going to have the same name and is undifferentiable

brandonLi8 commented 3 years ago

I was just about to ask about the name being required. The issue with having it default to the names of the users is that the group could be created without any users and thus the name would be an empty string?

Currently, it's impossible to create a group without users (min=1 validator, and the group has to contain the user that created the group). So I think it's OK to default to the names of the users (messenger model), and we can do that in a separate ticket (update the create group route) as well.

EDIT: But maybe it should be required for the model schema, and the route or the creating function can create the default name if necessary.

teekenzie commented 3 years ago

Currently, it's impossible to create a group without users (min=1 validator, and the group has to contain the user that created the group). So I think it's OK to default to the names of the users (messenger model), and we can do that in a separate ticket (update the create group route) as well.

Consider that the users array is guaranteed to exist, we should update the array to set binding:"required" to also reflect that

aidenszeto commented 3 years ago

name will be required in Group model. Default name implementation will happen in #46