-
Notifications
You must be signed in to change notification settings - Fork 35
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.
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
B-21941 TOO Queue Filtering For Destination Only Requests #14465
base: integrationTesting
Are you sure you want to change the base?
Conversation
…on-Submit-move-details-task-order-with-files
…task-order-with-files
…task-order-with-files
…task-order-with-files
…task-order-with-files
…t-move-details-task-order-with-files Main b 21373 validation submit move details task order with files
…m/transcom/mymove into B-22107-DynamicDP3-Stg-Prd-deploy
…ploy ci/cd work
}, nil) | ||
|
||
suite.NotNil(shipment) | ||
// newTestUUID := uuid.UUID{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left in a comment here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops! Fixed here
}) | ||
|
||
suite.Run("task order queue does not return move with ONLY requested destination SIT service items", func() { | ||
// officeUser, _, session := setupTestData() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops! Fixed here
func tooDestinationOnlyRequestsFilter(role roles.RoleType, locator *string) QueryOption { | ||
return func(query *pop.Query) { | ||
if role == roles.RoleTypeTOO { | ||
if locator != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I have a question on this part. It seems like this allows them to use the column filter to filter by a move code that wasn't in the table. Is that the desired behavior? It seems kind of odd to me, but maybe that's what they asked for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch - pushing up a change now to address that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this here but broke a test - moving PR back to draft, will pick back up when I come back next week
pkg/services/order/order_fetcher.go
Outdated
} else { | ||
query.Where(` | ||
( | ||
(mto_service_items.status IS NULL OR (mto_service_items.status = 'SUBMITTED' AND re_services.code IN ('DOFSIT', 'DOASIT', 'DODSIT', 'DOSHUT', 'DOSFSC', 'IOFSIT', 'IOASIT', 'IODSIT', 'IOSHUT'))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this not apply to other origin services like IOPSIT, ICRT, IOSFSC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added here
Agility ticket
Summary
This query gave me a run for my money - to filter out moves from the main TOO queue with only destination requests, we have to do some joins and check for destination requests, like destination SIT or destination address changes - as well as check that we only have those and not anything else that should keep the move in the TOO main queue (such as excess weight warnings).
I added a locator check in the
tooDestinationOnlyRequestsFilter
function, otherwise it would pull in moves with excess weight on every search, in addition to the move searched for.How to test
Some test cases to try - please try various combos of these as you see fit.
Test cases: