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

PIMS-1929: Unknown status for project notifications #2628

Merged
merged 8 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ export const getNotificationsByProjectId = async (req: Request, res: Response) =

return res.status(200).send(notificationsResult);
} catch (error) {
// not sure if the error codes can be handled better here?
// not sure if the 401 and 500 error codes can be handled better here?
// if the ches credentials are incorrect, we don't seem to catch the 401 error in the notificationService
return res.status(500).send({ message: 'Error fetching notifications' });
}
};
Expand Down
53 changes: 51 additions & 2 deletions express-api/src/services/notifications/notificationServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export enum NotificationStatus {
Cancelled = 2,
Failed = 3,
Completed = 4,
NotFound = 5,
Invalid = 6,
Unauthorized = 7,
}

export enum NotificationAudience {
Expand Down Expand Up @@ -442,6 +445,12 @@ const convertChesStatusToNotificationStatus = (chesStatus: string): Notification
return NotificationStatus.Failed;
case 'completed':
return NotificationStatus.Completed;
case '404':
return NotificationStatus.NotFound;
case '422':
return NotificationStatus.Invalid;
case '401':
return NotificationStatus.Unauthorized;
dbarkowsky marked this conversation as resolved.
Show resolved Hide resolved
default:
return null;
}
Expand All @@ -465,7 +474,6 @@ const updateNotificationStatus = async (notificationId: number, user: User) => {
}

const statusResponse = await chesServices.getStatusByIdAsync(notification.ChesMessageId);

if (typeof statusResponse?.status === 'string') {
const notificationStatus = convertChesStatusToNotificationStatus(statusResponse.status);
// If the CHES status is non-standard, don't update the notification.
Expand All @@ -481,7 +489,48 @@ const updateNotificationStatus = async (notificationId: number, user: User) => {
query.release();
return updatedNotification;
} else if (typeof statusResponse?.status === 'number') {
//If we get number type then this wound up being some HTTP code.
// handle 404, 422 code here....
switch (statusResponse.status) {
case 404: {
notification.Status = NotificationStatus.NotFound;
notification.UpdatedOn = new Date();
notification.UpdatedById = user.Id;
const updatedNotification = await query.manager.save(NotificationQueue, notification);
logger.error(`Notification with id ${notificationId} not found on CHES.`);
query.release();
return updatedNotification;
}
case 422: {
notification.Status = NotificationStatus.Invalid;
notification.UpdatedOn = new Date();
notification.UpdatedById = user.Id;
const updatedNotification = await query.manager.save(NotificationQueue, notification);
logger.error(
`Notification with id ${notificationId} could not be processed, some of the data could be formatted incorrectly.`,
);
query.release();
return updatedNotification;
}
case 401: {
notification.Status = NotificationStatus.Unauthorized;
notification.UpdatedOn = new Date();
notification.UpdatedById = user.Id;
const updatedNotification = await query.manager.save(NotificationQueue, notification);
logger.error(
`Cannot authorize the request to the CHES server, check your CHES credentials.`,
);
query.release();
return updatedNotification;
}
dbarkowsky marked this conversation as resolved.
Show resolved Hide resolved
case 500:
logger.error(
`Internal server error while retrieving status for notification with id ${notificationId}.`,
);
break;
default:
logger.error(`Received unexpected status code ${statusResponse.status}.`);
break;
}
query.release();
return notification;
} else {
Expand Down
6 changes: 6 additions & 0 deletions react-app/src/constants/chesNotificationStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ export enum NotificationStatus {
Cancelled = 2,
Failed = 3,
Completed = 4,
NotFound = 5,
Invalid = 6,
}

export const getStatusString = (status: number): string => {
Expand All @@ -18,6 +20,10 @@ export const getStatusString = (status: number): string => {
return 'Failed';
case NotificationStatus.Completed:
return 'Completed';
case NotificationStatus.NotFound:
return 'Not Found';
case NotificationStatus.Invalid:
return 'Invalid';
dbarkowsky marked this conversation as resolved.
Show resolved Hide resolved
default:
return 'Unknown';
}
Expand Down
Loading