Merge pull request #5880 from transcom/rr-MB-6628-eagerpreload

[MB-6628] Use EagerPreload to improve performance in some key spots

pkg/models/order.go

@@ -44,7 +44,7 @@ type Order struct {
 	HasDependents       bool                               `json:"has_dependents" db:"has_dependents"`
 	SpouseHasProGear    bool                               `json:"spouse_has_pro_gear" db:"spouse_has_pro_gear"`
 	NewDutyStationID    uuid.UUID                          `json:"new_duty_station_id" db:"new_duty_station_id"`
-	NewDutyStation      DutyStation                        `belongs_to:"duty_stations"`
+	NewDutyStation      DutyStation                        `belongs_to:"duty_stations" fk_id:"new_duty_station_id"`
 	UploadedOrders      Document                           `belongs_to:"documents"`
 	UploadedOrdersID    uuid.UUID                          `json:"uploaded_orders_id" db:"uploaded_orders_id"`
 	OrdersNumber        *string                            `json:"orders_number" db:"orders_number"`
@@ -56,7 +56,7 @@ type Order struct {
 	Grade               *string                            `json:"grade" db:"grade"`
 	Entitlement         *Entitlement                       `belongs_to:"entitlements"`
 	EntitlementID       *uuid.UUID                         `json:"entitlement_id" db:"entitlement_id"`
-	OriginDutyStation   *DutyStation                       `belongs_to:"duty_stations"`
+	OriginDutyStation   *DutyStation                       `belongs_to:"duty_stations" fk_id:"origin_duty_station_id"`
 	OriginDutyStationID *uuid.UUID                         `json:"origin_duty_station_id" db:"origin_duty_station_id"`
 }
 

pkg/services/move_order/move_order_fetcher.go

@@ -57,10 +57,12 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service
 	// Adding to an array so we can iterate over them and apply the filters after the query structure is set below
 	options := []QueryOption{branchQuery, locatorQuery, dodIDQuery, lastNameQuery, dutyStationQuery, moveStatusQuery, gblocQuery, sortOrderQuery}
 
-	query := f.db.Q().Eager(
+	query := f.db.Q().EagerPreload(
 		"Orders.ServiceMember",
 		"Orders.NewDutyStation.Address",
-		"Orders.OriginDutyStation",
+		"Orders.OriginDutyStation.Address",
+		// See note further below about having to do this in a separate Load call due to a Pop issue.
+		// "Orders.OriginDutyStation.TransportationOffice",
 		"Orders.Entitlement",
 		"MTOShipments",
 		"MTOServiceItems",
@@ -79,14 +81,6 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service
 		}
 	}
 
-	if err != nil {
-		switch err {
-		case sql.ErrNoRows:
-			return []models.Move{}, 0, services.NotFoundError{}
-		default:
-			return []models.Move{}, 0, err
-		}
-	}
 	// Pass zeros into paginate in this case. Which will give us 1 page and 20 per page respectively
 	if params.Page == nil {
 		params.Page = swag.Int64(0)
@@ -114,11 +108,21 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service
 	count := query.Paginator.TotalEntriesSize
 
 	for i := range moves {
-		// Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we
-		// cannot eager load the address as "OriginDutyStation.Address" because
-		// OriginDutyStation is a pointer.
+		// There appears to be a bug in Pop for EagerPreload when you have two or more eager paths with 3+ levels
+		// where the first 2 levels match.  For example:
+		//   "Orders.OriginDutyStation.Address" and "Orders.OriginDutyStation.TransportationOffice"
+		// In those cases, only the last relationship is loaded in the results.  So, we can only do one of the paths
+		// in the EagerPreload above and request the second one explicitly with a separate Load call.
+		//
+		// Note that we also had a problem before with Eager as well.  Here's what we found with it:
+		//   Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we
+		//   cannot eager load the address as "OriginDutyStation.Address" because
+		//   OriginDutyStation is a pointer.
 		if moves[i].Orders.OriginDutyStation != nil {
-			f.db.Load(moves[i].Orders.OriginDutyStation, "Address", "TransportationOffice")
+			loadErr := f.db.Load(moves[i].Orders.OriginDutyStation, "TransportationOffice")
+			if loadErr != nil {
+				return []models.Move{}, 0, err
+			}
 		}
 	}
 

pkg/services/move_task_order/move_task_order_fetcher.go

@@ -41,7 +41,7 @@ func (f moveTaskOrderFetcher) ListMoveTaskOrders(moveOrderID uuid.UUID, searchPa
 func (f moveTaskOrderFetcher) ListAllMoveTaskOrders(searchParams *services.ListMoveTaskOrderParams) (models.Moves, error) {
 	var moveTaskOrders models.Moves
 	var err error
-	query := f.db.Q().Eager(
+	query := f.db.Q().EagerPreload(
 		"PaymentRequests.PaymentServiceItems.PaymentServiceItemParams.ServiceItemParamKey",
 		"MTOServiceItems.ReService",
 		"MTOServiceItems.Dimensions",

pkg/services/payment_request/payment_request_list_fetcher.go

@@ -48,9 +48,10 @@ func (f *paymentRequestListFetcher) FetchPaymentRequestList(officeUserID uuid.UU
 	}
 
 	paymentRequests := models.PaymentRequests{}
-	query := f.db.Q().Eager(
-		"MoveTaskOrder.Orders.OriginDutyStation",
-		"MoveTaskOrder.Orders.ServiceMember",
+	query := f.db.Q().EagerPreload(
+		"MoveTaskOrder.Orders.OriginDutyStation.TransportationOffice",
+		// See note further below about having to do this in a separate Load call due to a Pop issue.
+		// "MoveTaskOrder.Orders.ServiceMember",
 	).
 		InnerJoin("moves", "payment_requests.move_id = moves.id").
 		InnerJoin("orders", "orders.id = moves.orders_id").
@@ -103,11 +104,19 @@ func (f *paymentRequestListFetcher) FetchPaymentRequestList(officeUserID uuid.UU
 	}
 
 	for i := range paymentRequests {
-		// Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we
-		// cannot eager load the address as "OriginDutyStation.Address" because
-		// OriginDutyStation is a pointer.
-		if originDutyStation := paymentRequests[i].MoveTaskOrder.Orders.OriginDutyStation; originDutyStation != nil {
-			f.db.Load(originDutyStation, "TransportationOffice")
+		// There appears to be a bug in Pop for EagerPreload when you have two or more eager paths with 3+ levels
+		// where the first 2 levels match.  For example:
+		//   "MoveTaskOrder.Orders.OriginDutyStation.TransportationOffice" and "MoveTaskOrder.Orders.ServiceMember"
+		// In those cases, only the last relationship is loaded in the results.  So, we can only do one of the paths
+		// in the EagerPreload above and request the second one explicitly with a separate Load call.
+		//
+		// Note that we also had a problem before with Eager as well.  Here's what we found with it:
+		//   Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we
+		//   cannot eager load the address as "OriginDutyStation.Address" because
+		//   OriginDutyStation is a pointer.
+		loadErr := f.db.Load(&paymentRequests[i].MoveTaskOrder.Orders, "ServiceMember")
+		if loadErr != nil {
+			return nil, 0, err
 		}
 	}