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

10464 Bug: Invalid documents in scenario dropdowns #5256

Conversation

Mwindo
Copy link
Contributor

@Mwindo Mwindo commented Aug 15, 2024

We do not want to show unserved, undocketed, or stricken documents from "What Document are You Filing?" dropdowns. This PR enforces these checks and updates tests accordingly.

@Mwindo Mwindo added the to test label Aug 15, 2024
Comment on lines +52 to +59
const mockDocuments = MOCK_DOCUMENTS.map(d => {
return {
...d,
isOnDocketRecord: true,
isStricken: false,
servedAt: '2019-08-25T05:00:00.000Z',
};
});
Copy link
Contributor Author

@Mwindo Mwindo Aug 15, 2024

Choose a reason for hiding this comment

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

MOCK_DOCUMENTS is used in a lot of places. I modified them here in the tests as needed to avoid affecting other tests.

Comment on lines +152 to +157
const formattedCaseDetail = applicationContext
.getUtilities()
.getFormattedCaseDetail({
applicationContext,
caseDetail,
});
Copy link
Contributor Author

@Mwindo Mwindo Aug 15, 2024

Choose a reason for hiding this comment

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

We already have logic on this function to determine if a document has been served or not.

@Mwindo Mwindo force-pushed the 10464-bug-intermediate-branch-to-test-1723757539 branch from 2418ece to 4644153 Compare August 15, 2024 21:43
@@ -733,6 +733,7 @@ export const baseState = {
pdfsAppended: 0,
totalPdfs: 0,
},
parentMessageId: undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were last-minute additions to 10102, in staging but not in test.

@@ -17,7 +16,6 @@ export const editUnsignedDraftDocumentSequence = [
isStatusReportOrder: [
setEditStatusReportOrderFormAction,
navigateToPathAction,
statusReportOrderPdfPreviewSequence,
Copy link
Contributor Author

@Mwindo Mwindo Aug 15, 2024

Choose a reason for hiding this comment

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

Thanks to #5254, I discovered this is a redundant call. I'm slipping it into this PR to avoid a whole new branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as you tell me you have high confidence that our test suite covers the functionality this would impact on the slim chance something did go wrong with this, then I think it's reasonable 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In addition to having tested manually, I've just double-checked that edit-status-report-order.cy.ts does check this (and fails successfully when I modify things further). Also, I've walked through the logic just to make sure I'm not lying to myself and taking advantage of your trust 🤣

We call statusReportOrderPdfPreviewSequence.ts in three places:

  1. navigateToEditOrderSequence.ts -- to show an existing order pdf if the user is editing
  2. gotoStatusReportOrderSequence.ts -- to show an existing order pdf if the user is editing
  3. StatusReportOrder.tsx -- to show the pdf when the user clicks "Preview"

In navigateToEditOrderSequence.ts, in the case of a status report order, we navigate to one of two "status-report-order-edit" routes, which both call gotoStatusReportOrderSequence.ts. However, in these edit cases, gotoStatusReportOrderSequence.ts itself calls statusReportOrderPdfPreviewSequence.ts. So any call to statusReportOrderPdfPreviewSequence.ts in navigateToEditOrderSequence.ts will always precede the call to statusReportOrderPdfPreviewSequence.ts in gotoStatusReportOrderSequence.ts and can thus be eliminated.

@@ -3,4 +3,4 @@ import { setFormValueAction } from '../../actions/setFormValueAction';
export const updateStatusReportOrderFormValueSequence = [
setFormValueAction,
clearJurisdictionRadioAction,
] as unknown as (props: { key: string; value: any }) => void;
] as unknown as (props: { key: string; value: string | boolean }) => void;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a last-minute addition to 10102, in staging but not in test.

@Mwindo Mwindo marked this pull request as ready for review August 15, 2024 21:50

export type ViewerDocument = {
docketEntryId: string;
documentTitle?: string; // Should this be required?
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe we can guarantee that there will be a documentTitle here, so I think this is correct

@Mwindo Mwindo merged commit ba09846 into ustaxcourt:test Aug 16, 2024
44 checks passed
@Mwindo Mwindo deleted the 10464-bug-intermediate-branch-to-test-1723757539 branch November 18, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants