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

[CST] Override the display name and status of "PMR Pending" requests. #32256

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[CST] Override the display name and status of "PMR Pending" requests. #32256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 60 additions & 33 deletions src/applications/claims-status/reducers/serialize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import { getTrackedItemDate } from '../utils/helpers';
import {
getTrackedItemDate,
getTrackedItemHumanReadableName,
} from '../utils/helpers';

// NOTE: I think that in the long term it will make sense to move
// this logic into the backend, but doing it here for now makes it
// easier to modify this in the short term
// NOTE: In the long term it will make sense to move some of this logic into
// the backend, but doing it here for now makes it easier to modify in
// the short-term.
//
// This does 2 things:
// 1) Grabs all of the supporting documents associated with
// tracked items and embeds them within their respective
// tracked items in a new `documents` key that has been
// added to each tracked item
// 2) Gets a new array of supporting documents that only
// includes the documents that are not associated with
// any tracked items. These items should already be
// embedded in their respective tracked items anyways,
// so they are no longer needed
// This does a few things:
// 1) Grabs all of the supporting documents associated with tracked items and
// embeds them within their respective tracked items in a new `documents`
// key that has been added to each tracked item
// 2) Gets a new array of supporting documents that only includes the documents
// that are not associated with any tracked items. These items should
// already be embedded in their respective tracked items anyways, so they
// are no longer needed
// 3) Performs any additional necessary transforms on supporting document and
// tracked item data, such as replacing display names and adding date
// attributes
const isLighthouseClaim = claim => claim.type === 'claim';

const isAssociatedWithAnyTrackedItem = doc => doc.trackedItemId !== null;
Expand All @@ -25,27 +29,49 @@ const getDocsAssociatedWithTrackedItems = docs =>
docs.filter(isAssociatedWithAnyTrackedItem);

const getDocsAssociatedWithTrackedItem = (item, docs) => {
const predicate = isAssociatedWithTrackedItem(item);
return docs.filter(predicate);
return docs.filter(isAssociatedWithTrackedItem(item));
};

const associateDocsWithTrackedItems = (items, docs) => {
return items.map(item => {
const newItem = { ...item };
const associatedDocs = getDocsAssociatedWithTrackedItem(newItem, docs);
newItem.documents = associatedDocs || [];
newItem.date = getTrackedItemDate(item);
return newItem;
});
return items.map(item => ({
Comment on lines -33 to -39
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned this up a little to make it more concise. Notable changes are:

  1. I removed the || [] for assigning documents, since I don't believe we can get here with a falsy return value for getDocsAssoociatedWithTrackedItems (since docs need to be an array already to run filter on them). For additional safety, I went ahead and added a default value (empty array) for both supportingDocuments and trackedItems at the beginning of the serializeClaim function.
  2. I moved setting date on the tracked item into the transform function, as I don't believe it's related to associating docs with the tracked item.

...item,
documents: getDocsAssociatedWithTrackedItem(item, docs),
}));
};

const filterAssociatedDocs = docs =>
docs.filter(doc => !isAssociatedWithAnyTrackedItem(doc));

const transformUnassociatedDocs = docs =>
docs.map(doc => ({
...doc,
date: doc.uploadDate,
}));

Comment on lines +45 to +49
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 was moved into its own function from the original return of serializeClaim.

const transformAssociatedTrackedItems = items =>
items.map(item => ({
...item,
// <tempfix>
// Override the first-party status of PMR (Private Medical Record) Pending
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have an established pattern I should use here for marking code as temporary? Ideally this would be some sort of consistent token we can search for during clean-up work.

// requests to third-party by replacing "NEEDED_FROM_YOU" with
// "NEEDED_FROM_OTHERS".
// See GH issue for more details:
// https://github.com/department-of-veterans-affairs/va.gov-team/issues/93456
// TODO: Remove this <tempfix> block once the Lighthouse API is returning
// PMR Pending requests with more accurate status information.
status:
item.displayName === 'PMR Pending' ? 'NEEDED_FROM_OTHERS' : item.status,
// </tempfix>
date: getTrackedItemDate(item),
// This might need to be removed / moved elsewhere if future application
// logic depends on the original `displayName` of the tracked item.
displayName: getTrackedItemHumanReadableName(item.displayName),
}));

