From 783f81a783c67eb811aa71e286f66e5d84aebb40 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 --- CHANGELOG.md | 11 +- .../cypress/integration/notification_spec.js | 1 + .../integration/workflowitem_history_spec.js | 2 +- frontend/src/helper.js | 135 ++---------------- frontend/src/index.js | 6 +- .../pages/Notifications/FlyInNotifications.js | 21 +-- .../pages/Notifications/NotificationList.js | 102 ++++--------- .../Notifications/NotificationListItems.js | 39 +++-- .../pages/Notifications/NotificationPage.js | 4 +- .../NotificationPageContainer.js | 21 ++- frontend/src/pages/Notifications/actions.js | 38 ++--- frontend/src/pages/Notifications/helper.js | 89 ++++++++---- frontend/src/pages/Notifications/reducer.js | 19 +-- .../Workflows/WorkflowAssigneeContainer.js | 4 +- frontend/src/sagas.js | 66 +++++---- 15 files changed, 227 insertions(+), 331 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89dc0e7cb..bf65be249 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] - +### Added +- Different background color for unread notifications [#300](https://github.com/openkfw/TruBudget/issues/300) - +### Changed +- Notification displays name of parent project and subproject [#298](https://github.com/openkfw/TruBudget/issues/298) +- Move 'Read All' button to the left side [#301](https://github.com/openkfw/TruBudget/issues/301) +- Don't display view button if user is not allowed to see project/subproject [#302](https://github.com/openkfw/TruBudget/issues/302) @@ -17,6 +21,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Empty history displayed after API call is finished [#294](https://github.com/openkfw/TruBudget/issues/294) +- Last page of notifications displays correct number of items [#288](https://github.com/openkfw/TruBudget/issues/288) +- Prevent assignee selection from overflowing [#299](https://github.com/openkfw/TruBudget/issues/299) +- Display correct name in notifications [#292](https://github.com/openkfw/TruBudget/issues/292) ## [1.0.1] - 2019-05-21 diff --git a/e2e-test/cypress/integration/notification_spec.js b/e2e-test/cypress/integration/notification_spec.js index 540d1d1b5..6b94bb049 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 be8bca7c6..60211ad01 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 7bac4f67a..3217a7fe0 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/index.js b/frontend/src/index.js index aabb3a0f7..180000e44 100644 --- a/frontend/src/index.js +++ b/frontend/src/index.js @@ -10,6 +10,7 @@ import relativeTime from "dayjs/plugin/relativeTime"; import red from "@material-ui/core/colors/deepOrange"; import blue from "@material-ui/core/colors/indigo"; +import grey from "@material-ui/core/colors/grey"; import Main from "./pages/Main/Main"; import LoginPageContainer from "./pages/Login/LoginPageContainer"; @@ -31,7 +32,10 @@ const store = configureStore(initialState, history); const muiTheme = createMuiTheme({ palette: { primary: blue, - secondary: red + secondary: red, + grey: { + main: grey[100] + } }, typography: { useNextVariants: true diff --git a/frontend/src/pages/Notifications/FlyInNotifications.js b/frontend/src/pages/Notifications/FlyInNotifications.js index a1c430bd8..6501ffaa3 100644 --- a/frontend/src/pages/Notifications/FlyInNotifications.js +++ b/frontend/src/pages/Notifications/FlyInNotifications.js @@ -9,7 +9,7 @@ import IconButton from "@material-ui/core/IconButton"; import LaunchIcon from "@material-ui/icons/ZoomIn"; import Typography from "@material-ui/core/Typography"; -import { intentMapping, getDisplayName, parseURI, isAllowedToSee } from "./helper"; +import { intentMapping, parseURI, isAllowedToSee, getParentData } from "./helper"; const styles = { notification: { @@ -32,6 +32,7 @@ export default class FlyInNotification extends Component { const subprojectId = metadata.subproject ? metadata.subproject.id : undefined; const { publisher } = businessEvent; const message = intentMapping(notification); + const { projectDisplayName, subprojectDisplayName } = getParentData(notification); return ( {publisher ? publisher[0].toString().toUpperCase() : "?"}} action={ - history.push(parseURI({ projectId, subprojectId }))} - > - - + isAllowedToSee(notification) ? ( + history.push(parseURI({ projectId, subprojectId }))} + > + + + ) : null } - title={getDisplayName(notification)} + title={projectDisplayName + " " + subprojectDisplayName} /> {message} diff --git a/frontend/src/pages/Notifications/NotificationList.js b/frontend/src/pages/Notifications/NotificationList.js index 3434a784f..b31f7509f 100644 --- a/frontend/src/pages/Notifications/NotificationList.js +++ b/frontend/src/pages/Notifications/NotificationList.js @@ -22,54 +22,21 @@ 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); + //Fetch first page again + fetchNotifications(0); }; const NotificationsList = props => { @@ -82,37 +49,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 +87,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 02254c60e..ac8d29a77 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"; @@ -9,9 +10,10 @@ import Read from "@material-ui/icons/MailOutline"; import LaunchIcon from "@material-ui/icons/ZoomIn"; import dayjs from "dayjs"; import React from "react"; +import classNames from "classnames"; -import { intentMapping, parseURI, isAllowedToSee } from "./helper"; - +import { intentMapping, parseURI, getParentData, isAllowedToSee } from "./helper"; +import strings from "../../localizeStrings"; const styles = theme => ({ row: { display: "flex", @@ -38,6 +40,9 @@ const styles = theme => ({ unread: { flex: 1, opacity: 1 + }, + unreadMessage: { + backgroundColor: theme.palette.grey.main } }); @@ -59,12 +64,13 @@ const NotificationListItems = ({ subprojectId: metadata.subproject ? metadata.subproject.id : undefined }); const testLabel = `notification-${isRead ? "read" : "unread"}`; + const { projectDisplayName, subprojectDisplayName } = getParentData(notification); return (
{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 508da4b66..280a818d6 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 819a13876..188c8e6d7 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 9bd375513..74e7e625f 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 2a40de965..6318758c6 100644 --- a/frontend/src/pages/Notifications/helper.js +++ b/frontend/src/pages/Notifications/helper.js @@ -1,31 +1,76 @@ import strings from "../../localizeStrings"; import { formatString } from "../../helper"; -// get hierarchic deepest displayName -export function getDisplayName(notification) { +const ACCESSMAP = { + PROJECT: "project", + SUBPROJECT: "subproject", + 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 !== undefined && metadata.project.hasViewPermissions === true) { - return metadata.project.displayName; + if (metadata.project) { + const { + displayName: projectName = "", + hasViewPermissions: hasProjectViewPermissions = false + } = getDataFromNotification(metadata, ACCESSMAP.PROJECT); + projectDisplayName = hasProjectViewPermissions ? projectName : redacted; + if (metadata.subproject) { + const { + displayName: subprojectName = "", + hasViewPermissions: hasSubprojectViewPermissions = false + } = getDataFromNotification(metadata, ACCESSMAP.SUBPROJECT); + subprojectDisplayName = hasSubprojectViewPermissions ? subprojectName : redacted; + } } } - return ""; + + return { + projectDisplayName, + subprojectDisplayName + }; } +export const isAllowedToSee = notification => { + const metadata = notification.metadata; + if (metadata !== undefined) { + const { hasViewPermissions = false } = getDataFromNotification(metadata); + return hasViewPermissions; + } + return false; +}; + 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 { displayName = "", hasViewPermissions = false } = getDataFromNotification(notificationMetaData); + + return hasViewPermissions + ? formatString(translation, displayName) + : formatString(translation, "") + " " + strings.notification.no_permissions; }; +const getDataFromNotification = (metadata, type) => + type + ? metadata[type] + : metadata[ACCESSMAP.WORKFLOWITEM] || metadata[ACCESSMAP.SUBPROJECT] || metadata[ACCESSMAP.PROJECT]; + export const parseURI = ({ projectId, subprojectId }) => { if (projectId && !subprojectId) { return `/projects/${projectId}`; @@ -35,19 +80,3 @@ export const parseURI = ({ projectId, subprojectId }) => { throw new Error("not implemented"); } }; - -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; - } - } - return false; -}; diff --git a/frontend/src/pages/Notifications/reducer.js b/frontend/src/pages/Notifications/reducer.js index 32bb369a9..b544569f5 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 cd8e1c1cc..df7cb860d 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 { + 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); @@ -549,17 +557,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); } @@ -568,14 +586,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({ @@ -584,29 +602,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); } @@ -1642,9 +1658,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),