-
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
pkg/services/order/order_fetcher.go
Outdated
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
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.
That test was incorrect - the TOO shouldn't have the move appear in the queue after a payment request is created, it should appear in the TIO queue. However, the TOO can use the higher level move search instead of the move code queue search, so I updated the test to do so.
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
pinging those who I know have been taking a look - I updated the description with the above statement about ignoring CRQST1. @danieljordan-caci @brianmanley-caci @cameroncaci |
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.
It's crazy how such little code changes can be so complex under the hood.
I tested a bunch of moves and it seems to be working well. Destination only shows up in the destination queue and not in the origin queue, destination address requests only show up in the destination queue if that's all there is...
Hope you're okay after doing this work
Twas a rough one on me! |
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
Please ignore the move
CRQST1
- it has only a destination address change and should be filtered out of the queue, but is hanging around for some reason. However, if you create a new move with only a destination address change - it is filtered out of the TOO queue correctly. I suspect the weird behavior is due to this being an old test move.Please try to think of any situation where a move should appear in the TOO queue and test that in addition the scenarios below.
Some test cases to try - please try various combos of these as you see fit.
Test cases:
Test service items, shipment address updates, excess weight triggered
Should be able to appear in both TOO and destination TOO queue:
SIT Extension should appear in TOO queue: