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.

Sign up for GitHub

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

Jump to bottom

B 21668 m #14363

Merged
merged 40 commits into from
Dec 11, 2024
Merged

B 21668 m #14363

merged 40 commits into from
Dec 11, 2024

Conversation

deandreJones
Copy link
Contributor

@deandreJones deandreJones commented Dec 10, 2024 •
edited
Loading

danieljordan-caci and others added 30 commits December 10, 2024 16:03
@danieljordan-caci @deandreJones
initial commit, added stored proc for creating service items
5e16ddf
@danieljordan-caci @deandreJones
adding stored proc for accessorials - not finished
00ee1ec
@deandreJones
checkin
2e7a382
@danieljordan-caci @deandreJones
refactored migrations to use single quotes
3bf731a
@deandreJones
change from json to mto_service_item_type
77ae982
@deandreJones
basics
c75440c
@deandreJones
add estimated and actual weight
5659f5d
@deandreJones
too many commas
a4bc26a
@deandreJones
modifications
2fb332c
@deandreJones
ed
b243534
@deandreJones
missed one
f730c4b
@deandreJones
remove test- not worth my headache right now
221384d
@deandreJones
approved
b6c4be7
@deandreJones
approved_at , now(()
448438b
@deandreJones
app now
06fc431
@deandreJones
add sts check- upated from read json to compostie trp
d48428a
@deandreJones
port of....sit postal
396da96
@deandreJones
add the dup check
90b3f53
@deandreJones
add type check
9e6e2bb
@deandreJones
cleanup - renaming..
e4a5a5f
@deandreJones
added test- fix call to proc after renaming
d857e3f
@deandreJones
mto_service_item_type
06efec0
@deandreJones
cleanup
5d76ed0
@deandreJones
correctiuions
60d9ce3
@deandreJones
json it is
43dd41c
@deandreJones
remove exception for oconus-oconus
5ee3379
@deandreJones
update ordering proc
ec1d065
@deandreJones
add import
84252de
@deandreJones
put it back
8e145b9
@deandreJones
2 decimals
a0ba644
@deandreJones deandreJones marked this pull request as ready for review December 10, 2024 16:40
@deandreJones deandreJones requested a review from a team as a code owner December 10, 2024 16:40
danieljordan-caci
danieljordan-caci approved these changes Dec 10, 2024
View reviewed changes
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.

LGTM

traskowskycaci
traskowskycaci reviewed Dec 10, 2024
View reviewed changes
Copy link
Contributor

@traskowskycaci traskowskycaci left a comment

Choose a reason for hiding this comment

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

Do we want this commit from int here as well?

@deandreJones
Copy link
Contributor Author

deandreJones commented Dec 10, 2024 •
edited
Loading

awaiting upstream

@deandreJones
Copy link
Contributor Author

Do we want this commit from int here as well?

good catch eagle eyes

traskowskycaci
traskowskycaci approved these changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@traskowskycaci traskowskycaci left a comment

Choose a reason for hiding this comment

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

Matches INT!

@deandreJones
Copy link
Contributor Author

Matches INT!

@traskowskycaci does this build for you? failing server tests that passed INT.. might need another set of eyes - thanks

@traskowskycaci
Copy link
Contributor

Matches INT!

@traskowskycaci does this build for you? failing server tests that passed INT.. might need another set of eyes - thanks

Builds fine locally, mid-run on server tests now. All your code is an exact match to the int PR

@traskowskycaci
Copy link
Contributor

Matches INT!

@traskowskycaci does this build for you? failing server tests that passed INT.. might need another set of eyes - thanks

Builds fine locally, mid-run on server tests now. All your code is an exact match to the int PR

does fail for me locally on the same test package as circle

2024/12/11 15:31:08 package models_test is attempting to connect to database postgres
2024/12/11 15:31:08 TXNDB: package models_test will use database test_db_1 in pid 21072
--- FAIL: TestModelSuite (0.58s)
    --- FAIL: TestModelSuite/TestCreateApprovedServiceItemsForShipment (0.32s)
        --- FAIL: TestModelSuite/TestCreateApprovedServiceItemsForShipment/test_creating_approved_service_items_for_shipment (0.32s)
            mto_shipments_test.go:317: 
                        Error Trace:    /Users/m.traskowsky_cn/Documents/github/mymove/pkg/models/mto_shipments_test.go:317
                                                                /Users/m.traskowsky_cn/go/pkg/mod/github.com/stretchr/testify@v1.9.0/suite/suite.go:115
                        Error:          Received unexpected error:
                                        error creating approved service items: pq: Error creating POEFSC service item for shipment 965f123b-2717-495b-8ba5-476d0826a52f: syntax error at or near "IF"
                        Test:           TestModelSuite/TestCreateApprovedServiceItemsForShipment/test_creating_approved_service_items_for_shipment
FAIL
FAIL    github.com/transcom/mymove/pkg/models   1.611s

@deandreJones
Copy link
Contributor Author

Matches INT!

@traskowskycaci does this build for you? failing server tests that passed INT.. might need another set of eyes - thanks

Builds fine locally, mid-run on server tests now. All your code is an exact match to the int PR

yea I don't get it

@deandreJones deandreJones merged commit 006493a into main Dec 11, 2024
42 checks passed
@deandreJones deandreJones deleted the B-21668--m branch December 11, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieljordan-caci danieljordan-caci danieljordan-caci approved these changes

@traskowskycaci traskowskycaci traskowskycaci approved these changes

@TevinAdams TevinAdams Awaiting requested review from TevinAdams TevinAdams was automatically assigned from transcom/mountain-movers

@cameroncaci cameroncaci Awaiting requested review from cameroncaci cameroncaci was automatically assigned from transcom/mountain-movers

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@deandreJones @traskowskycaci @danieljordan-caci