export const serializeClaim = claim => {
if (!claim || !isLighthouseClaim(claim)) return claim;

const { supportingDocuments, trackedItems } = claim.attributes;
const { supportingDocuments = [], trackedItems = [] } = claim.attributes;

const associatedDocuments = getDocsAssociatedWithTrackedItems(
supportingDocuments,
Expand All @@ -56,19 +82,20 @@ export const serializeClaim = claim => {
associatedDocuments,
);

const transformedUnassociatedDocuments = transformUnassociatedDocs(
filterAssociatedDocs(supportingDocuments),
);

const transformedAssociatedTrackedItems = transformAssociatedTrackedItems(
associatedTrackedItems,
);

return {
...claim,
attributes: {
...claim.attributes,
supportingDocuments: filterAssociatedDocs(supportingDocuments).map(
doc => {
return {
...doc,
date: doc.uploadDate,
};
},
),
trackedItems: associatedTrackedItems,
supportingDocuments: transformedUnassociatedDocuments,
trackedItems: transformedAssociatedTrackedItems,
},
Comment on lines +97 to +98
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 variables are a bit wordy; if we don't find the labeling helpful, we can just run the relevant functions like so:

      supportingDocuments: transformUnassociatedDocs(
        filterAssociatedDocs(supportingDocuments),
      ),
      trackedItems: transformAssociatedTrackedItems(
        associatedTrackedItems,
      ),

The result is the same regardless.

};
};
60 changes: 60 additions & 0 deletions src/applications/claims-status/tests/reducers/serialize.unit.spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@ describe('Claim Serializer', () => {
});
});

it('should set supporting documents and tracked items to empty arrays when data is missing', () => {
const claim = {
id: 1,
type: 'claim',
attributes: {},
};
const serializedClaim = serializeClaim(claim);
expect(serializedClaim).to.eql({
...claim,
attributes: {
supportingDocuments: [],
trackedItems: [],
},
});
});

it('should return no supporting docs when there is only one and it is associated with a tracked item', () => {
const claim = {
id: 1,
Expand Down Expand Up @@ -136,4 +152,48 @@ describe('Claim Serializer', () => {
expect(trackedItem3.documents.length).to.be.equal(2);
expect(trackedItem3.documents.map(d => d.id).sort()).to.eql([3, 4]);
});

// <tempfix>
// See notes on PMR Pending in serialize.js
it('should manually override the status of PMR Pending requests', () => {
const claim = {
id: 1,
type: 'claim',
attributes: {
trackedItems: [
{
id: 1,
displayName: 'PMR Pending',
status: 'NEEDED_FROM_YOU',
},
],
},
};
const serializedClaim = serializeClaim(claim);
const trackedItem1 = serializedClaim.attributes.trackedItems.find(
d => d.id === 1,
);
expect(trackedItem1.status).to.eql('NEEDED_FROM_OTHERS');
});
// </tempfix>

it('should replace the display name of some types of tracked items', () => {
const claim = {
id: 1,
type: 'claim',
attributes: {
trackedItems: [
{
id: 1,
displayName: 'PMR Pending',
},
],
},
};
const serializedClaim = serializeClaim(claim);
const trackedItem1 = serializedClaim.attributes.trackedItems.find(
d => d.id === 1,
);
expect(trackedItem1.displayName).to.eql('Private Medical Record');
});
Comment on lines +156 to +198
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 tests are basically doing the same thing, but one tests the status while the other tests the displayName. Since testing the status is temporary, I kept them separate so that we can just remove that whole test case down the road.

});
11 changes: 11 additions & 0 deletions src/applications/claims-status/tests/utils/helpers.unit.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
sentenceCase,
generateClaimTitle,
isStandard5103Notice,
getTrackedItemHumanReadableName,
} from '../../utils/helpers';

import {
Expand Down Expand Up @@ -1542,4 +1543,14 @@ describe('Disability benefits helpers: ', () => {
});
});
});
describe('getTrackedItemHumanReadableName', () => {
it('should return a replacment name if one is available', () => {
expect(getTrackedItemHumanReadableName('PMR Pending')).to.equal(
'Private Medical Record',
);
});
it("should return the provided name if a replacement doesn't exist", () => {
expect(getTrackedItemHumanReadableName('test')).to.equal('test');
});
});
});
12 changes: 12 additions & 0 deletions src/applications/claims-status/utils/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,3 +1231,15 @@ export const getTrackedItemDateFromStatus = item => {
return item.requestedDate;
}
};

// Many of the display names for tracked items provided by the API are rather
// esoteric, so we replace some of them to make them more user-friendly.
const trackedItemsHumanReadableNames = new Map([
['PMR Pending', 'Private Medical Record'],
]);
// Currently only being used by reducers/serialize, but it might need to be
// moved elsewhere if future application logic depends on the original
// `displayName` of the tracked item.
export const getTrackedItemHumanReadableName = name => {
return trackedItemsHumanReadableNames.get(name) || name;
};
Loading