Skip to content

Commit

Permalink
review workflow and fix author validation
Browse files Browse the repository at this point in the history
  • Loading branch information
lmd59 committed Oct 2, 2024
1 parent b7be28e commit 0515fe6
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 78 deletions.
90 changes: 40 additions & 50 deletions app/src/pages/review/[resourceType]/[id].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import ArtifactTimeline from '@/components/ArtifactTimeline';
*/
export default function CommentPage() {
const ctx = trpc.useContext();
const [dateSelected, setDateSelected] = useState(false);
const ref = useRef<HTMLInputElement>(null);
const [isLoading, setIsLoading] = useState(false);

Expand All @@ -48,8 +47,7 @@ export default function CommentPage() {
initialValues: {
type: '',
comment: '',
name: '',
date: ''
name: ''
},
// An error will be thrown if these fields aren't entered properly
validate: {
Expand All @@ -58,27 +56,37 @@ export default function CommentPage() {
}
});

// TODO: use review operation procedure instead
// Currently we can only update draft artifact resources.
// const resourceUpdate = trpc.draft.updateDraft.useMutation({
// onSuccess: () => {
// notifications.show({
// title: 'Comment Successfully added!',
// message: `Comment Successfully added to ${resourceType}/${resourceID}`,
// icon: <CircleCheck />,
// color: 'green'
// });
// ctx.draft.getDraftById.invalidate();
// },
// onError: e => {
// notifications.show({
// title: 'Update Failed!',
// message: `Attempt to update ${resourceType} failed with message: ${e.message}`,
// icon: <AlertCircle />,
// color: 'red'
// });
// }
// });

const utils = trpc.useUtils();
// Currently we can only update draft artifact resources. TODO: should we enable active resource review?
const resourceReview = trpc.draft.reviewDraft.useMutation({
onSuccess: (data) => {
notifications.show({
title: 'Review successfully added!',
message: `Review successfully added to ${resourceType}/${resourceID}`,
icon: <CircleCheck />,
color: 'green'
});
data.children.forEach(c => {
notifications.show({
title: 'Review successfully added!',
message: `Draft of child ${resourceType} artifact of url ${c.url} successfully reviewed`,
icon: <CircleCheck />,
color: 'green'
});
});
utils.draft.getDrafts.invalidate();
ctx.draft.getDraftById.invalidate();
},
onError: e => {
notifications.show({
title: 'Review Failed!',
message: `Attempt to review ${resourceType} failed with message: ${e.message}`,
icon: <AlertCircle />,
color: 'red'
});
}
});

function getResource() {
if (authoring === 'true') {
Expand Down Expand Up @@ -203,18 +211,7 @@ export default function CommentPage() {
/>
<Space h="md" />
<Group grow>
<TextInput radius="lg" label="Endorser Name" placeholder="Name" {...form.getInputProps('name')} />
<Checkbox
ref={ref}
id="checkbox"
color="white"
mt="md"
label="Include Date in Comment"
{...form.getInputProps('date')}
onChange={() => {
setDateSelected(!dateSelected);
}}
/>
<TextInput radius="lg" label="Endorser URI" placeholder="URI" {...form.getInputProps('name')} />
<Space h="md" />
</Group>
<Space h="md" />
Expand All @@ -226,27 +223,20 @@ export default function CommentPage() {
if (form.isValid()) {
setIsLoading(true);
setTimeout(() => {
setDateSelected(false);
form.reset();
if (ref?.current?.checked) {
ref.current.checked = false;
}
setIsLoading(false);
}, 1000);
if (authoring === 'true') {
// TODO: should be done as a $review operation
// const [additions, deletions] = parseUpdate(
// form.values.comment,
// form.values.type,
// form.values.name,
// dateSelected
// );
// resourceUpdate.mutate({
// resourceType: resourceType as ArtifactResourceType,
// id: resourceID as string,
// additions: additions,
// deletions: deletions
// });
resourceReview.mutate({
resourceType: resourceType as ArtifactResourceType,
id: resourceID as string,
type: form.values.type,
summary: form.values.comment,
author: form.values.name
});
}
}
}}
Expand Down
14 changes: 10 additions & 4 deletions app/src/server/trpc/routers/draft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,22 @@ export const draftRouter = router({

// passes in type, summary, and author from user (set date and target automatically)
reviewDraft: publicProcedure
.input(z.object({ id: z.string(), resourceType: z.enum(['Measure', 'Library']), type: z.enum(['documentation', 'guidance', 'review']), summary: z.string(), author: z.string() }))
.input(z.object({ id: z.string(), resourceType: z.enum(['Measure', 'Library']), type: z.string(), summary: z.string(), author: z.string() }))
.mutation(async ({ input }) => {
const raw = await fetch(`${process.env.MRS_SERVER}/${input.resourceType}/${input.id}`);
const resource = (await raw.json()) as FhirArtifact;
const date = new Date().toISOString();
const canonical = `${resource.url}|${resource.version}`;

const res = await fetch(`${process.env.MRS_SERVER}/${input.resourceType}/${input.id}/$review?reviewDate=${date}&artifactAssessmentType=${input.type}&artifactAssessmentSummary=${input.summary}&artifactAssessmentTarget=${canonical}&artifactAssessmentAuthor=${input.author}`);


const params = new URLSearchParams({
'reviewDate': date,
'artifactAssessmentType': input.type,
'artifactAssessmentSummary': input.summary,
'artifactAssessmentTarget': canonical,
'artifactAssessmentAuthor': input.author
});
const res = await fetch(`${process.env.MRS_SERVER}/${input.resourceType}/${input.id}/$review?${params}`);

if (res.status !== 200) {
const outcome: OperationOutcome = await res.json();
throw new Error(`Received ${res.status} error on $review: ${outcome.issue[0].details?.text}`);
Expand Down
4 changes: 2 additions & 2 deletions service/src/requestSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export const ApproveArgs = z
artifactAssessmentSummary: z.string().optional(),
artifactAssessmentTarget: checkUri.optional(),
artifactAssessmentRelatedArtifact: checkUri.optional(),
artifactAssessmentAuthor: z.object({ reference: z.string() }).optional()
artifactAssessmentAuthor: z.union([z.object({ reference: z.string()}).transform((val) => val.reference), z.string()]).optional() //object from POST or string from GET
})
.strict()
.superRefine(catchInvalidParams([catchMissingId, catchMissingTypeAndSummary]));
Expand All @@ -220,7 +220,7 @@ export const ReviewArgs = z
artifactAssessmentSummary: z.string().optional(),
artifactAssessmentTarget: checkUri.optional(),
artifactAssessmentRelatedArtifact: checkUri.optional(),
artifactAssessmentAuthor: z.object({ reference: z.string() }).optional()
artifactAssessmentAuthor: z.union([z.object({ reference: z.string()}).transform((val) => val.reference), z.string()]).optional() //object from POST or string from GET
})
.strict()
.superRefine(catchInvalidParams([catchMissingId, catchMissingTypeAndSummary]));
Expand Down
8 changes: 4 additions & 4 deletions service/src/services/LibraryService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export class LibraryService implements Service<CRMIShareableLibrary> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (library.extension) {
library.extension.push(comment);
Expand All @@ -348,7 +348,7 @@ export class LibraryService implements Service<CRMIShareableLibrary> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (child.extension) {
child.extension.push(comment);
Expand Down Expand Up @@ -403,7 +403,7 @@ export class LibraryService implements Service<CRMIShareableLibrary> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (library.extension) {
library.extension.push(comment);
Expand All @@ -424,7 +424,7 @@ export class LibraryService implements Service<CRMIShareableLibrary> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (child.extension) {
child.extension.push(comment);
Expand Down
8 changes: 4 additions & 4 deletions service/src/services/MeasureService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ export class MeasureService implements Service<CRMIShareableMeasure> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (measure.extension) {
measure.extension.push(comment);
Expand All @@ -354,7 +354,7 @@ export class MeasureService implements Service<CRMIShareableMeasure> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (child.extension) {
child.extension.push(comment);
Expand Down Expand Up @@ -409,7 +409,7 @@ export class MeasureService implements Service<CRMIShareableMeasure> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (measure.extension) {
measure.extension.push(comment);
Expand All @@ -430,7 +430,7 @@ export class MeasureService implements Service<CRMIShareableMeasure> {
parsedParams.artifactAssessmentSummary,
parsedParams.artifactAssessmentTarget,
parsedParams.artifactAssessmentRelatedArtifact,
parsedParams.artifactAssessmentAuthor?.reference
parsedParams.artifactAssessmentAuthor
);
if (child.extension) {
child.extension.push(comment);
Expand Down
58 changes: 52 additions & 6 deletions service/test/services/LibraryService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1194,16 +1194,39 @@ describe('LibraryService', () => {
id: 'approve-child1',
artifactAssessmentType: 'guidance',
artifactAssessmentSummary: 'Sample summary',
artifactAssessmentAuthor: { reference: 'Sample Author' }
artifactAssessmentAuthor: 'http://example.org/fhir/Organization/123'
})
.set('Accept', 'application/json+fhir')
.expect(200)
.then(response => {
expect(response.body.total).toEqual(2);
expect(response.body.entry[0].resource.date).toBeDefined();
expect(response.body.entry[0].resource.extension[0].extension[2].valueString).toEqual('Sample Author');
expect(response.body.entry[0].resource.extension[0].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
expect(response.body.entry[1].resource.date).toBeDefined();
expect(response.body.entry[1].resource.extension[1].extension[2].valueString).toEqual('Sample Author');
expect(response.body.entry[1].resource.extension[1].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
});
});

it('returns 200 status with a Bundle result containing the updated parent Library artifact and any children it has for POST /Library/$approve', async () => {
await supertest(server.app)
.post('/4_0_1/Library/$approve')
.send({
resourceType: 'Parameters',
parameter: [
{ name: 'id', valueString: 'approve-child1' },
{ name: 'artifactAssessmentType', valueCode: 'guidance' },
{ name: 'artifactAssessmentSummary', valueString: 'Sample summary' },
{ name: 'artifactAssessmentAuthor', valueReference: { reference: 'http://example.org/fhir/Organization/123' } }
]
})
.set('content-type', 'application/fhir+json')
.expect(200)
.then(response => {
expect(response.body.total).toEqual(2);
expect(response.body.entry[0].resource.date).toBeDefined();
expect(response.body.entry[0].resource.extension[0].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
expect(response.body.entry[1].resource.date).toBeDefined();
expect(response.body.entry[1].resource.extension[1].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
});
});

Expand Down Expand Up @@ -1310,16 +1333,39 @@ describe('LibraryService', () => {
id: 'review-child1',
artifactAssessmentType: 'guidance',
artifactAssessmentSummary: 'Sample summary',
artifactAssessmentAuthor: { reference: 'Sample Author' }
artifactAssessmentAuthor: 'http://example.org/fhir/Organization/123'
})
.set('Accept', 'application/json+fhir')
.expect(200)
.then(response => {
expect(response.body.total).toEqual(2);
expect(response.body.entry[0].resource.date).toBeDefined();
expect(response.body.entry[0].resource.extension[0].extension[2].valueString).toEqual('Sample Author');
expect(response.body.entry[0].resource.extension[0].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
expect(response.body.entry[1].resource.date).toBeDefined();
expect(response.body.entry[1].resource.extension[1].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
});
});

it('returns 200 status with a Bundle result containing the updated parent Library artifact and any children it has for POST /Library/$review', async () => {
await supertest(server.app)
.post('/4_0_1/Library/$review')
.send({
resourceType: 'Parameters',
parameter: [
{ name: 'id', valueString: 'review-child1' },
{ name: 'artifactAssessmentType', valueCode: 'guidance' },
{ name: 'artifactAssessmentSummary', valueString: 'Sample summary' },
{ name: 'artifactAssessmentAuthor', valueReference: { reference: 'http://example.org/fhir/Organization/123' } }
]
})
.set('content-type', 'application/fhir+json')
.expect(200)
.then(response => {
expect(response.body.total).toEqual(2);
expect(response.body.entry[0].resource.date).toBeDefined();
expect(response.body.entry[0].resource.extension[0].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
expect(response.body.entry[1].resource.date).toBeDefined();
expect(response.body.entry[1].resource.extension[1].extension[2].valueString).toEqual('Sample Author');
expect(response.body.entry[1].resource.extension[1].extension[2].valueString).toEqual('http://example.org/fhir/Organization/123');
});
});

Expand Down
Loading

0 comments on commit 0515fe6

Please sign in to comment.