From 4a3a2baa2454fafe02eb83df16330a8210074cbe Mon Sep 17 00:00:00 2001 From: Grady Ward Date: Tue, 19 Dec 2023 13:56:16 -0700 Subject: [PATCH 1/3] P-I Membership BE --- cmd/server/pactasrv/BUILD.bazel | 1 + cmd/server/pactasrv/conv/pacta_to_oapi.go | 94 +++++++++++--- cmd/server/pactasrv/initiative.go | 5 + .../initiative_portfolio_relationship.go | 43 ++++++ cmd/server/pactasrv/populate.go | 27 +++- cmd/server/pactasrv/portfolio.go | 6 + db/sqldb/portfolio.go | 62 +++++++-- db/sqldb/portfolio_group.go | 2 +- db/sqldb/portfolio_group_test.go | 2 +- db/sqldb/snapshot.go | 2 +- db/sqldb/sqldb.go | 9 ++ openapi/pacta.yaml | 86 ++++++++++++ pacta/pacta.go | 122 +++++++++--------- 13 files changed, 367 insertions(+), 94 deletions(-) create mode 100644 cmd/server/pactasrv/initiative_portfolio_relationship.go diff --git a/cmd/server/pactasrv/BUILD.bazel b/cmd/server/pactasrv/BUILD.bazel index 43c06f6..a9277b3 100644 --- a/cmd/server/pactasrv/BUILD.bazel +++ b/cmd/server/pactasrv/BUILD.bazel @@ -6,6 +6,7 @@ go_library( "incomplete_upload.go", "initiative.go", "initiative_invitation.go", + "initiative_portfolio_relationship.go", "initiative_user_relationship.go", "pacta_version.go", "pactasrv.go", diff --git a/cmd/server/pactasrv/conv/pacta_to_oapi.go b/cmd/server/pactasrv/conv/pacta_to_oapi.go index dc27050..8c739e6 100644 --- a/cmd/server/pactasrv/conv/pacta_to_oapi.go +++ b/cmd/server/pactasrv/conv/pacta_to_oapi.go @@ -11,21 +11,78 @@ func InitiativeToOAPI(i *pacta.Initiative) (*api.Initiative, error) { if i == nil { return nil, oapierr.Internal("initiativeToOAPI: can't convert nil pointer") } + pims, err := convAll(i.PortfolioInitiativeMemberships, portfolioInitiativeMembershipToOAPIPortfolio) + if err != nil { + return nil, oapierr.Internal("initiativeToOAPI: portfolioInitiativeMembershipToOAPIInitiative failed", zap.Error(err)) + } return &api.Initiative{ - Affiliation: i.Affiliation, - CreatedAt: i.CreatedAt, - Id: string(i.ID), - InternalDescription: i.InternalDescription, - IsAcceptingNewMembers: i.IsAcceptingNewMembers, - IsAcceptingNewPortfolios: i.IsAcceptingNewPortfolios, - Language: api.InitiativeLanguage(i.Language), - Name: i.Name, - PactaVersion: ptr(string(i.PACTAVersion.ID)), - PublicDescription: i.PublicDescription, - RequiresInvitationToJoin: i.RequiresInvitationToJoin, + Affiliation: i.Affiliation, + CreatedAt: i.CreatedAt, + Id: string(i.ID), + InternalDescription: i.InternalDescription, + IsAcceptingNewMembers: i.IsAcceptingNewMembers, + IsAcceptingNewPortfolios: i.IsAcceptingNewPortfolios, + Language: api.InitiativeLanguage(i.Language), + Name: i.Name, + PactaVersion: ptr(string(i.PACTAVersion.ID)), + PublicDescription: i.PublicDescription, + RequiresInvitationToJoin: i.RequiresInvitationToJoin, + PortfolioInitiativeMemberships: pims, }, nil } +func userIsPopulated(u *pacta.User) bool { + return u != nil && u.ID != "" && *u != (pacta.User{ID: u.ID}) +} + +func portfolioInitiativeMembershipToOAPIPortfolio(in *pacta.PortfolioInitiativeMembership) (api.PortfolioInitiativeMembershipPortfolio, error) { + var zero api.PortfolioInitiativeMembershipPortfolio + out := &api.PortfolioInitiativeMembershipPortfolio{ + CreatedAt: in.CreatedAt, + } + if in.AddedBy != nil && in.AddedBy.ID == "" { + out.AddedByUserId = ptr(string(in.AddedBy.ID)) + if userIsPopulated(in.AddedBy) { + u, err := UserToOAPI(in.AddedBy) + if err != nil { + return zero, oapierr.Internal("portfolioInitiativeMembershipToOAPI: userToOAPI failed", zap.Error(err)) + } + out.AddedByUser = u + } + } + p, err := PortfolioToOAPI(in.Portfolio) + if err != nil { + return zero, oapierr.Internal("portfolioInitiativeMembershipToOAPI: portfolioToOAPI failed", zap.Error(err)) + } + out.Portfolio = *p + return zero, nil +} + +func portfolioInitiativeMembershipToOAPIInitiative(in *pacta.PortfolioInitiativeMembership) (api.PortfolioInitiativeMembershipInitiative, error) { + var zero api.PortfolioInitiativeMembershipInitiative + out := api.PortfolioInitiativeMembershipInitiative{ + CreatedAt: in.CreatedAt, + } + if in.AddedBy != nil && in.AddedBy.ID == "" { + out.AddedByUserId = ptr(string(in.AddedBy.ID)) + if userIsPopulated(in.AddedBy) { + u, err := UserToOAPI(in.AddedBy) + if err != nil { + return zero, oapierr.Internal("portfolioInitiativeMembershipToOAPI: userToOAPI failed", zap.Error(err)) + } + out.AddedByUser = u + } + } + if in.Initiative != nil { + i, err := InitiativeToOAPI(in.Initiative) + if err != nil { + return zero, oapierr.Internal("portfolioInitiativeMembershipToOAPI: initiativeToOAPI failed", zap.Error(err)) + } + out.Initiative = *i + } + return out, nil +} + func UserToOAPI(user *pacta.User) (*api.User, error) { if user == nil { return nil, oapierr.Internal("userToOAPI: can't convert nil pointer") @@ -142,17 +199,21 @@ func PortfolioToOAPI(p *pacta.Portfolio) (*api.Portfolio, error) { if err != nil { return nil, oapierr.Internal("portfolioToOAPI: holdingsDateToOAPI failed", zap.Error(err)) } - memberOfs := []api.PortfolioGroupMembershipPortfolioGroup{} - for _, m := range p.MemberOf { + portfolioGroupMemberships := []api.PortfolioGroupMembershipPortfolioGroup{} + for _, m := range p.PortfolioGroupMemberships { pg, err := PortfolioGroupToOAPI(m.PortfolioGroup) if err != nil { return nil, oapierr.Internal("portfolioToOAPI: portfolioGroupToOAPI failed", zap.Error(err)) } - memberOfs = append(memberOfs, api.PortfolioGroupMembershipPortfolioGroup{ + portfolioGroupMemberships = append(portfolioGroupMemberships, api.PortfolioGroupMembershipPortfolioGroup{ CreatedAt: m.CreatedAt, PortfolioGroup: *pg, }) } + pims, err := convAll(p.PortfolioInitiativeMemberships, portfolioInitiativeMembershipToOAPIInitiative) + if err != nil { + return nil, oapierr.Internal("initiativeToOAPI: portfolioInitiativeMembershipToOAPIInitiative failed", zap.Error(err)) + } return &api.Portfolio{ Id: string(p.ID), Name: p.Name, @@ -161,7 +222,8 @@ func PortfolioToOAPI(p *pacta.Portfolio) (*api.Portfolio, error) { CreatedAt: p.CreatedAt, NumberOfRows: p.NumberOfRows, AdminDebugEnabled: p.AdminDebugEnabled, - Groups: &memberOfs, + Groups: &portfolioGroupMemberships, + Initiatives: &pims, }, nil } @@ -170,7 +232,7 @@ func PortfolioGroupToOAPI(pg *pacta.PortfolioGroup) (*api.PortfolioGroup, error) return nil, oapierr.Internal("portfolioGroupToOAPI: can't convert nil pointer") } members := []api.PortfolioGroupMembershipPortfolio{} - for _, m := range pg.Members { + for _, m := range pg.PortfolioGroupMemberships { portfolio, err := PortfolioToOAPI(m.Portfolio) if err != nil { return nil, oapierr.Internal("portfolioGroupToOAPI: portfolioToOAPI failed", zap.Error(err)) diff --git a/cmd/server/pactasrv/initiative.go b/cmd/server/pactasrv/initiative.go index 5d8ad4a..711b62e 100644 --- a/cmd/server/pactasrv/initiative.go +++ b/cmd/server/pactasrv/initiative.go @@ -93,6 +93,11 @@ func (s *Server) FindInitiativeById(ctx context.Context, request api.FindInitiat } return nil, oapierr.Internal("failed to load initiative", zap.String("initiative_id", request.Id), zap.Error(err)) } + portfolios, err := s.DB.PortfolioInitiativeMembershipsByInitiative(s.DB.NoTxn(ctx), i.ID) + if err != nil { + return nil, oapierr.Internal("failed to load portfolios for initiative", zap.String("initiative_id", string(i.ID)), zap.Error(err)) + } + i.PortfolioInitiativeMemberships = portfolios resp, err := conv.InitiativeToOAPI(i) if err != nil { return nil, err diff --git a/cmd/server/pactasrv/initiative_portfolio_relationship.go b/cmd/server/pactasrv/initiative_portfolio_relationship.go new file mode 100644 index 0000000..f4153a1 --- /dev/null +++ b/cmd/server/pactasrv/initiative_portfolio_relationship.go @@ -0,0 +1,43 @@ +package pactasrv + +import ( + "context" + + "github.com/RMI/pacta/oapierr" + api "github.com/RMI/pacta/openapi/pacta" + "github.com/RMI/pacta/pacta" + "go.uber.org/zap" +) + +// creates an initiative portfolio relationship +// (POST /initiative/{initiativeId}/portfolio-relationship/{portfolioId}) +func (s *Server) CreateInitiativePortfolioRelationship(ctx context.Context, request api.CreateInitiativePortfolioRelationshipRequestObject) (api.CreateInitiativePortfolioRelationshipResponseObject, error) { + // TODO(#12) Implement Authorization + userID, err := getUserID(ctx) + if err != nil { + return nil, err + } + err = s.DB.CreatePortfolioInitiativeMembership(s.DB.NoTxn(ctx), &pacta.PortfolioInitiativeMembership{ + Portfolio: &pacta.Portfolio{ID: pacta.PortfolioID(request.PortfolioId)}, + Initiative: &pacta.Initiative{ID: pacta.InitiativeID(request.InitiativeId)}, + AddedBy: &pacta.User{ID: userID}, + }) + if err != nil { + return nil, oapierr.Internal("failed to create initiative", zap.Error(err)) + } + return api.CreateInitiativePortfolioRelationship204Response{}, nil +} + +// Deletes an initiative:portfolio relationship +// (DELETE /initiative/{initiativeId}/portfolio-relationship/{portfolioId}) +func (s *Server) DeleteInitiativePortfolioRelationship(ctx context.Context, request api.DeleteInitiativePortfolioRelationshipRequestObject) (api.DeleteInitiativePortfolioRelationshipResponseObject, error) { + // TODO(#12) Implement Authorization + err := s.DB.DeletePortfolioInitiativeMembership(s.DB.NoTxn(ctx), pacta.PortfolioID(request.PortfolioId), pacta.InitiativeID(request.InitiativeId)) + if err != nil { + return nil, oapierr.Internal("failed to create initiative-portfolio relationship", + zap.String("initiative_id", request.InitiativeId), + zap.String("portfolio_id", request.PortfolioId), + zap.Error(err)) + } + return api.DeleteInitiativePortfolioRelationship204Response{}, nil +} diff --git a/cmd/server/pactasrv/populate.go b/cmd/server/pactasrv/populate.go index 7ee5af4..2176f7b 100644 --- a/cmd/server/pactasrv/populate.go +++ b/cmd/server/pactasrv/populate.go @@ -15,7 +15,7 @@ func (s *Server) populatePortfoliosInPortfolioGroups( ) error { getFn := func(pg *pacta.PortfolioGroup) ([]*pacta.Portfolio, error) { result := []*pacta.Portfolio{} - for _, member := range pg.Members { + for _, member := range pg.PortfolioGroupMemberships { result = append(result, member.Portfolio) } return result, nil @@ -32,13 +32,36 @@ func (s *Server) populatePortfoliosInPortfolioGroups( return nil } +func (s *Server) populateInitiativesInPortfolios( + ctx context.Context, + is []*pacta.Portfolio, +) error { + getFn := func(pg *pacta.Portfolio) ([]*pacta.Initiative, error) { + result := []*pacta.Initiative{} + for _, member := range pg.PortfolioInitiativeMemberships { + result = append(result, member.Initiative) + } + return result, nil + } + lookupFn := func(ids []pacta.InitiativeID) (map[pacta.InitiativeID]*pacta.Initiative, error) { + return s.DB.Initiatives(s.DB.NoTxn(ctx), ids) + } + getIDFn := func(p *pacta.Initiative) pacta.InitiativeID { + return p.ID + } + if err := populateAll(is, getFn, getIDFn, lookupFn); err != nil { + return oapierr.Internal("populating initiatives in portfolios failed", zap.Error(err)) + } + return nil +} + func (s *Server) populatePortfolioGroupsInPortfolios( ctx context.Context, ts []*pacta.Portfolio, ) error { getFn := func(pg *pacta.Portfolio) ([]*pacta.PortfolioGroup, error) { result := []*pacta.PortfolioGroup{} - for _, member := range pg.MemberOf { + for _, member := range pg.PortfolioGroupMemberships { result = append(result, member.PortfolioGroup) } return result, nil diff --git a/cmd/server/pactasrv/portfolio.go b/cmd/server/pactasrv/portfolio.go index 8520d3e..7cd06f9 100644 --- a/cmd/server/pactasrv/portfolio.go +++ b/cmd/server/pactasrv/portfolio.go @@ -27,6 +27,9 @@ func (s *Server) ListPortfolios(ctx context.Context, request api.ListPortfoliosR if err := s.populatePortfolioGroupsInPortfolios(ctx, ps); err != nil { return nil, err } + if err := s.populateInitiativesInPortfolios(ctx, ps); err != nil { + return nil, err + } items, err := dereference(conv.PortfoliosToOAPI(ps)) if err != nil { return nil, err @@ -62,6 +65,9 @@ func (s *Server) FindPortfolioById(ctx context.Context, request api.FindPortfoli if err := s.populatePortfolioGroupsInPortfolios(ctx, []*pacta.Portfolio{p}); err != nil { return nil, err } + if err := s.populateInitiativesInPortfolios(ctx, []*pacta.Portfolio{p}); err != nil { + return nil, err + } converted, err := conv.PortfolioToOAPI(p) if err != nil { return nil, err diff --git a/db/sqldb/portfolio.go b/db/sqldb/portfolio.go index 5ee12cf..13b2393 100644 --- a/db/sqldb/portfolio.go +++ b/db/sqldb/portfolio.go @@ -24,9 +24,14 @@ func portfolioQueryStanza(where string) string { portfolio.number_of_rows, ARRAY_AGG(portfolio_group_membership.portfolio_group_id), ARRAY_AGG(portfolio_group_membership.created_at) + ARRAY_AGG(portfolio_initiative_membership.initiative_id), + ARRAY_AGG(portfolio_initiative_membership.added_by_user_id), + ARRAY_AGG(portfolio_initiative_membership.created_at) FROM portfolio LEFT JOIN portfolio_group_membership ON portfolio_group_membership.portfolio_id = portfolio.id + LEFT JOIN portfolio_initiative_membership + ON portfolio_initiative_membership.portfolio_id = portfolio.id %s GROUP BY portfolio.id;`, where) } @@ -159,8 +164,11 @@ func (d *DB) DeletePortfolio(tx db.Tx, id pacta.PortfolioID) ([]pacta.BlobURI, e func rowToPortfolio(row rowScanner) (*pacta.Portfolio, error) { p := &pacta.Portfolio{Owner: &pacta.Owner{}, Blob: &pacta.Blob{}} hd := pgtype.Timestamptz{} - mid := []pgtype.Text{} - mca := []pgtype.Timestamptz{} + groupsIDs := []pgtype.Text{} + groupsCreatedAts := []pgtype.Timestamptz{} + initiativesIDs := []pgtype.Text{} + initiativesAddedByIDs := []pgtype.Text{} + initiativesCreatedAts := []pgtype.Timestamptz{} err := row.Scan( &p.ID, &p.Owner.ID, @@ -171,8 +179,11 @@ func rowToPortfolio(row rowScanner) (*pacta.Portfolio, error) { &p.Blob.ID, &p.AdminDebugEnabled, &p.NumberOfRows, - &mid, - &mca, + &groupsIDs, + &groupsCreatedAts, + &initiativesIDs, + &initiativesAddedByIDs, + &initiativesCreatedAts, ) if err != nil { return nil, fmt.Errorf("scanning into portfolio row: %w", err) @@ -181,24 +192,49 @@ func rowToPortfolio(row rowScanner) (*pacta.Portfolio, error) { if err != nil { return nil, fmt.Errorf("decoding holdings date: %w", err) } - if len(mid) != len(mca) { - return nil, fmt.Errorf("portfolio group membership ids and created ats must be the same length") + if err := checkSizesEquivalent("groups", len(groupsIDs), len(groupsCreatedAts)); err != nil { + return nil, err } - for i := range mid { - if !mid[i].Valid && !mca[i].Valid { + for i := range groupsIDs { + if !groupsIDs[i].Valid && !groupsCreatedAts[i].Valid { continue // skip nulls } - if !mid[i].Valid { + if !groupsIDs[i].Valid { return nil, fmt.Errorf("portfolio group membership ids must be non-null") } - if !mca[i].Valid { + if !groupsCreatedAts[i].Valid { return nil, fmt.Errorf("portfolio group membership createdAt must be non-null") } - p.MemberOf = append(p.MemberOf, &pacta.PortfolioGroupMembership{ + p.PortfolioGroupMemberships = append(p.PortfolioGroupMemberships, &pacta.PortfolioGroupMembership{ PortfolioGroup: &pacta.PortfolioGroup{ - ID: pacta.PortfolioGroupID(mid[i].String), + ID: pacta.PortfolioGroupID(groupsIDs[i].String), }, - CreatedAt: mca[i].Time, + CreatedAt: groupsCreatedAts[i].Time, + }) + } + if err := checkSizesEquivalent("initiatives", len(initiativesIDs), len(initiativesAddedByIDs), len(initiativesCreatedAts)); err != nil { + return nil, err + } + for i := range initiativesIDs { + if !initiativesIDs[i].Valid && !initiativesCreatedAts[i].Valid { + continue // skip nulls + } + if !groupsIDs[i].Valid { + return nil, fmt.Errorf("portfolio group membership ids must be non-null") + } + if !groupsCreatedAts[i].Valid { + return nil, fmt.Errorf("portfolio group membership createdAt must be non-null") + } + var addedBy *pacta.User + if initiativesAddedByIDs[i].Valid { + addedBy = &pacta.User{ID: pacta.UserID(initiativesAddedByIDs[i].String)} + } + p.PortfolioInitiativeMemberships = append(p.PortfolioInitiativeMemberships, &pacta.PortfolioInitiativeMembership{ + Initiative: &pacta.Initiative{ + ID: pacta.InitiativeID(initiativesIDs[i].String), + }, + CreatedAt: groupsCreatedAts[i].Time, + AddedBy: addedBy, }) } return p, nil diff --git a/db/sqldb/portfolio_group.go b/db/sqldb/portfolio_group.go index b0d26b2..23cf962 100644 --- a/db/sqldb/portfolio_group.go +++ b/db/sqldb/portfolio_group.go @@ -148,7 +148,7 @@ func rowToPortfolioGroup(row rowScanner) (*pacta.PortfolioGroup, error) { if !mca[i].Valid { return nil, fmt.Errorf("portfolio group membership createdAt must be non-null") } - p.Members = append(p.Members, &pacta.PortfolioGroupMembership{ + p.PortfolioGroupMemberships = append(p.PortfolioGroupMemberships, &pacta.PortfolioGroupMembership{ Portfolio: &pacta.Portfolio{ ID: pacta.PortfolioID(mid[i].String), }, diff --git a/db/sqldb/portfolio_group_test.go b/db/sqldb/portfolio_group_test.go index 3610d66..d9c9cb3 100644 --- a/db/sqldb/portfolio_group_test.go +++ b/db/sqldb/portfolio_group_test.go @@ -109,7 +109,7 @@ func TestPortfolioGroupCRUD(t *testing.T) { } expectedP1 := p1.Clone() - expectedP1.MemberOf = []*pacta.PortfolioGroupMembership{{ + expectedP1.PortfolioGroupMemberships = []*pacta.PortfolioGroupMembership{{ PortfolioGroup: &pacta.PortfolioGroup{ID: pg1.ID}, CreatedAt: time.Now(), }, { diff --git a/db/sqldb/snapshot.go b/db/sqldb/snapshot.go index 8bb88f1..74b52fd 100644 --- a/db/sqldb/snapshot.go +++ b/db/sqldb/snapshot.go @@ -20,7 +20,7 @@ func (d *DB) CreateSnapshotOfPortfolioGroup(tx db.Tx, pgID pacta.PortfolioGroupI return "", fmt.Errorf("reading portfolio group: %w", err) } ids := []pacta.PortfolioID{} - for _, p := range pg.Members { + for _, p := range pg.PortfolioGroupMemberships { ids = append(ids, p.Portfolio.ID) } return d.createSnapshot(tx, "", pgID, "", ids) diff --git a/db/sqldb/sqldb.go b/db/sqldb/sqldb.go index 32fcd2c..adaab07 100644 --- a/db/sqldb/sqldb.go +++ b/db/sqldb/sqldb.go @@ -389,3 +389,12 @@ func mapRowsToIDs[T ~string](name string, rows pgx.Rows) ([]T, error) { } return mapRows(name, rows, fn) } + +func checkSizesEquivalent(name string, sizes ...int) error { + for i := 1; i < len(sizes); i++ { + if sizes[i] != sizes[0] { + return fmt.Errorf("array_agg at %q results are not the same size: len([%d])=%d but len([%d])=%d", name, i, sizes[i], 0, sizes[0]) + } + } + return nil +} diff --git a/openapi/pacta.yaml b/openapi/pacta.yaml index 12c6a45..e4f34d6 100644 --- a/openapi/pacta.yaml +++ b/openapi/pacta.yaml @@ -371,6 +371,47 @@ paths: responses: '204': description: the relationship changes were applied successfully + /initiative/{initiativeId}/portfolio-relationship/{portfolioId}: + post: + summary: creates an initiative portfolio relationship + description: creates a membership relationship between the portfolio and the initiative + operationId: createInitiativePortfolioRelationship + parameters: + - name: initiativeId + in: path + description: ID of the initiative + required: true + schema: + type: string + - name: portfolioId + in: path + description: ID of the portfolio + required: true + schema: + type: string + responses: + '204': + description: the relationship was created successfully + delete: + summary: Deletes an initiative:portfolio relationship + description: Deletes a given portfolio's relationship with a given initiative + operationId: deleteInitiativePortfolioRelationship + parameters: + - name: initiativeId + in: path + description: ID of the initiative + required: true + schema: + type: string + - name: portfolioId + in: path + description: ID of the portfolio + required: true + schema: + type: string + responses: + '204': + description: the relationship was deleted, if it existed /portfolio-groups: get: summary: Returns the portfolio groups that the user has access to @@ -915,6 +956,40 @@ components: description: type: string description: the description of the contents or purpose of the portfolio group + PortfolioInitiativeMembershipPortfolio: + type: object + required: + - createdAt + - portfolio + properties: + addedByUserId: + type: string + description: the user that added this portfolio to the initiative + addedByUser: + $ref: '#/components/schemas/User' + portfolio: + $ref: '#/components/schemas/Portfolio' + createdAt: + type: string + format: date-time + description: The time at which this relationship was created + PortfolioInitiativeMembershipInitiative: + type: object + required: + - createdAt + - initiative + properties: + addedByUserId: + type: string + description: the user that added this portfolio to the initiative + addedByUser: + $ref: '#/components/schemas/User' + initiative: + $ref: '#/components/schemas/Initiative' + createdAt: + type: string + format: date-time + description: The time at which this relationship was created InitiativeCreate: type: object required: @@ -967,6 +1042,7 @@ components: - pactaVersionId - language - createdAt + - portfolioInitiativeMemberships properties: id: type: string @@ -999,6 +1075,11 @@ components: pactaVersion: type: string description: The pacta model that this initiative should use, if not specified, the default pacta model will be used. + portfolioInitiativeMemberships: + type: array + description: the list of portfolios that are members of this initiative + items: + $ref: '#/components/schemas/PortfolioInitiativeMembershipPortfolio' createdAt: type: string format: date-time @@ -1328,6 +1409,11 @@ components: description: The list of portfolio groups that this portfolio is a member of items: $ref: '#/components/schemas/PortfolioGroupMembershipPortfolioGroup' + initiatives: + type: array + description: The list of initiatives that this portfolio is a member of + items: + $ref: '#/components/schemas/PortfolioInitiativeMembershipInitiative' PortfolioChanges: type: object properties: diff --git a/pacta/pacta.go b/pacta/pacta.go index 8a4f223..8861cb6 100644 --- a/pacta/pacta.go +++ b/pacta/pacta.go @@ -117,20 +117,20 @@ func (o *User) Clone() *User { type InitiativeID string type Initiative struct { - ID InitiativeID - Name string - Affiliation string - PublicDescription string - InternalDescription string - RequiresInvitationToJoin bool - IsAcceptingNewMembers bool - IsAcceptingNewPortfolios bool - PACTAVersion *PACTAVersion - Language Language - CreatedAt time.Time - UserRelationships []*InitiativeUserRelationship - PortfolioRelationships []*PortfolioInitiativeMembership - Invitations []*InitiativeInvitation + ID InitiativeID + Name string + Affiliation string + PublicDescription string + InternalDescription string + RequiresInvitationToJoin bool + IsAcceptingNewMembers bool + IsAcceptingNewPortfolios bool + PACTAVersion *PACTAVersion + Language Language + CreatedAt time.Time + InitiativeUserRelationships []*InitiativeUserRelationship + PortfolioInitiativeMemberships []*PortfolioInitiativeMembership + Invitations []*InitiativeInvitation } func (o *Initiative) Clone() *Initiative { @@ -138,20 +138,20 @@ func (o *Initiative) Clone() *Initiative { return nil } return &Initiative{ - ID: o.ID, - Name: o.Name, - Affiliation: o.Affiliation, - PublicDescription: o.PublicDescription, - InternalDescription: o.InternalDescription, - RequiresInvitationToJoin: o.RequiresInvitationToJoin, - IsAcceptingNewMembers: o.IsAcceptingNewMembers, - IsAcceptingNewPortfolios: o.IsAcceptingNewPortfolios, - PACTAVersion: o.PACTAVersion.Clone(), - Language: o.Language, - CreatedAt: o.CreatedAt, - UserRelationships: cloneAll(o.UserRelationships), - PortfolioRelationships: cloneAll(o.PortfolioRelationships), - Invitations: cloneAll(o.Invitations), + ID: o.ID, + Name: o.Name, + Affiliation: o.Affiliation, + PublicDescription: o.PublicDescription, + InternalDescription: o.InternalDescription, + RequiresInvitationToJoin: o.RequiresInvitationToJoin, + IsAcceptingNewMembers: o.IsAcceptingNewMembers, + IsAcceptingNewPortfolios: o.IsAcceptingNewPortfolios, + PACTAVersion: o.PACTAVersion.Clone(), + Language: o.Language, + CreatedAt: o.CreatedAt, + InitiativeUserRelationships: cloneAll(o.InitiativeUserRelationships), + PortfolioInitiativeMemberships: cloneAll(o.PortfolioInitiativeMemberships), + Invitations: cloneAll(o.Invitations), } } @@ -347,16 +347,17 @@ func (o *IncompleteUpload) Clone() *IncompleteUpload { type PortfolioID string type Portfolio struct { - ID PortfolioID - Name string - Description string - CreatedAt time.Time - HoldingsDate *HoldingsDate - Owner *Owner - Blob *Blob - AdminDebugEnabled bool - NumberOfRows int - MemberOf []*PortfolioGroupMembership + ID PortfolioID + Name string + Description string + CreatedAt time.Time + HoldingsDate *HoldingsDate + Owner *Owner + Blob *Blob + AdminDebugEnabled bool + NumberOfRows int + PortfolioGroupMemberships []*PortfolioGroupMembership + PortfolioInitiativeMemberships []*PortfolioInitiativeMembership } func (o *Portfolio) Clone() *Portfolio { @@ -364,27 +365,28 @@ func (o *Portfolio) Clone() *Portfolio { return nil } return &Portfolio{ - ID: o.ID, - Name: o.Name, - Description: o.Description, - CreatedAt: o.CreatedAt, - HoldingsDate: o.HoldingsDate.Clone(), - Owner: o.Owner.Clone(), - Blob: o.Blob.Clone(), - AdminDebugEnabled: o.AdminDebugEnabled, - NumberOfRows: o.NumberOfRows, - MemberOf: cloneAll(o.MemberOf), + ID: o.ID, + Name: o.Name, + Description: o.Description, + CreatedAt: o.CreatedAt, + HoldingsDate: o.HoldingsDate.Clone(), + Owner: o.Owner.Clone(), + Blob: o.Blob.Clone(), + AdminDebugEnabled: o.AdminDebugEnabled, + NumberOfRows: o.NumberOfRows, + PortfolioGroupMemberships: cloneAll(o.PortfolioGroupMemberships), + PortfolioInitiativeMemberships: cloneAll(o.PortfolioInitiativeMemberships), } } type PortfolioGroupID string type PortfolioGroup struct { - ID PortfolioGroupID - Owner *Owner - Name string - Description string - CreatedAt time.Time - Members []*PortfolioGroupMembership + ID PortfolioGroupID + Owner *Owner + Name string + Description string + CreatedAt time.Time + PortfolioGroupMemberships []*PortfolioGroupMembership } func (o *PortfolioGroup) Clone() *PortfolioGroup { @@ -392,12 +394,12 @@ func (o *PortfolioGroup) Clone() *PortfolioGroup { return nil } return &PortfolioGroup{ - ID: o.ID, - Owner: o.Owner.Clone(), - Name: o.Name, - Description: o.Description, - CreatedAt: o.CreatedAt, - Members: cloneAll(o.Members), + ID: o.ID, + Owner: o.Owner.Clone(), + Name: o.Name, + Description: o.Description, + CreatedAt: o.CreatedAt, + PortfolioGroupMemberships: cloneAll(o.PortfolioGroupMemberships), } } From 7200460e67fba8b17c2ff8cae224346f057ccd08 Mon Sep 17 00:00:00 2001 From: Grady Ward Date: Tue, 19 Dec 2023 14:01:46 -0700 Subject: [PATCH 2/3] Self Review --- cmd/server/pactasrv/conv/pacta_to_oapi.go | 18 ------------------ db/sqldb/portfolio.go | 2 +- db/sqldb/portfolio_group_test.go | 12 ++++++------ openapi/pacta.yaml | 4 ---- 4 files changed, 7 insertions(+), 29 deletions(-) diff --git a/cmd/server/pactasrv/conv/pacta_to_oapi.go b/cmd/server/pactasrv/conv/pacta_to_oapi.go index 8c739e6..e846dab 100644 --- a/cmd/server/pactasrv/conv/pacta_to_oapi.go +++ b/cmd/server/pactasrv/conv/pacta_to_oapi.go @@ -31,10 +31,6 @@ func InitiativeToOAPI(i *pacta.Initiative) (*api.Initiative, error) { }, nil } -func userIsPopulated(u *pacta.User) bool { - return u != nil && u.ID != "" && *u != (pacta.User{ID: u.ID}) -} - func portfolioInitiativeMembershipToOAPIPortfolio(in *pacta.PortfolioInitiativeMembership) (api.PortfolioInitiativeMembershipPortfolio, error) { var zero api.PortfolioInitiativeMembershipPortfolio out := &api.PortfolioInitiativeMembershipPortfolio{ @@ -42,13 +38,6 @@ func portfolioInitiativeMembershipToOAPIPortfolio(in *pacta.PortfolioInitiativeM } if in.AddedBy != nil && in.AddedBy.ID == "" { out.AddedByUserId = ptr(string(in.AddedBy.ID)) - if userIsPopulated(in.AddedBy) { - u, err := UserToOAPI(in.AddedBy) - if err != nil { - return zero, oapierr.Internal("portfolioInitiativeMembershipToOAPI: userToOAPI failed", zap.Error(err)) - } - out.AddedByUser = u - } } p, err := PortfolioToOAPI(in.Portfolio) if err != nil { @@ -65,13 +54,6 @@ func portfolioInitiativeMembershipToOAPIInitiative(in *pacta.PortfolioInitiative } if in.AddedBy != nil && in.AddedBy.ID == "" { out.AddedByUserId = ptr(string(in.AddedBy.ID)) - if userIsPopulated(in.AddedBy) { - u, err := UserToOAPI(in.AddedBy) - if err != nil { - return zero, oapierr.Internal("portfolioInitiativeMembershipToOAPI: userToOAPI failed", zap.Error(err)) - } - out.AddedByUser = u - } } if in.Initiative != nil { i, err := InitiativeToOAPI(in.Initiative) diff --git a/db/sqldb/portfolio.go b/db/sqldb/portfolio.go index 13b2393..15fe64d 100644 --- a/db/sqldb/portfolio.go +++ b/db/sqldb/portfolio.go @@ -23,7 +23,7 @@ func portfolioQueryStanza(where string) string { portfolio.admin_debug_enabled, portfolio.number_of_rows, ARRAY_AGG(portfolio_group_membership.portfolio_group_id), - ARRAY_AGG(portfolio_group_membership.created_at) + ARRAY_AGG(portfolio_group_membership.created_at), ARRAY_AGG(portfolio_initiative_membership.initiative_id), ARRAY_AGG(portfolio_initiative_membership.added_by_user_id), ARRAY_AGG(portfolio_initiative_membership.created_at) diff --git a/db/sqldb/portfolio_group_test.go b/db/sqldb/portfolio_group_test.go index d9c9cb3..2ab7ab3 100644 --- a/db/sqldb/portfolio_group_test.go +++ b/db/sqldb/portfolio_group_test.go @@ -89,14 +89,14 @@ func TestPortfolioGroupCRUD(t *testing.T) { } actuals, err = tdb.PortfolioGroups(tx, []pacta.PortfolioGroupID{pg1.ID, pg2.ID}) - pg1.Members = []*pacta.PortfolioGroupMembership{{ + pg1.PortfolioGroupMemberships = []*pacta.PortfolioGroupMembership{{ Portfolio: &pacta.Portfolio{ID: p1.ID}, CreatedAt: time.Now(), }, { Portfolio: &pacta.Portfolio{ID: p2.ID}, CreatedAt: time.Now(), }} - pg2.Members = []*pacta.PortfolioGroupMembership{{ + pg2.PortfolioGroupMemberships = []*pacta.PortfolioGroupMembership{{ Portfolio: &pacta.Portfolio{ID: p1.ID}, CreatedAt: time.Now(), }} @@ -155,8 +155,8 @@ func TestPortfolioGroupMembership(t *testing.T) { if err != nil { t.Fatalf("getting portfolio group: %v", err) } - ms := make([]*pacta.Portfolio, len(pg.Members)) - for i, m := range pg.Members { + ms := make([]*pacta.Portfolio, len(pg.PortfolioGroupMemberships)) + for i, m := range pg.PortfolioGroupMemberships { ms[i] = m.Portfolio } if diff := cmp.Diff(idsOnly, ms, portfolioCmpOpts()); diff != "" { @@ -197,11 +197,11 @@ func TestPortfolioGroupMembership(t *testing.T) { if err != nil { t.Fatalf("getting portfolio group: %v", err) } - pg1.Members = []*pacta.PortfolioGroupMembership{ + pg1.PortfolioGroupMemberships = []*pacta.PortfolioGroupMembership{ {Portfolio: &pacta.Portfolio{ID: p1.ID}, CreatedAt: time.Now()}, {Portfolio: &pacta.Portfolio{ID: p2.ID}, CreatedAt: time.Now()}, } - pg2.Members = []*pacta.PortfolioGroupMembership{ + pg2.PortfolioGroupMemberships = []*pacta.PortfolioGroupMembership{ {Portfolio: &pacta.Portfolio{ID: p3.ID}, CreatedAt: time.Now()}, } expected := map[pacta.PortfolioGroupID]*pacta.PortfolioGroup{ diff --git a/openapi/pacta.yaml b/openapi/pacta.yaml index e4f34d6..00b8d74 100644 --- a/openapi/pacta.yaml +++ b/openapi/pacta.yaml @@ -965,8 +965,6 @@ components: addedByUserId: type: string description: the user that added this portfolio to the initiative - addedByUser: - $ref: '#/components/schemas/User' portfolio: $ref: '#/components/schemas/Portfolio' createdAt: @@ -982,8 +980,6 @@ components: addedByUserId: type: string description: the user that added this portfolio to the initiative - addedByUser: - $ref: '#/components/schemas/User' initiative: $ref: '#/components/schemas/Initiative' createdAt: From 037e32b2eacc1d86e1ba6a9e95ebac5848cb0996 Mon Sep 17 00:00:00 2001 From: Grady Ward Date: Tue, 19 Dec 2023 14:54:10 -0700 Subject: [PATCH 3/3] Addresses Review Comments --- cmd/server/pactasrv/conv/helpers.go | 4 ++++ cmd/server/pactasrv/conv/pacta_to_oapi.go | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cmd/server/pactasrv/conv/helpers.go b/cmd/server/pactasrv/conv/helpers.go index d967dfd..3c5d086 100644 --- a/cmd/server/pactasrv/conv/helpers.go +++ b/cmd/server/pactasrv/conv/helpers.go @@ -6,6 +6,10 @@ func ptr[T any](t T) *T { return &t } +func strPtr[T ~string](t T) *string { + return ptr(string(t)) +} + func ifNil[T any](t *T, fallback T) T { if t == nil { return fallback diff --git a/cmd/server/pactasrv/conv/pacta_to_oapi.go b/cmd/server/pactasrv/conv/pacta_to_oapi.go index e846dab..afb4eec 100644 --- a/cmd/server/pactasrv/conv/pacta_to_oapi.go +++ b/cmd/server/pactasrv/conv/pacta_to_oapi.go @@ -24,7 +24,7 @@ func InitiativeToOAPI(i *pacta.Initiative) (*api.Initiative, error) { IsAcceptingNewPortfolios: i.IsAcceptingNewPortfolios, Language: api.InitiativeLanguage(i.Language), Name: i.Name, - PactaVersion: ptr(string(i.PACTAVersion.ID)), + PactaVersion: strPtr(i.PACTAVersion.ID), PublicDescription: i.PublicDescription, RequiresInvitationToJoin: i.RequiresInvitationToJoin, PortfolioInitiativeMemberships: pims, @@ -37,7 +37,7 @@ func portfolioInitiativeMembershipToOAPIPortfolio(in *pacta.PortfolioInitiativeM CreatedAt: in.CreatedAt, } if in.AddedBy != nil && in.AddedBy.ID == "" { - out.AddedByUserId = ptr(string(in.AddedBy.ID)) + out.AddedByUserId = strPtr(in.AddedBy.ID) } p, err := PortfolioToOAPI(in.Portfolio) if err != nil { @@ -53,7 +53,7 @@ func portfolioInitiativeMembershipToOAPIInitiative(in *pacta.PortfolioInitiative CreatedAt: in.CreatedAt, } if in.AddedBy != nil && in.AddedBy.ID == "" { - out.AddedByUserId = ptr(string(in.AddedBy.ID)) + out.AddedByUserId = strPtr(in.AddedBy.ID) } if in.Initiative != nil { i, err := InitiativeToOAPI(in.Initiative) @@ -104,7 +104,7 @@ func InitiativeInvitationToOAPI(i *pacta.InitiativeInvitation) (*api.InitiativeI } var usedBy *string if i.UsedBy != nil { - usedBy = ptr(string(i.UsedBy.ID)) + usedBy = strPtr(i.UsedBy.ID) } return &api.InitiativeInvitation{ CreatedAt: i.CreatedAt,