diff --git a/pkg/models/order.go b/pkg/models/order.go index dacb47e0e5e..73991b0758e 100644 --- a/pkg/models/order.go +++ b/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"` } diff --git a/pkg/services/move_order/move_order_fetcher.go b/pkg/services/move_order/move_order_fetcher.go index 8bdf1b03296..132aea52275 100644 --- a/pkg/services/move_order/move_order_fetcher.go +++ b/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 + } } } diff --git a/pkg/services/move_task_order/move_task_order_fetcher.go b/pkg/services/move_task_order/move_task_order_fetcher.go index cfaad87cd34..9de808ed90b 100644 --- a/pkg/services/move_task_order/move_task_order_fetcher.go +++ b/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", diff --git a/pkg/services/payment_request/payment_request_list_fetcher.go b/pkg/services/payment_request/payment_request_list_fetcher.go index 739688df3de..989dee58446 100644 --- a/pkg/services/payment_request/payment_request_list_fetcher.go +++ b/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 } }