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 20619 prime create mobile home int #13710

Merged
merged 15 commits into from
Sep 19, 2024
Merged

B 20619 prime create mobile home int #13710

merged 15 commits into from
Sep 19, 2024

Conversation

KonstanceH
Copy link
Contributor

@KonstanceH KonstanceH commented Sep 13, 2024 •
edited
Loading

B-20619

Summary

This PR adds the capability for Prime counselors to create mobile home shipments for existing moves.

Verification Steps for Reviewers

These are to be checked by a reviewer.

  • Has the branch been pulled in and checked out?
  • Have the BL acceptance criteria been met for this change?
  • Was the CircleCI build successful?
  • Has the code been reviewed from a standards and best practices point of view?

How to test

  1. Login as Prime
  2. Choose any move
  3. Click "Create Shipment" button and choose "MOBILE_HOME"
  4. Confirm unique mobile home shipment fields show
  5. Confirm to see mobile home shipment success banner
  6. From the confirmation banner you can use that id for this query to check in DB that it was added
    select id, shipment_type, status from mto_shipments ms where id ='shipment id'

Frontend

  • There are no aXe warnings for UI.
  • This works in Supported Browsers and their phone views (Chrome, Firefox, Edge).
  • There are no new console errors in the browser devtools.
  • There are no new console errors in the test output.
  • If this PR adds a new component to Storybook, it ensures the component is fully responsive, OR if it is intentionally not, a wrapping div using the officeApp class or custom min-width styling is used to hide any states the would not be visible to the user.
  • This change meets the standards for Section 508 compliance.

Backend

Screenshots

Mobile Home Form
Screenshot 2024-09-13 at 1 53 53 PM

Mobile Home added to shipment types dropdown list
Screenshot 2024-09-13 at 1 53 25 PM

Created Mobile Home Shipment Successful banner
Screenshot 2024-09-13 at 1 51 05 PM

Mobile Home Shipment specific fields
Screenshot 2024-09-17 at 2 10 49 PM

@KonstanceH KonstanceH self-assigned this Sep 13, 2024
@KonstanceH KonstanceH added ByteSize M&Ms Team ByteSized M&Ms INTEGRATION Slated for Integration Testing labels Sep 13, 2024
@KonstanceH KonstanceH marked this pull request as ready for review September 13, 2024 18:45
@KonstanceH KonstanceH requested a review from a team as a code owner September 13, 2024 18:45
@taeJungCaci
Copy link
Contributor

taeJungCaci commented Sep 13, 2024 •
edited
Loading

I see this section for Boat shipment. Should this also be the same for mobile homes?
image

Also, I think mobileHomeShipment needs to show in the Prime GET api

JamesHawks224
JamesHawks224 reviewed Sep 13, 2024
View reviewed changes
pkg/handlers/primeapiv3/mto_shipment.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

"mobile_home"
This string literal occurs in multiple places. Should it be made a constant and referenced in the code?

isMobileHomeFeatureOn
This was already initialized to false at line 62. Does it need to be initialized again after the nil check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

honestly I'm not sure, I'll poke around and see what's standard or if I should/could make it a constant somewhere for the feature flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I poked around and came to the conclusion that this would be outside of the scope for this story. I left comments responding to yours above.

JamesHawks224
JamesHawks224 requested changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@JamesHawks224 JamesHawks224 left a comment

Choose a reason for hiding this comment

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

testing: success
code: see comments

@KonstanceH
Copy link
Contributor Author

I see this section for Boat shipment. Should this also be the same for mobile homes? image

Also, I think mobileHomeShipment needs to show in the Prime GET api

I definitely added a section for mobile home too... I must of overwrote it when I merged in main. I'll add these back in now

@KonstanceH
Copy link
Contributor Author

I see this section for Boat shipment. Should this also be the same for mobile homes? image
Also, I think mobileHomeShipment needs to show in the Prime GET api

I definitely added a section for mobile home too... I must of overwrote it when I merged in main. I'll add these back in now

Okay idk how but I guess it was taken out of the model to payload so I just added it back and added a screenshot of it in the description

taeJungCaci
taeJungCaci reviewed Sep 17, 2024
View reviewed changes
pkg/handlers/primeapiv3/payloads/model_to_payload.go Outdated
Copy link
Contributor

@taeJungCaci taeJungCaci Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should changes in this file also be added in prime v1 and v2?

Copy link
Contributor

Choose a reason for hiding this comment

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

@brooklyn-welsh

Copy link
Contributor

@taeJungCaci taeJungCaci Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

maybe in pricing stories if so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried adding them in and the behaviors been a little weird so I'm going to add a note to add these in to the pricing stories during planning.

taeJungCaci
taeJungCaci approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@taeJungCaci taeJungCaci left a comment

Choose a reason for hiding this comment

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

LGTM

@KonstanceH KonstanceH merged commit ee98a66 into integrationTesting Sep 19, 2024
30 checks passed
@KonstanceH KonstanceH deleted the B-20619-Prime-Create-Mobile-Home-INT branch September 19, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taeJungCaci taeJungCaci taeJungCaci approved these changes

@JamesHawks224 JamesHawks224 JamesHawks224 approved these changes

Assignees

@KonstanceH KonstanceH

Labels
ByteSize M&Ms Team ByteSized M&Ms INTEGRATION Slated for Integration Testing
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@KonstanceH @taeJungCaci @JamesHawks224