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.

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

Merged
merged 32 commits into from
Jan 9, 2025

Conversation

traskowskycaci
Copy link
Contributor

@traskowskycaci traskowskycaci commented Dec 24, 2024

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:

1. New move
2. SC counsels
3. TOO approves
4. Prime adds origin SIT service items
5. Check TOO queue - should appear
6. TOO approves origin SIT service items
7. Check too queue - should not appear
8. Prime adds dest SIT service items
9. Check TOO queue - should not appear
10. Check dest queue - should appear
11. Approves dest SIT service items
12. Check TOO queue - should still not appear
13. Check dest queue - should not appear
14. Update dest address to have a delivery address update request ( > 50 miles)
15. Check too queue - should not appear
16. Check dest queue - should appear
17. Trigger move for excess weight as prime
18. Check TOO queue - should appear
19. Check dest queue - should still appear if you didn't approve the destination address change, or won't appear in dest queue if you did

Test service items, shipment address updates, excess weight triggered

1. New move
2. SC counsels
3. TOO approves
4. Prime adds origin SIT service items
5. Check TOO queue - should appear
6. TOO approves origin SIT service items
7. Check TOO queue - should not appear
8. Prime adds dest SIT service items
9. Check TOO queue - should not appear
10. Check dest queue - should appear
11. Trigger move for excess weight as prime
12. Check TOO queue - should appear
13. Check dest queue - should not appear
14. Approve dest SIT service items
15. Check TOO queue - should still not appear
16. Check dest queue - should not appear
17. Update dest address to have a delivery address update request ( > 50 miles)
18. Check TOO queue - should not appear
19. Check dest queue - should appear

Should be able to appear in both TOO and destination TOO queue:

1. New move
2. SC counsels
3. TOO approve
4. Prime addes origin SIT service items
5. Check TOO queue - should appear
6. Prime adds dest SIT service items
7. check too queue - should appear
8. check dest queue - should appear
9. approve dest SIT service items
10. check too queue - should still appear
11. check dest queue - should not appear

SIT Extension should appear in TOO queue:

1. New move
2. SC counsels
3. TOO approves
4. Prime adds origin SIT service items
5. Check TOO queue - should appear
6. TOO approves origin SIT service items
7. Check TOO queue - should not appear
8. Prime requests SIT extension
9. Check TOO queue - should appear
10. Check dest queue - should not appear

@traskowskycaci traskowskycaci self-assigned this Dec 24, 2024
@traskowskycaci traskowskycaci added Mountain Movers Movin' Mountains 1 Sprint at a time INTEGRATION Slated for Integration Testing labels Dec 24, 2024
@traskowskycaci traskowskycaci marked this pull request as ready for review December 24, 2024 14:24
@traskowskycaci traskowskycaci requested review from a team as code owners December 24, 2024 14:24
@traskowskycaci traskowskycaci marked this pull request as draft December 27, 2024 00:42
@traskowskycaci traskowskycaci marked this pull request as ready for review January 3, 2025 22:46
@traskowskycaci
Copy link
Contributor Author

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.

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

Copy link
Contributor

@danieljordan-caci danieljordan-caci left a 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

@traskowskycaci
Copy link
Contributor Author

Hope you're okay after doing this work

Twas a rough one on me!

Copy link
Contributor

@TevinAdams TevinAdams left a comment

Choose a reason for hiding this comment

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

Tested and didn't break anything

Copy link
Contributor

@cameroncaci cameroncaci left a comment

Choose a reason for hiding this comment

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

Leaving this note here per refactor discussions with @traskowskycaci

func excludeDestinationRequestsFilter(role roles.RoleType) QueryOption {
	return func(query *pop.Query) {
		codesToStringSlice := func(codes []models.ReServiceCode) []string {
			codeStrings := make([]string, len(codes))
			for i, code := range codes {
				codeStrings[i] = string(code)
			}
			return codeStrings
		}

		if role == roles.RoleTypeTOO {
			// Do not return moves that are purely in the destination queue
			// eg, If they match this condition (status=APPROVALS REQUESTED + only destination SIT/shuttle/address updates)
			// then exclude them from the results
			query.Where(`
			NOT ( 
				moves.status = 'APPROVALS REQUESTED'
				-- Check that destination address or re service codes are present and not approved
				AND (
					shipment_address_updates.status = 'REQUESTED'
					OR (
						mto_service_items.status = 'SUBMITTED'
						AND (
							re_services.code IN (?) 
							OR re_services.code IN (?)
						)
					)
				)
				-- Check that origin service codes, if present, are not of status SUBMITTED
				AND NOT EXISTS (
					-- Select any service item on this move that is submitted and NOT destination, if found then this move should not be excluded
					SELECT 1
					FROM mto_service_items AS msi
					JOIN re_services AS re_service ON re_service.id = msi.re_service_id
					WHERE msi.move_id = moves.id
						AND msi.status = 'SUBMITTED'
						-- Put our destination codes here, we want toANY service found that isn't destination
						AND re_service.code NOT IN (?)
						AND re_service.code NOT IN (?)
					)
			)
			`,
				codesToStringSlice(models.ValidDomesticDestinationSITReServiceCodes),
				codesToStringSlice(models.ValidInternationalDestinationSITReServiceCodes),
				codesToStringSlice(models.ValidDomesticDestinationSITReServiceCodes),
				codesToStringSlice(models.ValidInternationalDestinationSITReServiceCodes),
			)
		}
	}
}

The following query adjustments were proposed, but for equivalent functionality and remaining bugs to iron out it wasn't pursued. Could be an idea if we need to come back to this query in the future for a new feature.

Existing functionality tested successfuly 👍

Copy link
Contributor

@brianmanley-caci brianmanley-caci left a comment

Choose a reason for hiding this comment

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

Tested a bunch of different scenarios, and everything is working as expected. Great work on this!

@traskowskycaci
Copy link
Contributor Author

Appreciate all the reviews on this one, thanks guys!

@traskowskycaci traskowskycaci merged commit e203a18 into integrationTesting Jan 9, 2025
34 checks passed
@traskowskycaci traskowskycaci deleted the B-21941-INT-v2 branch January 9, 2025 14:56
traskowskycaci added a commit that referenced this pull request Jan 17, 2025
This reverts commit e203a18, reversing
changes made to 610bcb9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTEGRATION Slated for Integration Testing Mountain Movers Movin' Mountains 1 Sprint at a time
Development

Successfully merging this pull request may close these issues.

5 participants