Frontend Build Out #41
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| package pactasrv | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/RMI/pacta/cmd/server/pactasrv/conv" | ||
| "github.com/RMI/pacta/db" | ||
| "github.com/RMI/pacta/oapierr" | ||
| api "github.com/RMI/pacta/openapi/pacta" | ||
| "github.com/RMI/pacta/pacta" | ||
| "go.uber.org/zap" | ||
| ) | ||
|
|
||
| // Creates an initiative invitation | ||
| // (POST /initiative-invitation) | ||
| func (s *Server) CreateInitiativeInvitation(ctx context.Context, request api.CreateInitiativeInvitationRequestObject) (api.CreateInitiativeInvitationResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| ii, err := conv.InitiativeInvitationFromOAPI(request.Body) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| id, err := s.DB.CreateInitiativeInvitation(s.DB.NoTxn(ctx), ii) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to create initiative invitation", zap.Error(err)) | ||
| } | ||
| if id != ii.ID { | ||
| return nil, oapierr.Internal( | ||
| "failed to create initiative invitation: ID mismatch", | ||
| zap.String("requested_id", string(ii.ID)), | ||
| zap.String("actual_id", string(id)), | ||
| ) | ||
| } | ||
| return api.CreateInitiativeInvitation204Response{}, nil | ||
| } | ||
|
|
||
| // Deletes an initiative invitation by id | ||
| // (DELETE /initiative-invitation/{id}) | ||
| func (s *Server) DeleteInitiativeInvitation(ctx context.Context, request api.DeleteInitiativeInvitationRequestObject) (api.DeleteInitiativeInvitationResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| err := s.DB.DeleteInitiativeInvitation(s.DB.NoTxn(ctx), pacta.InitiativeInvitationID(request.Id)) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to delete initiative invitation", zap.Error(err)) | ||
| } | ||
| return api.DeleteInitiativeInvitation204Response{}, nil | ||
| } | ||
|
|
||
| // Returns the initiative invitation from this id, if it exists | ||
| // (GET /initiative-invitation/{id}) | ||
| func (s *Server) GetInitiativeInvitation(ctx context.Context, request api.GetInitiativeInvitationRequestObject) (api.GetInitiativeInvitationResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| ii, err := s.DB.InitiativeInvitation(s.DB.NoTxn(ctx), pacta.InitiativeInvitationID(request.Id)) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to retrieve initiative invitation", zap.Error(err)) | ||
| } | ||
| result, err := conv.InitiativeInvitationToOAPI(ii) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return api.GetInitiativeInvitation200JSONResponse(*result), nil | ||
| } | ||
|
|
||
| // Claims this initiative invitation, if it exists | ||
| // (POST /initiative-invitation/{id}) | ||
|
There was a problem hiding this comment. nit: I think of "claiming an invitation" as a non-CRUD action that should get a custom verb, like
|
||
| func (s *Server) ClaimInitiativeInvitation(ctx context.Context, request api.ClaimInitiativeInvitationRequestObject) (api.ClaimInitiativeInvitationResponseObject, error) { | ||
| userID, err := getUserID(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| err = s.DB.Transactional(ctx, func(tx db.Tx) error { | ||
| ii, err := s.DB.InitiativeInvitation(tx, pacta.InitiativeInvitationID(request.Id)) | ||
| if err != nil { | ||
| return fmt.Errorf("looking up initiative invite: %w", err) | ||
| } | ||
| if !ii.UsedAt.IsZero() || ii.UsedBy != nil { | ||
| if ii.UsedBy != nil && ii.UsedBy.ID == userID { | ||
|
There was a problem hiding this comment. My usual grumbling about new paradigms I don't like: the whole "We use objects for everything and only populate the IDs" makes all of this logic one conditional comparision more than it otherwise would need to be. E.g. as compared to if ii.UsedByID == userID { ... }There was a problem hiding this comment. Yeah I think in most cases you'll come to like this pattern. I've found it increasingly lovely for side projects. |
||
| // We don't return an error if the same user tries to claim the same invitation twice, | ||
| // which might happen by accident, but wouldn't impact the state of the initiative memberships. | ||
| // We may want to log this, though. | ||
|
|
||
| return nil | ||
| } else { | ||
| return fmt.Errorf("initiative is already used: %+v", ii) | ||
|
There was a problem hiding this comment. We should be using oapierr.BadRequest (or similar) for this. For this case in particular, we likely want a custom error code so we can show a specific error to the end user. Then you'd just need to update the error logic below to: if errors.Is(err, oapierr.Error{}) { // or something like that, could also make oapierr.Is(err) or similar
return err
} else if err != nil {
return oapierr.Internal(...)
}You could even make a lil helper, like There was a problem hiding this comment. Done, but through different means, as an external var. |
||
| } | ||
| } | ||
| err = s.DB.UpdateInitiativeInvitation(tx, ii.ID, | ||
| db.SetInitiativeInvitationUsedAt(time.Now()), | ||
| db.SetInitiativeInvitationUsedBy(userID)) | ||
| if err != nil { | ||
| return fmt.Errorf("updating initiative invite: %w", err) | ||
| } | ||
| err = s.DB.UpdateInitiativeUserRelationship(tx, ii.Initiative.ID, userID, | ||
| db.SetInitiativeUserRelationshipMember(true)) | ||
| if err != nil { | ||
| return fmt.Errorf("creating initiative membership: %w", err) | ||
| } | ||
| return nil | ||
| }) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to claim initiative invitation", zap.Error(err)) | ||
| } | ||
| return api.ClaimInitiativeInvitation204Response{}, nil | ||
| } | ||
|
|
||
| // Returns all initiative invitations associated with the initiative | ||
| // (GET /initiative/{id}/invitations) | ||
|
There was a problem hiding this comment. Why is this There was a problem hiding this comment. This is getting the invitations associated with an initiative. I think this is the standard paradigm: resources that have a nested/ownership structure can be requested this way. The ID in this case is the initiative ID. There was a problem hiding this comment. Following up on this, I'd just roll "listing invitations" into There was a problem hiding this comment. We won't want that for public access. I think keeping them distinct will make authz much easier. |
||
| func (s *Server) ListInitiativeInvitations(ctx context.Context, request api.ListInitiativeInvitationsRequestObject) (api.ListInitiativeInvitationsResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| iis, err := s.DB.InitiativeInvitationsByInitiative(s.DB.NoTxn(ctx), pacta.InitiativeID(request.Id)) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to list initiative invitations", zap.Error(err)) | ||
| } | ||
| result, err := dereference(mapAll(iis, conv.InitiativeInvitationToOAPI)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return api.ListInitiativeInvitations200JSONResponse(result), nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package pactasrv | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/RMI/pacta/cmd/server/pactasrv/conv" | ||
| "github.com/RMI/pacta/db" | ||
| "github.com/RMI/pacta/oapierr" | ||
| api "github.com/RMI/pacta/openapi/pacta" | ||
| "github.com/RMI/pacta/pacta" | ||
| "go.uber.org/zap" | ||
| ) | ||
|
|
||
| // Returns all initiative user relationships for the user that the user has access to view | ||
| // (GET /initiative/{id}/user-relationships) | ||
| func (s *Server) ListInitiativeUserRelationshipsByUser(ctx context.Context, request api.ListInitiativeUserRelationshipsByUserRequestObject) (api.ListInitiativeUserRelationshipsByUserResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| iurs, err := s.DB.InitiativeUserRelationshipsByUser(s.DB.NoTxn(ctx), pacta.UserID(request.UserId)) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to retrieve initiative user relationships by user", zap.Error(err)) | ||
| } | ||
| result, err := dereference(mapAll(iurs, conv.InitiativeUserRelationshipToOAPI)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return api.ListInitiativeUserRelationshipsByUser200JSONResponse(result), nil | ||
| } | ||
|
|
||
| // Returns all initiative user relationships for the initiative that the user has access to view | ||
| // (GET /initiative/user-relationships/{id}) | ||
|
There was a problem hiding this comment. This URL scheme is not intuitive to me, e.g. the difference between:
The benefit of RESTful stuff is that usually it's pretty obvious from the URL what the endpoint does, but I'm not getting that from these. How about something like:
There was a problem hiding this comment.
Yep! Public access won't load relationships. I think this actually fits nicely into the "composite key exception" listed here |
||
| func (s *Server) ListInitiativeUserRelationshipsByInitiative(ctx context.Context, request api.ListInitiativeUserRelationshipsByInitiativeRequestObject) (api.ListInitiativeUserRelationshipsByInitiativeResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| iurs, err := s.DB.InitiativeUserRelationshipsByInitiatives(s.DB.NoTxn(ctx), pacta.InitiativeID(request.InitiativeId)) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to retrieve initiative user relationships by user", zap.Error(err)) | ||
| } | ||
| result, err := dereference(mapAll(iurs, conv.InitiativeUserRelationshipToOAPI)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return api.ListInitiativeUserRelationshipsByInitiative200JSONResponse(result), nil | ||
| } | ||
|
|
||
| // Returns the initiative user relationship from this id, if it exists | ||
| // (GET /initiative/{initiativeId}/user-relationship/{userId}) | ||
|
There was a problem hiding this comment. This API surface seems really broad to me, what's the motivation for having three different endpoints that return a user's relationship to an initiative? Unless we expect the list of relationships to be exceptionally large (>1,000), I'd just let the client filter for the stuff they're looking for. More API surface means more stuff to secure, and more DB functions/tests/code to maintain overall. There was a problem hiding this comment. Yes, they will be large. There is a public initiative, which anyone will have the ability to join, and may be segmented by region (i.e. can't be handled as a standalone case). |
||
| func (s *Server) GetInitiativeUserRelationship(ctx context.Context, request api.GetInitiativeUserRelationshipRequestObject) (api.GetInitiativeUserRelationshipResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| iur, err := s.DB.InitiativeUserRelationship(s.DB.NoTxn(ctx), pacta.InitiativeID(request.InitiativeId), pacta.UserID(request.UserId)) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to retrieve initiative user relationship", zap.Error(err)) | ||
| } | ||
| result, err := conv.InitiativeUserRelationshipToOAPI(iur) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return api.GetInitiativeUserRelationship200JSONResponse(*result), nil | ||
| } | ||
|
|
||
| // Updates initiative user relationship properties | ||
| // (PATCH /initiative/{initiativeId}/user-relationship/{userId}) | ||
| func (s *Server) UpdateInitiativeUserRelationship(ctx context.Context, request api.UpdateInitiativeUserRelationshipRequestObject) (api.UpdateInitiativeUserRelationshipResponseObject, error) { | ||
| // TODO(#12) Implement Authorization | ||
| mutations := []db.UpdateInitiativeUserRelationshipFn{} | ||
| if request.Body.Manager != nil { | ||
| mutations = append(mutations, db.SetInitiativeUserRelationshipManager(*request.Body.Manager)) | ||
| } | ||
| if request.Body.Member != nil { | ||
| mutations = append(mutations, db.SetInitiativeUserRelationshipMember(*request.Body.Member)) | ||
| } | ||
| err := s.DB.UpdateInitiativeUserRelationship(s.DB.NoTxn(ctx), pacta.InitiativeID(request.InitiativeId), pacta.UserID(request.UserId), mutations...) | ||
| if err != nil { | ||
| return nil, oapierr.Internal("failed to update initiative user relationship", zap.Error(err)) | ||
| } | ||
| return api.UpdateInitiativeUserRelationship204Response{}, nil | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.