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 20939 int #13538

Merged
merged 13 commits into from
Aug 23, 2024
Merged

B 20939 int #13538

merged 13 commits into from
Aug 23, 2024

Conversation

antgmann
Copy link
Contributor

B-20939

Summary

This task adds in the preparation date field for the SSW in the top right hand corner, and includes some other miscellaneous organization changes and test fixes to make the service easier to work with for upcoming features.

How to test

  1. Access the application as a service member.
  2. Create and submit a PPM with an advance request.
  3. Log in to the office app as a service counselor.
  4. Edit shipment details and approve the advance.
  5. Edit order details and insert missing data.
  6. Submit move information (This is the action that triggers AOA Preparation date as per AC)
  7. Download the AOA Packet. Ensure the preparation date is present.
  8. Log back in as service member, optionally download and check again.
  9. Complete the closeout process by uploading documents.
  10. Log back in as Service Counselor, approve the closeout documents. (This is the action that triggers the preparation date for the SSW per AC)
  11. Download the Payment Packet, ensure preparation date is present.
  12. Optionally, log back in as a customer and download the payment packet, ensure preparation date is present.

Backend

Screenshots

image

@antgmann antgmann added Scrummy Bears Scrum Team H INTEGRATION Slated for Integration Testing labels Aug 22, 2024
@antgmann antgmann self-assigned this Aug 22, 2024
@antgmann antgmann requested a review from a team as a code owner August 22, 2024 14:01
}
}
}
return "Uncertified"
Copy link
Contributor

@MInthavongsay MInthavongsay Aug 22, 2024

Choose a reason for hiding this comment

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

Lines 418 and 434: I'm assuming this is a worse case scenario? It should never get to this, right? You may want to throw an error and/or log here.

In addition, you want to create a generic method in place of formatAOADate/formatSSWDate methods that takes in the cert array, uuid and enum. It should return null if nothing matches. From there you can log or throw an error in the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

418 is what we discussed in the standup this morning. Right now, there's a way to download the AOA before the SC submits move details where their signature is added. Until we can revisit the aoa download availability, our PO just wants it to use the date the PDF generation was ran.

As for 434, this should never occur, so an error may be more appropriate here. If that's what we want to go with, I'll add it to the todo list for later today.

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've gone ahead and added the error handling to this method. Now, if there's ever an uncertified payment packet downloaded before it should be, it will error out. Honestly, making this sort of thing easier in this service was long overdue, so I think that's a great change.

As for the generic method idea, I'm going to push back against that because that's the solution I attempted initially. Due to the implementation in the way that the service determines which packet is being generated, as well as the best way for handling adding the dates themselves from the certs, and the two alternate solutions to when a date shouldn't be used... I believe the current solution is easier to read, easier to maintain, is marginally better performance-wise, and uses about the same amount of code as combining them.

I do realize that the amount of methods in this service is... kind of a lot, so I'd see why we'd want to cut down on them. But if I need more justification still, then an additional reason for adding 2 in this story is that these changes also removed a near-duplicate method in formatSignatureDate.

Just let me know if any objections.

Copy link
Contributor

@MInthavongsay MInthavongsay left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@ajlusk ajlusk left a comment

Choose a reason for hiding this comment

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

Date appears properly on both pages for both the AOA and Payment Packet. Regarding the case commented on above, I believe that was discussed with the PO at standup but I would also like confirmation on that.

Screenshot 2024-08-22 at 1 34 30 PM Screenshot 2024-08-22 at 1 34 15 PM

@antgmann antgmann merged commit c951716 into integrationTesting Aug 23, 2024
30 checks passed
@antgmann antgmann deleted the B-20939-INT branch August 23, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTEGRATION Slated for Integration Testing Scrummy Bears Scrum Team H
Development

Successfully merging this pull request may close these issues.

3 participants