Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[EASI-4457] TRB Advice letter dataloader #2702

Merged
merged 11 commits into from
Jul 16, 2024
Merged

[EASI-4457] TRB Advice letter dataloader #2702

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6a0f16a
add dataloaders to TRB Feedback
mynar7 Jul 15, 2024
d6c7b3e
convert slice to postgres array
mynar7 Jul 15, 2024
4c74d42
use dataloaders in status calls
mynar7 Jul 16, 2024
2c6e27e
Merge branch 'feature/EASI-4457' of github.com:CMS-Enterprise/easi-ap…
mynar7 Jul 16, 2024
2a92ce1
remove unnecessary no rows error check
mynar7 Jul 16, 2024
f4d12ff
add dataloaders to advice letter and finish refactoring statuses
mynar7 Jul 16, 2024
42090c3
fix conflicts
mynar7 Jul 16, 2024
90f5a4d
fix merge conflict and remove circular seed data dependency
mynar7 Jul 16, 2024
8e90df3
sort mock users
mynar7 Jul 16, 2024
ed83b18
nil check statuses
mynar7 Jul 16, 2024
c23214a
Merge branch 'feature/EASI-4457' of github.com:CMS-Enterprise/easi-ap…
mynar7 Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/devdata/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func main() {
s3Client := upload.NewS3Client(s3Cfg)

ctx := mock.CtxWithLoggerAndPrincipal(logger, store, mock.PrincipalUser)
ctx = mock.CtxWithNewDataloaders(ctx, store)
seederConfig := &seederConfig{
logger: logger,
store: store,
Expand Down
16 changes: 16 additions & 0 deletions cmd/devdata/mock/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (

"github.com/cms-enterprise/easi-app/pkg/appcontext"
"github.com/cms-enterprise/easi-app/pkg/authentication"
"github.com/cms-enterprise/easi-app/pkg/dataloaders"
"github.com/cms-enterprise/easi-app/pkg/local"
"github.com/cms-enterprise/easi-app/pkg/local/cedarcoremock"
"github.com/cms-enterprise/easi-app/pkg/models"
"github.com/cms-enterprise/easi-app/pkg/storage"
"github.com/cms-enterprise/easi-app/pkg/userhelpers"
Expand Down Expand Up @@ -60,3 +63,16 @@ func CtxWithLoggerAndPrincipal(logger *zap.Logger, store *storage.Store, euaID s
ctx = appcontext.WithPrincipal(ctx, princ)
return ctx
}

func CtxWithNewDataloaders(ctx context.Context, store *storage.Store) context.Context {
getCedarSystems := func(ctx context.Context) ([]*models.CedarSystem, error) {
return cedarcoremock.GetActiveSystems(), nil
}

buildDataloaders := func() *dataloaders.Dataloaders {
return dataloaders.NewDataloaders(store, local.NewOktaAPIClient().FetchUserInfos, getCedarSystems)
}

// Set up mocked dataloaders for the test context
return dataloaders.CTXWithLoaders(ctx, buildDataloaders)
}
2 changes: 1 addition & 1 deletion cmd/devdata/trb_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ func (s *seederConfig) addAdviceLetter(ctx context.Context, trb *models.TRBReque
}
}

letter, outsideErr = resolvers.GetTRBAdviceLetterByTRBRequestID(ctx, s.store, trb.ID)
letter, outsideErr = resolvers.GetTRBAdviceLetterByTRBRequestID(ctx, trb.ID)
if outsideErr != nil {
return nil, outsideErr
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/dataloaders/dataloaders.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ type Dataloaders struct {
SystemIntakeRelatedSystemIntakes *dataloadgen.Loader[uuid.UUID, []*models.SystemIntake]
SystemIntakeRelatedTRBRequests *dataloadgen.Loader[uuid.UUID, []*models.TRBRequest]
SystemIntakeSystems *dataloadgen.Loader[uuid.UUID, []*models.SystemIntakeSystem]
TRBRequestAdviceLetter *dataloadgen.Loader[uuid.UUID, *models.TRBAdviceLetter]
TRBRequestContractNumbers *dataloadgen.Loader[uuid.UUID, []*models.TRBRequestContractNumber]
TRBRequestForm *dataloadgen.Loader[uuid.UUID, *models.TRBRequestForm]
TRBRequestFeedback *dataloadgen.Loader[uuid.UUID, []*models.TRBRequestFeedback]
Expand Down Expand Up @@ -109,10 +110,11 @@ func NewDataloaders(store *storage.Store, fetchUserInfos fetchUserInfosFunc, get
SystemIntakeRelatedSystemIntakes: dataloadgen.NewLoader(dr.batchRelatedSystemIntakesBySystemIntakeIDs),
SystemIntakeRelatedTRBRequests: dataloadgen.NewLoader(dr.batchRelatedTRBRequestsBySystemIntakeIDs),
SystemIntakeSystems: dataloadgen.NewLoader(dr.batchSystemIntakeSystemsBySystemIntakeIDs),
TRBRequestAdviceLetter: dataloadgen.NewLoader(dr.batchTRBRequestAdviceLettersByTRBRequestIDs),
TRBRequestContractNumbers: dataloadgen.NewLoader(dr.batchTRBRequestContractNumbersByTRBRequestIDs),
TRBRequestForm: dataloadgen.NewLoader(dr.batchTRBRequestFormsByTRBRequestIDs),
TRBRequestFeedback: dataloadgen.NewLoader(dr.batchTRBRequestFeedbackByTRBRequestIDs),
TRBRequestFeedbackNewest: dataloadgen.NewLoader(dr.batchTRBRequestNewestFeedbackByTRBRequestIDs),
TRBRequestContractNumbers: dataloadgen.NewLoader(dr.batchTRBRequestContractNumbersByTRBRequestIDs),
TRBRequestRelatedSystemIntakes: dataloadgen.NewLoader(dr.batchRelatedSystemIntakesByTRBRequestIDs),
TRBRequestRelatedTRBRequests: dataloadgen.NewLoader(dr.batchRelatedTRBRequestsByTRBRequestIDs),
TRBRequestSystems: dataloadgen.NewLoader(dr.batchTRBRequestSystemsByTRBRequestIDs),
Expand Down
29 changes: 29 additions & 0 deletions pkg/dataloaders/trb_request_advice_letter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package dataloaders

import (
"context"
"errors"

"github.com/google/uuid"

"github.com/cms-enterprise/easi-app/pkg/helpers"
"github.com/cms-enterprise/easi-app/pkg/models"
)

func (d *dataReader) batchTRBRequestAdviceLettersByTRBRequestIDs(ctx context.Context, trbRequestIDs []uuid.UUID) ([]*models.TRBAdviceLetter, []error) {
data, err := d.db.GetTRBAdviceLettersByTRBRequestIDs(ctx, trbRequestIDs)
if err != nil {
return nil, []error{err}
}

return helpers.OneToOne(trbRequestIDs, data), nil
}

func GetTRBAdviceLetterByTRBRequestID(ctx context.Context, trbRequestID uuid.UUID) (*models.TRBAdviceLetter, error) {
loaders, ok := loadersFromCTX(ctx)
if !ok {
return nil, errors.New("unexpected nil loaders in GetTRBRequestContractNumbersByTRBRequestID")
}

return loaders.TRBRequestAdviceLetter.Load(ctx, trbRequestID)
}
5 changes: 3 additions & 2 deletions pkg/graph/resolvers/trb_advice_letter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ import (
"golang.org/x/sync/errgroup"

"github.com/cms-enterprise/easi-app/pkg/appcontext"
"github.com/cms-enterprise/easi-app/pkg/dataloaders"
"github.com/cms-enterprise/easi-app/pkg/email"
"github.com/cms-enterprise/easi-app/pkg/models"
"github.com/cms-enterprise/easi-app/pkg/storage"
)

// GetTRBAdviceLetterByTRBRequestID fetches a TRB advice letter record by its associated request's ID.
func GetTRBAdviceLetterByTRBRequestID(ctx context.Context, store *storage.Store, id uuid.UUID) (*models.TRBAdviceLetter, error) {
return store.GetTRBAdviceLetterByTRBRequestID(ctx, id)
func GetTRBAdviceLetterByTRBRequestID(ctx context.Context, id uuid.UUID) (*models.TRBAdviceLetter, error) {
return dataloaders.GetTRBAdviceLetterByTRBRequestID(ctx, id)
}

// CreateTRBAdviceLetter creates an advice letter for a TRB request, in the "In Progress" status, when the advice letter is ready to be worked on.
Expand Down
2 changes: 1 addition & 1 deletion pkg/graph/resolvers/trb_request_feedback.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func CreateTRBRequestFeedback(
}

// GetTRBRequestFeedbackByTRBRequestID retrieves TRB request feedback records for a given TRB request ID
func GetTRBRequestFeedbackByTRBRequestID(ctx context.Context, store *storage.Store, id uuid.UUID) ([]*models.TRBRequestFeedback, error) {
func GetTRBRequestFeedbackByTRBRequestID(ctx context.Context, id uuid.UUID) ([]*models.TRBRequestFeedback, error) {
results, err := dataloaders.GetTRBRequestFeedbackByTRBRequestID(ctx, id)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/graph/resolvers/trb_request_feedback_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (s *ResolverSuite) TestCreateTRBRequestFeedback() {
s.NoError(err)
s.Nil(latestFeedback)

form, err := GetTRBRequestFormByTRBRequestID(s.ctxWithNewDataloaders(), store, trbRequest.ID)
form, err := GetTRBRequestFormByTRBRequestID(s.ctxWithNewDataloaders(), trbRequest.ID)
s.NoError(err)

s.Run("create/update/fetch TRB request feedback", func() {
Expand Down Expand Up @@ -127,7 +127,7 @@ func (s *ResolverSuite) TestCreateTRBRequestFeedback() {
updatedFeedbackStatus, err := getTRBFeedbackStatus(s.ctxWithNewDataloaders(), trbRequest.ID)
s.NoError(err)
s.EqualValues(models.TRBFeedbackStatusEditsRequested, *updatedFeedbackStatus)
form, err := GetTRBRequestFormByTRBRequestID(s.ctxWithNewDataloaders(), store, trbRequest.ID)
form, err := GetTRBRequestFormByTRBRequestID(s.ctxWithNewDataloaders(), trbRequest.ID)
s.NoError(err)
s.EqualValues(form.Status, models.TRBFormStatusInProgress)
s.EqualValues(models.TRBFormStatusInProgress, form.Status)
Expand Down
2 changes: 1 addition & 1 deletion pkg/graph/resolvers/trb_request_form.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func UpdateTRBRequestForm(
}

// GetTRBRequestFormByTRBRequestID retrieves a TRB request form record for a given TRB request ID
func GetTRBRequestFormByTRBRequestID(ctx context.Context, store *storage.Store, id uuid.UUID) (*models.TRBRequestForm, error) {
func GetTRBRequestFormByTRBRequestID(ctx context.Context, id uuid.UUID) (*models.TRBRequestForm, error) {
form, err := dataloaders.GetTRBRequestFormByTRBRequestID(ctx, id)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/graph/resolvers/trb_request_form_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (s *ResolverSuite) TestCreateTRBRequestForm() {

s.Run("create/update/fetch TRB request forms", func() {
// fetch the form
fetched, err := GetTRBRequestFormByTRBRequestID(s.ctxWithNewDataloaders(), s.testConfigs.Store, trbRequest.ID)
fetched, err := GetTRBRequestFormByTRBRequestID(s.ctxWithNewDataloaders(), trbRequest.ID)
s.NoError(err)
s.NotNil(fetched)

Expand Down
95 changes: 44 additions & 51 deletions pkg/graph/resolvers/trb_request_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ package resolvers

import (
"context"
"fmt"
"time"

"github.com/google/uuid"
"golang.org/x/sync/errgroup"

"github.com/cms-enterprise/easi-app/pkg/dataloaders"
"github.com/cms-enterprise/easi-app/pkg/models"
"github.com/cms-enterprise/easi-app/pkg/storage"
)

func getTRBFormStatus(ctx context.Context, trbRequestID uuid.UUID) (*models.TRBFormStatus, error) {
Expand Down Expand Up @@ -62,6 +61,9 @@ func getTRBConsultPrepStatus(ctx context.Context, trbRequest models.TRBRequest)
if err != nil {
return nil, err
}
if feedbackStatus == nil {
mynar7 marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("feedback status is nil for trb request %v", trbRequest.ID)
}

if *feedbackStatus == models.TRBFeedbackStatusCompleted {
if trbRequest.ConsultMeetingTime != nil && time.Now().After(*trbRequest.ConsultMeetingTime) {
Expand All @@ -79,6 +81,9 @@ func getTRBAttendConsultStatus(ctx context.Context, trbRequest models.TRBRequest
if err != nil {
return nil, err
}
if feedbackStatus == nil {
return nil, fmt.Errorf("feedback status is nil for trb request %v", trbRequest.ID)
}

if *feedbackStatus == models.TRBFeedbackStatusCompleted {
if trbRequest.ConsultMeetingTime != nil {
Expand All @@ -94,10 +99,10 @@ func getTRBAttendConsultStatus(ctx context.Context, trbRequest models.TRBRequest
return &status, nil
}

func getTRBAdviceLetterStatus(ctx context.Context, store *storage.Store, trbRequest models.TRBRequest) (*models.TRBAdviceLetterStatus, error) {
func getTRBAdviceLetterStatus(ctx context.Context, trbRequest models.TRBRequest) (*models.TRBAdviceLetterStatus, error) {
var status models.TRBAdviceLetterStatus

letter, err := store.GetTRBAdviceLetterByTRBRequestID(ctx, trbRequest.ID)
letter, err := dataloaders.GetTRBAdviceLetterByTRBRequestID(ctx, trbRequest.ID)
if err != nil {
return nil, err
}
Expand All @@ -117,54 +122,42 @@ func getTRBAdviceLetterStatus(ctx context.Context, store *storage.Store, trbRequ
}

// GetTRBTaskStatuses retrieves all of the statuses for the steps of a given TRB request's task list
func GetTRBTaskStatuses(ctx context.Context, store *storage.Store, trbRequest models.TRBRequest) (*models.TRBTaskStatuses, error) {
errGroup := new(errgroup.Group)

var formStatus *models.TRBFormStatus
var errForm error
errGroup.Go(func() error {
formStatus, errForm = getTRBFormStatus(ctx, trbRequest.ID)
return errForm
})

var feedbackStatus *models.TRBFeedbackStatus
var errFeedback error
errGroup.Go(func() error {
feedbackStatus, errFeedback = getTRBFeedbackStatus(ctx, trbRequest.ID)
return errFeedback
})

var consultPrepStatus *models.TRBConsultPrepStatus
var errConsultPrep error
errGroup.Go(func() error {
consultPrepStatus, errConsultPrep = getTRBConsultPrepStatus(ctx, trbRequest)
return errConsultPrep
})

var attendConsultStatus *models.TRBAttendConsultStatus
var errAttendConsult error
errGroup.Go(func() error {
attendConsultStatus, errAttendConsult = getTRBAttendConsultStatus(ctx, trbRequest)
return errAttendConsult
})

var adviceLetterStatus *models.TRBAdviceLetterStatus
adviceLetterStatusTaskList := models.TRBAdviceLetterStatusTaskListInReview
var errAdviceLetter error
errGroup.Go(func() error {
adviceLetterStatus, errAdviceLetter = getTRBAdviceLetterStatus(ctx, store, trbRequest)
if *adviceLetterStatus == models.TRBAdviceLetterStatusCannotStartYet {
adviceLetterStatusTaskList = models.TRBAdviceLetterStatusTaskListCannotStartYet
} else if *adviceLetterStatus == models.TRBAdviceLetterStatusCompleted {
adviceLetterStatusTaskList = models.TRBAdviceLetterStatusTaskListCompleted
}
return errAdviceLetter
})
func GetTRBTaskStatuses(ctx context.Context, trbRequest models.TRBRequest) (*models.TRBTaskStatuses, error) {
formStatus, err := getTRBFormStatus(ctx, trbRequest.ID)
if err != nil {
return nil, err
}

if err := errGroup.Wait(); err != nil {
feedbackStatus, err := getTRBFeedbackStatus(ctx, trbRequest.ID)
if err != nil {
return nil, err
}

consultPrepStatus, err := getTRBConsultPrepStatus(ctx, trbRequest)
if err != nil {
return nil, err
}

attendConsultStatus, err := getTRBAttendConsultStatus(ctx, trbRequest)
if err != nil {
return nil, err
}

adviceLetterStatus, err := getTRBAdviceLetterStatus(ctx, trbRequest)
if err != nil {
return nil, err
}
if adviceLetterStatus == nil {
Copy link
Contributor

@samoddball samoddball Jul 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

should we nil-check all of these? i see they are all dereferenced below

also a curiosity - why remove the goroutines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that opening a zillion channels is probably worse than just writing it synchronously and letting the dataloaders return cached data, but I might be wrong there. @ClayBenson94 do you have any thoughts?

I missed the dereference! I will add checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nil checks added here: ed83b18

Copy link
Collaborator

Choose a reason for hiding this comment

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

If each synchronous call is behind a dataloader it probably boils down to readability more than anything, since each of those calls will probably already be cached for this request

return nil, fmt.Errorf("advice letter status is nil for trb request %v", trbRequest.ID)
}

adviceLetterStatusTaskList := models.TRBAdviceLetterStatusTaskListInReview
if *adviceLetterStatus == models.TRBAdviceLetterStatusCannotStartYet {
adviceLetterStatusTaskList = models.TRBAdviceLetterStatusTaskListCannotStartYet
} else if *adviceLetterStatus == models.TRBAdviceLetterStatusCompleted {
adviceLetterStatusTaskList = models.TRBAdviceLetterStatusTaskListCompleted
}

statuses := models.TRBTaskStatuses{
FormStatus: *formStatus,
FeedbackStatus: *feedbackStatus,
Expand All @@ -178,11 +171,11 @@ func GetTRBTaskStatuses(ctx context.Context, store *storage.Store, trbRequest mo
}

// GetTRBRequestStatus calculates the overall status of the TRB request
func GetTRBRequestStatus(ctx context.Context, store *storage.Store, trbRequest models.TRBRequest) (models.TRBRequestStatus, error) {
func GetTRBRequestStatus(ctx context.Context, trbRequest models.TRBRequest) (models.TRBRequestStatus, error) {
var status models.TRBRequestStatus
status = models.TRBRequestStatusNew

taskStatuses, err := GetTRBTaskStatuses(ctx, store, trbRequest)
taskStatuses, err := GetTRBTaskStatuses(ctx, trbRequest)
if err != nil {
return status, err
}
Expand Down Expand Up @@ -236,7 +229,7 @@ func GetTRBRequestStatus(ctx context.Context, store *storage.Store, trbRequest m
// Advice letter sent
if adviceLetterStatus == models.TRBAdviceLetterStatusCompleted {
// Get the advice letter and check if follow-up was recommended
adviceLetter, err := GetTRBAdviceLetterByTRBRequestID(ctx, store, trbRequest.ID)
adviceLetter, err := GetTRBAdviceLetterByTRBRequestID(ctx, trbRequest.ID)
if err != nil {
return status, err
}
Expand Down
Loading