Skip to content

Commit

Permalink
Change and improve implementation for notifications
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mathiashoeld
mathiashoeld committed May 22, 2019
1 parent 94c829a commit 9752696
Show file tree
Hide file tree
Showing 13 changed files with 230 additions and 316 deletions.
1 change: 1 addition & 0 deletions e2e-test/cypress/integration/notification_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
);
});

Expand Down
2 changes: 1 addition & 1 deletion e2e-test/cypress/integration/workflowitem_history_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
135 changes: 9 additions & 126 deletions frontend/src/helper.js
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -136,112 +120,11 @@ export const statusIconMapping = {
open: <OpenIcon />
};

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]);
};
16 changes: 9 additions & 7 deletions frontend/src/pages/Notifications/FlyInNotifications.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ export default class FlyInNotification extends Component {
<CardHeader
avatar={<Avatar>{publisher ? publisher[0].toString().toUpperCase() : "?"}</Avatar>}
action={
<IconButton
disabled={!isAllowedToSee(notification)}
color="primary"
onClick={() => history.push(parseURI({ projectId, subprojectId }))}
>
<LaunchIcon />
</IconButton>
isAllowedToSee(notification) ? (
<IconButton
disabled={!isAllowedToSee(notification)}
color="primary"
onClick={() => history.push(parseURI({ projectId, subprojectId }))}
>
<LaunchIcon />
</IconButton>
) : null
}
title={getDisplayName(notification)}
/>
Expand Down
103 changes: 29 additions & 74 deletions frontend/src/pages/Notifications/NotificationList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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 (
<Card>
<CardHeader
title="Notifications"
action={
<Button
variant="outlined"
onClick={() =>
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}
</Button>
}
/>
<CardHeader title="Notifications" action={null} />
<div style={{ display: "flex", verticalAlign: "middle", padding: "11px 16px" }}>
<Button
variant="outlined"
onClick={() => markPageAsRead(markMultipleNotificationsAsRead, notifications, currentPage)}
className={classes.button + " mark-all-notifications-as-read"}
data-test="read-multiple-notifications"
disabled={!allNotificationsRead}
style={{ margin: "0px" }}
>
{strings.notification.read_all}
</Button>
</div>

<List component="div" data-test="notification-list">
<NotificationListItems
notifications={notifications}
history={history}
markNotificationAsRead={markNotificationAsRead}
markNotificationAsRead={notificationId => markNotificationAsRead(notificationId, currentPage)}
notificationsPerPage={notificationsPerPage}
notificationOffset={notificationOffset}
/>
Expand All @@ -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)}
/>
</div>
</Card>
Expand Down
Loading

0 comments on commit 9752696

Please sign in to comment.