From 975269640c3a0117aa22c50766729987642a2d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20H=C3=B6ld?= Date: Mon, 20 May 2019 15:03:16 +0200 Subject: [PATCH] Change and improve implementation for notifications Pagnination The implementation for fetching notifications is changed. Instead of keeping offset and limit in the state, the page is fetched by page number instead (similar to the history). The issue of overfetching on the last page was solved by calculating the number of remaining items to be fetched for the last page. Closes #288 Prevent assignee selection from overflowing When the name of the assignee is too long it gets shortened by ellipses in the workflow item table. Closes #299 Display parent project and subproject name for notifications The displaynames of the project and subproject are now displayed on the notification page for each entry. If the user does not have permissions to see the project/subproject it gets replaced by the word "Redacted". Closes #298 Display correct names in notifications If the user does not have the permission to see a project/subproject/workflowitem, the displayname is not displayed in the notification and is instead left blank. Closes #292 Refactor Helpers - Functions that are not used were removed - Other functions were re-written for code clarity Move 'Read All' button To increase usability, the 'Read All' button was moved to the left side of the page. It is now above the icon that indicates whether the notification has been read which makes its purpose clearer. The color has also been changed to black instead of blue. Closes #301 Background color for unread notifications To increase usability, the background color for unread notifications has been changed to a light gray color. Closes #300 Remove view button if disabled - The 'view' button has been changed to an icon button instead of floating action button to make the look a little leaner - The button has been surrounded by a tooltip which tells the user what the button does - If the user does not have the permissions to see the project/subproject/workflowitem, the button is not displayed instead of disabled Closes #302 --- .../cypress/integration/notification_spec.js | 1 + .../integration/workflowitem_history_spec.js | 2 +- frontend/src/helper.js | 135 ++---------------- .../pages/Notifications/FlyInNotifications.js | 16 ++- .../pages/Notifications/NotificationList.js | 103 ++++--------- .../Notifications/NotificationListItems.js | 40 ++++-- .../pages/Notifications/NotificationPage.js | 4 +- .../NotificationPageContainer.js | 21 ++- frontend/src/pages/Notifications/actions.js | 38 ++--- frontend/src/pages/Notifications/helper.js | 97 ++++++++++--- frontend/src/pages/Notifications/reducer.js | 19 +-- .../Workflows/WorkflowAssigneeContainer.js | 4 +- frontend/src/sagas.js | 66 +++++---- 13 files changed, 230 insertions(+), 316 deletions(-) diff --git a/e2e-test/cypress/integration/notification_spec.js b/e2e-test/cypress/integration/notification_spec.js index 540d1d1b56..6b94bb0496 100644 --- a/e2e-test/cypress/integration/notification_spec.js +++ b/e2e-test/cypress/integration/notification_spec.js @@ -13,6 +13,7 @@ describe("open notifications", function() { .then(() => cy.closeProject(projectId, "project.close", assignee)) .then(() => cy.login("jxavier")) .then(() => cy.visit("/notifications")) + .then(() => cy.get("[data-test=notification-unread-0]").should("be.visible")) ); }); diff --git a/e2e-test/cypress/integration/workflowitem_history_spec.js b/e2e-test/cypress/integration/workflowitem_history_spec.js index be8bca7c6c..60211ad013 100644 --- a/e2e-test/cypress/integration/workflowitem_history_spec.js +++ b/e2e-test/cypress/integration/workflowitem_history_spec.js @@ -45,7 +45,7 @@ describe("Workflowitem's history", function() { it("The history is sorted from new to old", function() { // Change assignee to create new history event - cy.get(".workflowitem-assignee").click(); + cy.get("[data-test=workflowitem-assignee]").click(); cy.get("[role=listbox]") .find("[value=jdoe]") .click() diff --git a/frontend/src/helper.js b/frontend/src/helper.js index 7bac4f67a3..3217a7fe07 100644 --- a/frontend/src/helper.js +++ b/frontend/src/helper.js @@ -1,29 +1,24 @@ -import React from "react"; -import { Iterable } from "immutable"; -import dayjs from "dayjs"; - -import OpenIcon from "@material-ui/icons/Remove"; import DoneIcon from "@material-ui/icons/Check"; - -import indigo from "@material-ui/core/colors/indigo"; - +import OpenIcon from "@material-ui/icons/Remove"; +import accounting from "accounting"; +import dayjs from "dayjs"; +import { Iterable } from "immutable"; +import _cloneDeep from "lodash/cloneDeep"; import _isEmpty from "lodash/isEmpty"; import _isEqual from "lodash/isEqual"; -import _cloneDeep from "lodash/cloneDeep"; -import _isUndefined from "lodash/isUndefined"; import _isString from "lodash/isString"; +import _isUndefined from "lodash/isUndefined"; +import React from "react"; -import accounting from "accounting"; -import strings from "./localizeStrings"; import currencies from "./currency"; +import strings from "./localizeStrings"; + const numberFormat = { decimal: ".", thousand: ",", precision: 2 }; -const statusColors = [indigo[100], indigo[300]]; - export const toJS = WrappedComponent => wrappedComponentProps => { const KEY = 0; const VALUE = 1; @@ -71,12 +66,6 @@ export const fromAmountString = (amount, currency) => { return accounting.unformat(amount, getCurrencyFormat(currency).decimal); }; -export const formatAmountString = (amount, currency) => { - if (_isString(amount) && amount.trim().length <= 0) { - return ""; - } - return amount; -}; export const getCurrencies = () => { return Object.keys(currencies).map(currency => { return { @@ -97,11 +86,6 @@ export const toAmountString = (amount, currency) => { return accounting.formatMoney(amount, getCurrencyFormat(currency)); }; -export const tsToString = ts => { - let dateString = dayjs.unix(ts).format("MMM D, YYYY"); - return dateString; -}; - export const unixTsToString = ts => { let dateString = dayjs.unix(ts).format("MMM D, YYYY"); return dateString; @@ -136,112 +120,11 @@ export const statusIconMapping = { open: }; -export const roleMapper = { - approver: strings.common.approver, - bank: strings.common.bank, - assignee: strings.common.assignee -}; - -export const createDoughnutData = (labels, data, colors = statusColors) => ({ - labels, - datasets: [ - { - data: data, - backgroundColor: colors, - hoverBackgroundColor: colors - } - ] -}); - -export const calculateUnspentAmount = items => { - const amount = items.reduce((acc, item) => { - return acc + parseFloat(item.data.amount, 10); - }, 0); - return amount; -}; - -export const getCompletionRatio = subprojects => { - const completedSubprojects = getCompletedSubprojects(subprojects); - const percentageCompleted = completedSubprojects.length / subprojects.length * 100; - return percentageCompleted > 0 ? percentageCompleted : 0; -}; - -const getCompletedSubprojects = subprojects => { - const completedSubprojects = subprojects.filter(subproject => { - return subproject.data.status === "closed"; - }); - return completedSubprojects; -}; - -export const getCompletionString = subprojects => { - const completedSubprojects = getCompletedSubprojects(subprojects); - return strings.formatString( - strings.subproject.subproject_completion_string, - completedSubprojects.length, - subprojects.length - ); -}; - export const formatString = (text, ...args) => { return strings.formatString(text, ...args); }; -export const formatUpdateString = (identifier, createdBy, data) => { - let string = strings.formatString(strings.history.changed_by, identifier, createdBy); - const changes = Object.keys(data) - .map(key => formatString(strings.history.to, key, data[key])) - .join(", "); - return string.concat(changes); -}; - -export const calculateWorkflowBudget = workflows => { - return workflows.reduce( - (acc, workflow) => { - const { amount, amountType, status } = workflow.data; - const parsedAmount = parseFloat(amount, 10); - const next = { - assigned: amountType === "allocated" ? acc.assigned + parsedAmount : acc.assigned, - disbursed: amountType === "disbursed" ? acc.disbursed + parsedAmount : acc.disbursed, - currentDisbursement: - amountType === "disbursed" && status === "closed" - ? acc.currentDisbursement + parsedAmount - : acc.currentDisbursement - }; - return next; - }, - { - assigned: 0, - disbursed: 0, - currentDisbursement: 0 - } - ); -}; - -export const getNotAssignedBudget = (amount, assignedBudget, disbursedBudget) => { - const notAssigned = amount - assignedBudget - disbursedBudget; - return notAssigned >= 0 ? notAssigned : 0; -}; - -export const getProgressInformation = items => { - let startValue = { - open: 0, - closed: 0 - }; - const projectStatus = items.reduce((acc, item) => { - const status = item.data.status; - return { - open: status === "open" ? acc.open + 1 : acc.open, - closed: status === "closed" ? acc.closed + 1 : acc.closed - }; - }, startValue); - return projectStatus; -}; export const preselectCurrency = (parentCurrency, setCurrency) => { const preSelectedCurrency = _isUndefined(parentCurrency) ? "EUR" : parentCurrency; setCurrency(preSelectedCurrency); }; - -export const createTaskData = (items, type) => { - const projectStatus = getProgressInformation(items); - return createDoughnutData([strings.common.open, strings.common.closed], [projectStatus.open, projectStatus.closed]); -}; diff --git a/frontend/src/pages/Notifications/FlyInNotifications.js b/frontend/src/pages/Notifications/FlyInNotifications.js index a1c430bd84..807f158e9a 100644 --- a/frontend/src/pages/Notifications/FlyInNotifications.js +++ b/frontend/src/pages/Notifications/FlyInNotifications.js @@ -43,13 +43,15 @@ export default class FlyInNotification extends Component { {publisher ? publisher[0].toString().toUpperCase() : "?"}} action={ - history.push(parseURI({ projectId, subprojectId }))} - > - - + isAllowedToSee(notification) ? ( + history.push(parseURI({ projectId, subprojectId }))} + > + + + ) : null } title={getDisplayName(notification)} /> diff --git a/frontend/src/pages/Notifications/NotificationList.js b/frontend/src/pages/Notifications/NotificationList.js index 3434a784f9..421988c6bf 100644 --- a/frontend/src/pages/Notifications/NotificationList.js +++ b/frontend/src/pages/Notifications/NotificationList.js @@ -22,54 +22,22 @@ const styles = { } }; -const markPageAsRead = (markMultipleNotificationsAsRead, notifications, notificationOffset, notificationsPerPage) => { +const markPageAsRead = (markMultipleNotificationsAsRead, notifications, notificationPage) => { const notificationIds = notifications.map(notification => notification.id); - markMultipleNotificationsAsRead(notificationIds, notificationOffset, notificationsPerPage); + markMultipleNotificationsAsRead(notificationIds, notificationPage); }; const onChangeRowsPerPage = ( - event, + newNotificationsPerPage, setNotifcationsPerPage, - notificationOffset, fetchNotifications, - setNotificationOffset -) => { - let offset = notificationOffset; - if (offset < event.target.value) { - offset = 0; - } else if (offset % event.target.value > 0) { - offset = offset - offset % event.target.value; - } - setNotifcationsPerPage(event.target.value); - fetchNotifications(offset, event.target.value); - setNotificationOffset(offset); -}; - -const onChangePage = ( - nextPage, currentPage, - notificationOffset, - notificationsPerPage, - fetchNotifications, - setNotificationOffset + notificationsPerPage ) => { - if (nextPage > currentPage) { - //Moving forward - let offset = 0; - if (nextPage > 0) { - offset = notificationOffset + notificationsPerPage; - } - fetchNotifications(offset, notificationsPerPage); - setNotificationOffset(offset); - } else { - //moving backward - let offset = 0; - if (nextPage > 0) { - offset = notificationOffset - notificationsPerPage; - } - fetchNotifications(offset, notificationsPerPage); - setNotificationOffset(offset); - } + setNotifcationsPerPage(newNotificationsPerPage); + // Fetching notifications based on the first notification displayed on the page + const pageWithNewPageSize = Math.max(0, currentPage - 1) * Math.floor(notificationsPerPage / newNotificationsPerPage); + fetchNotifications(pageWithNewPageSize); }; const NotificationsList = props => { @@ -82,37 +50,33 @@ const NotificationsList = props => { fetchNotifications, notificationCount, notificationOffset, - setNotificationOffset, history, - markNotificationAsRead + markNotificationAsRead, + currentPage } = props; const allNotificationsRead = notifications.some(notification => notification.isRead === false); const rowsPerPageOptions = [5, 10, 20, 50]; - const currentPage = Math.floor(notificationOffset / notificationsPerPage); return ( - - markPageAsRead(markMultipleNotificationsAsRead, notifications, notificationOffset, notificationsPerPage) - } - color="primary" - className={classes.button + " mark-all-notifications-as-read"} - data-test="read-multiple-notifications" - disabled={!allNotificationsRead} - > - {strings.notification.read_all} - - } - /> + +
+ +
+ markNotificationAsRead(notificationId, currentPage)} notificationsPerPage={notificationsPerPage} notificationOffset={notificationOffset} /> @@ -124,25 +88,16 @@ const NotificationsList = props => { rowsPerPage={notificationsPerPage} onChangeRowsPerPage={event => onChangeRowsPerPage( - event, + event.target.value, setNotifcationsPerPage, - notificationOffset, fetchNotifications, - setNotificationOffset + currentPage, + notificationsPerPage ) } count={notificationCount} page={currentPage} - onChangePage={(_, nextPage) => - onChangePage( - nextPage, - currentPage, - notificationOffset, - notificationsPerPage, - fetchNotifications, - setNotificationOffset - ) - } + onChangePage={(_, nextPage) => fetchNotifications(nextPage)} /> diff --git a/frontend/src/pages/Notifications/NotificationListItems.js b/frontend/src/pages/Notifications/NotificationListItems.js index 02254c60e2..7938754999 100644 --- a/frontend/src/pages/Notifications/NotificationListItems.js +++ b/frontend/src/pages/Notifications/NotificationListItems.js @@ -1,6 +1,7 @@ import { withStyles } from "@material-ui/core/styles"; +import IconButton from "@material-ui/core/IconButton"; +import Tooltip from "@material-ui/core/Tooltip"; import Divider from "@material-ui/core/Divider"; -import Fab from "@material-ui/core/Fab"; import ListItem from "@material-ui/core/ListItem"; import ListItemIcon from "@material-ui/core/ListItemIcon"; import ListItemText from "@material-ui/core/ListItemText"; @@ -10,8 +11,8 @@ import LaunchIcon from "@material-ui/icons/ZoomIn"; import dayjs from "dayjs"; import React from "react"; -import { intentMapping, parseURI, isAllowedToSee } from "./helper"; - +import { intentMapping, parseURI, getParentData, isAllowedToSee } from "./helper"; +import strings from "../../localizeStrings"; const styles = theme => ({ row: { display: "flex", @@ -41,6 +42,12 @@ const styles = theme => ({ } }); +const styling = { + unreadMessage: { + backgroundColor: "#f1f1f1" + } +}; + const NotificationListItems = ({ classes, notifications, @@ -59,6 +66,7 @@ const NotificationListItems = ({ subprojectId: metadata.subproject ? metadata.subproject.id : undefined }); const testLabel = `notification-${isRead ? "read" : "unread"}`; + const { projectDisplayName, subprojectDisplayName } = getParentData(notification); return (
@@ -69,12 +77,17 @@ const NotificationListItems = ({ button={isRead ? false : true} data-test={testLabel} onClick={isRead ? undefined : () => markNotificationAsRead(id, notificationOffset, notificationsPerPage)} + style={!isRead ? styling.unreadMessage : null} >
{isRead ? : }
- - +
- history.push(redirectUri)} - > - - + {isAllowedToSee(notification) ? ( + +
+ history.push(redirectUri)}> + + +
+ + ) : null}
diff --git a/frontend/src/pages/Notifications/NotificationPage.js b/frontend/src/pages/Notifications/NotificationPage.js index 508da4b66a..280a818d69 100644 --- a/frontend/src/pages/Notifications/NotificationPage.js +++ b/frontend/src/pages/Notifications/NotificationPage.js @@ -12,7 +12,7 @@ const NotificationPage = ({ fetchNotifications, notificationCount, notificationOffset, - setNotificationOffset + currentPage }) => { return (
@@ -25,8 +25,8 @@ const NotificationPage = ({ notificationsPerPage={notificationsPerPage} fetchNotifications={fetchNotifications} notificationCount={notificationCount} - setNotificationOffset={setNotificationOffset} notificationOffset={notificationOffset} + currentPage={currentPage} />
); diff --git a/frontend/src/pages/Notifications/NotificationPageContainer.js b/frontend/src/pages/Notifications/NotificationPageContainer.js index 819a13876b..188c8e6d7b 100644 --- a/frontend/src/pages/Notifications/NotificationPageContainer.js +++ b/frontend/src/pages/Notifications/NotificationPageContainer.js @@ -5,7 +5,6 @@ import { markNotificationAsRead, fetchNotifications, setNotifcationsPerPage, - setNotificationOffset, markMultipleNotificationsAsRead, enableLiveUpdates, fetchNotificationCounts, @@ -18,7 +17,7 @@ import { toJS } from "../../helper"; class NotificationPageContainer extends Component { componentWillMount() { - this.props.fetchNotifications(this.props.notificationOffset, this.props.notificationsPerPage); + this.props.fetchNotifications(this.props.currentPage); this.props.disableLiveUpdates(); } @@ -38,13 +37,11 @@ class NotificationPageContainer extends Component { const mapDispatchToProps = (dispatch, props) => { return { - fetchNotifications: (offset, limit) => dispatch(fetchNotifications(true, offset, limit)), - markNotificationAsRead: (notificationId, offset, limit) => - dispatch(markNotificationAsRead(notificationId, offset, limit)), - markMultipleNotificationsAsRead: (notificationIds, offset, limit) => - dispatch(markMultipleNotificationsAsRead(notificationIds, offset, limit)), - setNotifcationsPerPage: limit => dispatch(setNotifcationsPerPage(limit)), - setNotificationOffset: offset => dispatch(setNotificationOffset(offset)), + fetchNotifications: page => dispatch(fetchNotifications(true, page)), + markNotificationAsRead: (notificationId, page) => dispatch(markNotificationAsRead(notificationId, page)), + markMultipleNotificationsAsRead: (notificationIds, page) => + dispatch(markMultipleNotificationsAsRead(notificationIds, page)), + setNotifcationsPerPage: notificationPageSize => dispatch(setNotifcationsPerPage(notificationPageSize)), enableLiveUpdates: () => dispatch(enableLiveUpdates()), disableLiveUpdates: () => dispatch(disableLiveUpdates()), fetchNotificationCounts: () => dispatch(fetchNotificationCounts()) @@ -54,10 +51,10 @@ const mapDispatchToProps = (dispatch, props) => { const mapStateToProps = state => { return { notifications: state.getIn(["notifications", "notifications"]), - notificationsPerPage: state.getIn(["notifications", "notificationsPerPage"]), + notificationsPerPage: state.getIn(["notifications", "notificationPageSize"]), unreadNotificationCount: state.getIn(["notifications", "unreadNotificationCount"]), - notificationCount: state.getIn(["notifications", "notificationCount"]), - notificationOffset: state.getIn(["notifications", "notificationOffset"]) + notificationCount: state.getIn(["notifications", "totalNotificationCount"]), + currentPage: state.getIn(["notifications", "currentNotificationPage"]) }; }; diff --git a/frontend/src/pages/Notifications/actions.js b/frontend/src/pages/Notifications/actions.js index fa6ede161b..353f0c1706 100644 --- a/frontend/src/pages/Notifications/actions.js +++ b/frontend/src/pages/Notifications/actions.js @@ -20,11 +20,11 @@ export const FETCH_ALL_NOTIFICATIONS_SUCCESS = "FETCH_ALL_NOTIFICATIONS_SUCCESS" export const LIVE_UPDATE_NOTIFICATIONS = "LIVE_UPDATE_NOTIFICATIONS"; export const LIVE_UPDATE_NOTIFICATIONS_SUCCESS = "LIVE_UPDATE_NOTIFICATIONS_SUCCESS"; -export const MARK_MULTIPLE_NOTIFICATION_AS_READ = "MARK_MULTIPLE_NOTIFICATION_AS_READ"; -export const MARK_MULTIPLE_NOTIFICATION_AS_READ_SUCCESS = "MARK_MULTIPLE_NOTIFICATION_AS_READ_SUCCESS"; +export const MARK_MULTIPLE_NOTIFICATIONS_AS_READ = "MARK_MULTIPLE_NOTIFICATIONS_AS_READ"; +export const MARK_MULTIPLE_NOTIFICATIONS_AS_READ_SUCCESS = "MARK_MULTIPLE_NOTIFICATIONS_AS_READ_SUCCESS"; -export const FETCH_NOTIFICATION_COUNTS = "FETCH_NOTIFICATION_COUNTS"; -export const FETCH_NOTIFICATION_COUNTS_SUCCESS = "FETCH_NOTIFICATION_COUNTS_SUCCESS"; +export const FETCH_NOTIFICATION_COUNT = "FETCH_NOTIFICATION_COUNT"; +export const FETCH_NOTIFICATION_COUNT_SUCCESS = "FETCH_NOTIFICATION_COUNT_SUCCESS"; export const SET_NOTIFICATIONS_PER_PAGE = "SET_NOTIFICATIONS_PER_PAGE"; export const SET_NOTIFICATION_OFFSET = "SET_NOTIFICATION_OFFSET"; @@ -60,28 +60,26 @@ export function updateNotification(showLoading = false, offset) { offset }; } -export function fetchNotifications(showLoading = false, offset, limit) { +export function fetchNotifications(showLoading = false, notificationPage) { return { type: FETCH_ALL_NOTIFICATIONS, showLoading, - offset, - limit + notificationPage }; } export function fetchNotificationCounts(showLoading = false) { return { - type: FETCH_NOTIFICATION_COUNTS, + type: FETCH_NOTIFICATION_COUNT, showLoading }; } -export function markNotificationAsRead(notificationId, offset, limit) { +export function markNotificationAsRead(notificationId, notificationPage) { return { type: MARK_NOTIFICATION_AS_READ, notificationId, - offset, - limit + notificationPage }; } @@ -105,26 +103,18 @@ export function fetchHistoryItems(project, offset, limit) { }; } -export function markMultipleNotificationsAsRead(notificationIds, offset, limit) { +export function markMultipleNotificationsAsRead(notificationIds, notificationPage) { return { - type: MARK_MULTIPLE_NOTIFICATION_AS_READ, + type: MARK_MULTIPLE_NOTIFICATIONS_AS_READ, notificationIds, - offset, - limit + notificationPage }; } -export function setNotifcationsPerPage(limit) { +export function setNotifcationsPerPage(notificationPageSize) { return { type: SET_NOTIFICATIONS_PER_PAGE, - limit - }; -} - -export function setNotificationOffset(offset) { - return { - type: SET_NOTIFICATION_OFFSET, - offset + notificationPageSize: notificationPageSize }; } diff --git a/frontend/src/pages/Notifications/helper.js b/frontend/src/pages/Notifications/helper.js index 2a40de9654..0438210d5e 100644 --- a/frontend/src/pages/Notifications/helper.js +++ b/frontend/src/pages/Notifications/helper.js @@ -1,29 +1,91 @@ import strings from "../../localizeStrings"; import { formatString } from "../../helper"; -// get hierarchic deepest displayName -export function getDisplayName(notification) { +const project = "Project"; +const subproject = "Subproject"; +const workflowitem = "Workflowitem"; + +export function getParentData(notification) { + const redacted = "Redacted"; const metadata = notification.metadata; + let projectDisplayName = ""; + let subprojectDisplayName = ""; + if (metadata !== undefined) { - if (metadata.workflowitem !== undefined && metadata.workflowitem.hasViewPermissions === true) { - return metadata.workflowitem.displayName; - } - if (metadata.subproject !== undefined && metadata.subproject.hasViewPermissions === true) { - return metadata.subproject.displayName; + if (metadata.project) { + projectDisplayName = isAllowedToSeeType(metadata, project) ? getDisplayName(metadata, project) : redacted; + if (metadata.subproject) { + subprojectDisplayName = isAllowedToSeeType(metadata, subproject) + ? getDisplayName(metadata, subproject) + : redacted; + } } - if (metadata.project !== undefined && metadata.project.hasViewPermissions === true) { - return metadata.project.displayName; + } + + return { + projectDisplayName, + subprojectDisplayName + }; +} + +export const getType = notificationMetaData => { + if (notificationMetaData.project !== undefined) { + if (notificationMetaData.subproject !== undefined) { + if (notificationMetaData.workflowitem !== undefined) { + return workflowitem; + } + return subproject; } + return project; + } + return null; +}; + +export function getDisplayName(notificationMetaData, type) { + if (type === project) { + return notificationMetaData.project.displayName; + } + if (type === subproject) { + return notificationMetaData.subproject.displayName; + } + if (type === workflowitem) { + return notificationMetaData.workflowitem.displayName; } return ""; } +const isAllowedToSeeType = (notificationMetaData, type) => { + if (type === project) { + return notificationMetaData.project.hasViewPermissions; + } + if (type === subproject) { + return notificationMetaData.subproject.hasViewPermissions; + } + if (type === workflowitem) { + return notificationMetaData.workflowitem.hasViewPermissions; + } +}; + export const intentMapping = notification => { const businessEvent = notification.businessEvent; + if (!businessEvent) { + console.warn("Notification has no business event"); + return ""; + } const translation = strings.notification[businessEvent.type]; - const displayName = getDisplayName(notification); - const text = formatString(translation, displayName); - return `${text} ${isAllowedToSee(notification) ? "" : strings.notification.no_permissions}`; + + const notificationMetaData = notification.metadata; + if (!notificationMetaData) { + console.warn("Notification has no metadata"); + return ""; + } + + const type = getType(notificationMetaData); + const displayName = getDisplayName(notificationMetaData, type); + const text = isAllowedToSeeType(notificationMetaData, type) + ? formatString(translation, displayName) + : formatString(translation, ""); + return `${text} ${isAllowedToSeeType(notificationMetaData, type) ? "" : strings.notification.no_permissions}`; }; export const parseURI = ({ projectId, subprojectId }) => { @@ -39,15 +101,8 @@ export const parseURI = ({ projectId, subprojectId }) => { export const isAllowedToSee = notification => { const metadata = notification.metadata; if (metadata !== undefined) { - if (metadata.workflowitem !== undefined) { - return metadata.workflowitem.hasViewPermissions; - } - if (metadata.subproject !== undefined) { - return metadata.subproject.hasViewPermissions; - } - if (metadata.project !== undefined) { - return metadata.project.hasViewPermissions; - } + const type = getType(metadata); + return isAllowedToSeeType(metadata, type); } return false; }; diff --git a/frontend/src/pages/Notifications/reducer.js b/frontend/src/pages/Notifications/reducer.js index 32bb369a9d..b544569f5e 100644 --- a/frontend/src/pages/Notifications/reducer.js +++ b/frontend/src/pages/Notifications/reducer.js @@ -7,10 +7,9 @@ import { HIDE_HISTORY, FETCH_ALL_NOTIFICATIONS_SUCCESS, HIDE_SNACKBAR, - FETCH_NOTIFICATION_COUNTS_SUCCESS, + FETCH_NOTIFICATION_COUNT_SUCCESS, SET_NOTIFICATIONS_PER_PAGE, LIVE_UPDATE_NOTIFICATIONS_SUCCESS, - SET_NOTIFICATION_OFFSET, TIME_OUT_FLY_IN, ENABLE_LIVE_UPDATES, DISABLE_LIVE_UPDATES @@ -26,17 +25,21 @@ const defaultState = fromJS({ snackbarError: false, historyItems: [], unreadNotificationCount: 0, - notificationCount: 0, notificationsPerPage: 20, notificationOffset: 0, - isLiveUpdatesEnabled: true + isLiveUpdatesEnabled: true, + totalNotificationCount: 0, + currentNotificationPage: 1, + notificationPageSize: 20 }); export default function navbarReducer(state = defaultState, action) { switch (action.type) { case FETCH_ALL_NOTIFICATIONS_SUCCESS: return state.merge({ - notifications: fromJS(action.notifications) + notifications: fromJS(action.notifications), + currentNotificationPage: action.currentNotificationPage, + totalNotificationCount: action.totalNotificationCount }); case ENABLE_LIVE_UPDATES: { @@ -62,7 +65,7 @@ export default function navbarReducer(state = defaultState, action) { case TIME_OUT_FLY_IN: { return state.set("newNotifications", defaultState.get("newNotifications")); } - case FETCH_NOTIFICATION_COUNTS_SUCCESS: + case FETCH_NOTIFICATION_COUNT_SUCCESS: return state.merge({ unreadNotificationCount: action.unreadNotificationCount, notificationCount: action.notificationCount @@ -83,9 +86,7 @@ export default function navbarReducer(state = defaultState, action) { case HIDE_HISTORY: return state.set("showHistory", false); case SET_NOTIFICATIONS_PER_PAGE: - return state.set("notificationsPerPage", action.limit); - case SET_NOTIFICATION_OFFSET: - return state.set("notificationOffset", action.offset); + return state.set("notificationPageSize", action.notificationPageSize); case LOGOUT: return defaultState; default: diff --git a/frontend/src/pages/Workflows/WorkflowAssigneeContainer.js b/frontend/src/pages/Workflows/WorkflowAssigneeContainer.js index cd8e1c1cc4..df7cb860d4 100644 --- a/frontend/src/pages/Workflows/WorkflowAssigneeContainer.js +++ b/frontend/src/pages/Workflows/WorkflowAssigneeContainer.js @@ -28,10 +28,10 @@ class WorkflowAssigneeContainer extends Component { }; render() { - const { workflowItems, workflowitemId, users, title, disabled, workflowSortEnabled, status } = this.props; + const { workflowItems, workflowitemId, classes, users, title, disabled, workflowSortEnabled, status } = this.props; const assignee = this.getWorkflowAssignee(workflowItems, workflowitemId); return ( -
+
{ return "Test"; }; +const getNotificationState = state => { + return { + currentNotificationPage: state.getIn(["notifications", "currentNotificationPage"]), + numberOfNotificationPages: state.getIn(["notifications", "numberOfNotificationPages"]), + notificationPageSize: state.getIn(["notifications", "notificationPageSize"]) + }; +}; + function* callApi(func, ...args) { const token = yield select(getJwt); yield call(api.setAuthorizationHeader, token); @@ -520,17 +528,27 @@ export function* getEnvironmentSaga() { }); } -export function* fetchNotificationsSaga({ showLoading, offset, limit }) { - yield commonfetchNotifications(showLoading, offset, limit, FETCH_ALL_NOTIFICATIONS_SUCCESS); -} - -export function* commonfetchNotifications(showLoading, offset, limit, type) { +export function* fetchNotificationsSaga({ showLoading, notificationPage }) { yield execute(function*() { - // Get most recent items with negative offset - const { data } = yield callApi(api.fetchNotifications, 0 - offset - limit, limit); + const { data: notificationCountData } = yield callApi(api.fetchNotificationCounts); + const { notificationPageSize } = yield select(getNotificationState); + + const totalNotificationCount = notificationCountData.total; + + const numberOfNotificationPages = + notificationPageSize !== 0 ? Math.ceil(totalNotificationCount / notificationPageSize) : 1; + + const isLastNotificationPage = notificationPage + 1 === numberOfNotificationPages; + const offset = 0 - (notificationPage + 1) * notificationPageSize; + const itemsToFetch = isLastNotificationPage + ? totalNotificationCount - notificationPage * notificationPageSize + : notificationPageSize; + const { data } = yield callApi(api.fetchNotifications, offset, itemsToFetch); yield put({ - type, - notifications: data.notifications + type: FETCH_ALL_NOTIFICATIONS_SUCCESS, + notifications: data.notifications, + currentNotificationPage: notificationPage, + totalNotificationCount: totalNotificationCount }); }, showLoading); } @@ -539,14 +557,14 @@ export function* fetchNotificationCountsSaga({ showLoading }) { yield execute(function*() { const { data } = yield callApi(api.fetchNotificationCounts); yield put({ - type: FETCH_NOTIFICATION_COUNTS_SUCCESS, + type: FETCH_NOTIFICATION_COUNT_SUCCESS, unreadNotificationCount: data.unread, notificationCount: data.total }); }, showLoading); } -export function* markNotificationAsReadSaga({ notificationId, offset, limit }) { +export function* markNotificationAsReadSaga({ notificationId, notificationPage }) { yield execute(function*() { yield callApi(api.markNotificationAsRead, notificationId); yield put({ @@ -555,29 +573,27 @@ export function* markNotificationAsReadSaga({ notificationId, offset, limit }) { yield put({ type: FETCH_ALL_NOTIFICATIONS, showLoading: true, - offset, - limit + notificationPage }); yield put({ - type: FETCH_NOTIFICATION_COUNTS + type: FETCH_NOTIFICATION_COUNT }); }, true); } -export function* markMultipleNotificationsAsReadSaga({ notificationIds, offset, limit }) { +export function* markMultipleNotificationsAsReadSaga({ notificationIds, notificationPage }) { yield execute(function*() { yield callApi(api.markMultipleNotificationsAsRead, notificationIds); yield put({ - type: MARK_MULTIPLE_NOTIFICATION_AS_READ_SUCCESS + type: MARK_MULTIPLE_NOTIFICATIONS_AS_READ_SUCCESS }); yield put({ type: FETCH_ALL_NOTIFICATIONS, showLoading: true, - offset, - limit + notificationPage }); yield put({ - type: FETCH_NOTIFICATION_COUNTS + type: FETCH_NOTIFICATION_COUNT }); }, true); } @@ -1544,9 +1560,9 @@ export default function* rootSaga() { // Notifications yield takeEvery(FETCH_ALL_NOTIFICATIONS, fetchNotificationsSaga), - yield takeEvery(FETCH_NOTIFICATION_COUNTS, fetchNotificationCountsSaga), + yield takeEvery(FETCH_NOTIFICATION_COUNT, fetchNotificationCountsSaga), yield takeEvery(MARK_NOTIFICATION_AS_READ, markNotificationAsReadSaga), - yield takeEvery(MARK_MULTIPLE_NOTIFICATION_AS_READ, markMultipleNotificationsAsReadSaga), + yield takeEvery(MARK_MULTIPLE_NOTIFICATIONS_AS_READ, markMultipleNotificationsAsReadSaga), // Peers yield takeLatest(FETCH_ACTIVE_PEERS, fetchActivePeersSaga),