Report notifications not useful

Summary:
* adds a button to hide notifications by category
* adds a quick action to report notifications as not helpful
* fixes type of network plugin's notifications to be string instead of number

Reviewed By: jknoxville

Differential Revision: D12999883

fbshipit-source-id: 32be5bde5931cca4a27ab1ad34418300196452cc
This commit is contained in:
Daniel Büchele
2018-11-12 01:47:53 -08:00
committed by Facebook Github Bot
parent bd03f891d0
commit bf7c35387c
2 changed files with 164 additions and 44 deletions

View File

@@ -7,12 +7,13 @@
import type {SearchableProps, FlipperBasePlugin, Device} from 'flipper'; import type {SearchableProps, FlipperBasePlugin, Device} from 'flipper';
import type {PluginNotification} from './reducers/notifications'; import type {PluginNotification} from './reducers/notifications';
import {selectPlugin} from './reducers/connections'; import type Logger from './fb-stubs/Logger';
import { import {
FlipperDevicePlugin, FlipperDevicePlugin,
Searchable, Searchable,
Button, Button,
ButtonGroup,
FlexBox, FlexBox,
FlexColumn, FlexColumn,
FlexRow, FlexRow,
@@ -29,7 +30,9 @@ import PropTypes from 'prop-types';
import { import {
clearAllNotifications, clearAllNotifications,
updatePluginBlacklist, updatePluginBlacklist,
updateCategoryBlacklist,
} from './reducers/notifications'; } from './reducers/notifications';
import {selectPlugin} from './reducers/connections';
import {createPaste, textContent} from './utils/index'; import {createPaste, textContent} from './utils/index';
export default class Notifications extends FlipperDevicePlugin<{}> { export default class Notifications extends FlipperDevicePlugin<{}> {
@@ -57,19 +60,30 @@ export default class Notifications extends FlipperDevicePlugin<{}> {
}; };
render() { render() {
const {
blacklistedPlugins,
blacklistedCategories,
} = this.context.store.getState().notifications;
return ( return (
<ConnectedNotificationsTable <ConnectedNotificationsTable
onClear={this.onClear} onClear={this.onClear}
selectedID={this.props.deepLinkPayload} selectedID={this.props.deepLinkPayload}
onSelectPlugin={this.props.selectPlugin} onSelectPlugin={this.props.selectPlugin}
defaultFilters={this.context.store logger={this.props.logger}
.getState() defaultFilters={[
.notifications.blacklistedPlugins.map(value => ({ ...blacklistedPlugins.map(value => ({
value, value,
invertible: false, invertible: false,
type: 'exclude', type: 'exclude',
key: 'plugin', key: 'plugin',
}))} })),
...blacklistedCategories.map(value => ({
value,
invertible: false,
type: 'exclude',
key: 'category',
})),
]}
actions={ actions={
<Fragment> <Fragment>
<Button onClick={this.onClear}>Clear</Button> <Button onClick={this.onClear}>Clear</Button>
@@ -85,14 +99,17 @@ type Props = {|
activeNotifications: Array<PluginNotification>, activeNotifications: Array<PluginNotification>,
invalidatedNotifications: Array<PluginNotification>, invalidatedNotifications: Array<PluginNotification>,
blacklistedPlugins: Array<string>, blacklistedPlugins: Array<string>,
blacklistedCategories: Array<string>,
onClear: () => void, onClear: () => void,
updatePluginBlacklist: (blacklist: Array<string>) => mixed, updatePluginBlacklist: (blacklist: Array<string>) => mixed,
updateCategoryBlacklist: (blacklist: Array<string>) => mixed,
selectPlugin: ({ selectPlugin: ({
selectedPlugin: ?string, selectedPlugin: ?string,
selectedApp: ?string, selectedApp: ?string,
deepLinkPayload?: ?string, deepLinkPayload?: ?string,
}) => mixed, }) => mixed,
selectedID: ?string, selectedID: ?string,
logger: Logger,
|}; |};
type State = {| type State = {|
@@ -131,12 +148,6 @@ const NoContent = styled(FlexColumn)({
}); });
class NotificationsTable extends Component<Props, State> { class NotificationsTable extends Component<Props, State> {
static getDerivedStateFromProps(props: Props): State {
return {
selectedNotification: props.selectedID,
};
}
contextMenuItems = [{label: 'Clear all', click: this.props.onClear}]; contextMenuItems = [{label: 'Clear all', click: this.props.onClear}];
state: State = { state: State = {
selectedNotification: this.props.selectedID, selectedNotification: this.props.selectedID,
@@ -149,10 +160,27 @@ class NotificationsTable extends Component<Props, State> {
.filter(f => f.type === 'exclude' && f.key.toLowerCase() === 'plugin') .filter(f => f.type === 'exclude' && f.key.toLowerCase() === 'plugin')
.map(f => String(f.value)), .map(f => String(f.value)),
); );
this.props.updateCategoryBlacklist(
this.props.filters
.filter(
f => f.type === 'exclude' && f.key.toLowerCase() === 'category',
)
.map(f => String(f.value)),
);
}
if (
this.props.selectedID &&
prevProps.selectedID !== this.props.selectedID
) {
this.setState({
selectedNotification: this.props.selectedID,
});
} }
} }
onHide = (pluginId: string) => { onHidePlugin = (pluginId: string) => {
// add filter to searchbar // add filter to searchbar
this.props.addFilter({ this.props.addFilter({
value: pluginId, value: pluginId,
@@ -165,17 +193,43 @@ class NotificationsTable extends Component<Props, State> {
); );
}; };
onHideCategory = (category: string) => {
// add filter to searchbar
this.props.addFilter({
value: category,
type: 'exclude',
key: 'category',
invertible: false,
});
this.props.updatePluginBlacklist(
this.props.blacklistedCategories.concat(category),
);
};
getFilter = (): ((n: PluginNotification) => boolean) => ( getFilter = (): ((n: PluginNotification) => boolean) => (
n: PluginNotification, n: PluginNotification,
) => { ) => {
const searchTerm = this.props.searchTerm.toLowerCase(); const searchTerm = this.props.searchTerm.toLowerCase();
const blacklist = new Set(
// filter plugins
const blacklistedPlugins = new Set(
this.props.blacklistedPlugins.map(p => p.toLowerCase()), this.props.blacklistedPlugins.map(p => p.toLowerCase()),
); );
if (blacklist.has(n.pluginId.toLowerCase())) { if (blacklistedPlugins.has(n.pluginId.toLowerCase())) {
return false; return false;
} }
// filter categories
const {category} = n.notification;
if (category) {
const blacklistedCategories = new Set(
this.props.blacklistedCategories.map(p => p.toLowerCase()),
);
if (blacklistedCategories.has(category.toLowerCase())) {
return false;
}
}
if (searchTerm.length === 0) { if (searchTerm.length === 0) {
return true; return true;
} else if (n.notification.title.toLowerCase().indexOf(searchTerm) > -1) { } else if (n.notification.title.toLowerCase().indexOf(searchTerm) > -1) {
@@ -192,19 +246,27 @@ class NotificationsTable extends Component<Props, State> {
render() { render() {
const activeNotifications = this.props.activeNotifications const activeNotifications = this.props.activeNotifications
.filter(this.getFilter()) .filter(this.getFilter())
.map((n: PluginNotification) => ( .map((n: PluginNotification) => {
const {category} = n.notification;
return (
<NotificationItem <NotificationItem
key={n.notification.id} key={n.notification.id}
{...n} {...n}
isSelected={this.state.selectedNotification === n.notification.id} isSelected={this.state.selectedNotification === n.notification.id}
onClick={() => onHighlight={() =>
this.setState({selectedNotification: n.notification.id}) this.setState({selectedNotification: n.notification.id})
} }
onClear={this.props.onClear} onClear={this.props.onClear}
onHide={() => this.onHide(n.pluginId)} onHidePlugin={() => this.onHidePlugin(n.pluginId)}
onHideCategory={
category ? () => this.onHideCategory(category) : undefined
}
selectPlugin={this.props.selectPlugin} selectPlugin={this.props.selectPlugin}
logger={this.props.logger}
/> />
)) );
})
.reverse(); .reverse();
const invalidatedNotifications = this.props.invalidatedNotifications const invalidatedNotifications = this.props.invalidatedNotifications
@@ -255,14 +317,17 @@ const ConnectedNotificationsTable = connect(
activeNotifications, activeNotifications,
invalidatedNotifications, invalidatedNotifications,
blacklistedPlugins, blacklistedPlugins,
blacklistedCategories,
}, },
}) => ({ }) => ({
activeNotifications, activeNotifications,
invalidatedNotifications, invalidatedNotifications,
blacklistedPlugins, blacklistedPlugins,
blacklistedCategories,
}), }),
{ {
updatePluginBlacklist, updatePluginBlacklist,
updateCategoryBlacklist,
selectPlugin, selectPlugin,
}, },
)(Searchable(NotificationsTable)); )(Searchable(NotificationsTable));
@@ -348,7 +413,7 @@ const NotificationButton = styled('div')({
borderRadius: 4, borderRadius: 4,
textAlign: 'center', textAlign: 'center',
padding: 4, padding: 4,
width: 55, width: 80,
marginBottom: 4, marginBottom: 4,
opacity: 0, opacity: 0,
transition: '0.15s opacity', transition: '0.15s opacity',
@@ -365,8 +430,9 @@ const NotificationButton = styled('div')({
type ItemProps = { type ItemProps = {
...PluginNotification, ...PluginNotification,
onClick?: () => mixed, onHighlight?: () => mixed,
onHide?: () => mixed, onHidePlugin?: () => mixed,
onHideCategory?: () => mixed,
isSelected?: boolean, isSelected?: boolean,
inactive?: boolean, inactive?: boolean,
selectPlugin?: ({ selectPlugin?: ({
@@ -374,18 +440,29 @@ type ItemProps = {
selectedApp: ?string, selectedApp: ?string,
deepLinkPayload?: ?string, deepLinkPayload?: ?string,
}) => mixed, }) => mixed,
logger?: Logger,
}; };
class NotificationItem extends Component<ItemProps> { type ItemState = {|
reportedNotHelpful: boolean,
|};
class NotificationItem extends Component<ItemProps, ItemState> {
constructor(props: ItemProps) { constructor(props: ItemProps) {
super(props); super(props);
const plugin = plugins.find(p => p.id === props.pluginId); const plugin = plugins.find(p => p.id === props.pluginId);
const items = []; const items = [];
if (props.onHide && plugin) { if (props.onHidePlugin && plugin) {
items.push({ items.push({
label: `Hide ${plugin.title} plugin`, label: `Hide ${plugin.title} plugin`,
click: this.props.onHide, click: this.props.onHidePlugin,
});
}
if (props.onHideCategory) {
items.push({
label: 'Hide Similar',
click: this.props.onHideCategory,
}); });
} }
items.push( items.push(
@@ -397,6 +474,7 @@ class NotificationItem extends Component<ItemProps> {
this.plugin = plugin; this.plugin = plugin;
} }
state = {reportedNotHelpful: false};
plugin: ?Class<FlipperBasePlugin<>>; plugin: ?Class<FlipperBasePlugin<>>;
contextMenuItems; contextMenuItems;
deepLinkButton = React.createRef(); deepLinkButton = React.createRef();
@@ -429,8 +507,37 @@ class NotificationItem extends Component<ItemProps> {
} }
}; };
reportNotUseful = (e: UIEvent) => {
e.preventDefault();
e.stopPropagation();
if (this.props.logger) {
this.props.logger.track(
'usage',
'notification-not-useful',
this.props.notification,
);
}
this.setState({reportedNotHelpful: true});
};
onHide = (e: UIEvent) => {
e.preventDefault();
e.stopPropagation();
if (this.props.onHideCategory) {
this.props.onHideCategory();
} else if (this.props.onHidePlugin) {
this.props.onHidePlugin();
}
};
render() { render() {
const {notification, isSelected, inactive, onHide} = this.props; const {
notification,
isSelected,
inactive,
onHidePlugin,
onHideCategory,
} = this.props;
const {action} = notification; const {action} = notification;
return ( return (
@@ -438,7 +545,7 @@ class NotificationItem extends Component<ItemProps> {
data-role="notification" data-role="notification"
component={NotificationBox} component={NotificationBox}
severity={notification.severity} severity={notification.severity}
onClick={this.props.onClick} onClick={this.props.onHighlight}
isSelected={isSelected} isSelected={isSelected}
inactive={inactive} inactive={inactive}
items={this.contextMenuItems}> items={this.contextMenuItems}>
@@ -449,7 +556,7 @@ class NotificationItem extends Component<ItemProps> {
{!inactive && {!inactive &&
isSelected && isSelected &&
this.plugin && this.plugin &&
(action || onHide) && ( (action || onHidePlugin || onHideCategory) && (
<Actions> <Actions>
<FlexRow> <FlexRow>
{action && ( {action && (
@@ -457,9 +564,16 @@ class NotificationItem extends Component<ItemProps> {
Open in {this.plugin.title} Open in {this.plugin.title}
</Button> </Button>
)} )}
{onHide && ( <ButtonGroup>
<Button onClick={onHide}>Hide {this.plugin.title}</Button> {onHideCategory && (
<Button onClick={onHideCategory}>Hide similar</Button>
)} )}
{onHidePlugin && (
<Button onClick={onHidePlugin}>
Hide {this.plugin.title}
</Button>
)}
</ButtonGroup>
</FlexRow> </FlexRow>
<span> <span>
{notification.timestamp {notification.timestamp
@@ -478,10 +592,14 @@ class NotificationItem extends Component<ItemProps> {
Open Open
</NotificationButton> </NotificationButton>
)} )}
{onHide && ( {this.state.reportedNotHelpful ? (
<NotificationButton compact onClick={onHide}> <NotificationButton compact onClick={this.onHide}>
Hide Hide
</NotificationButton> </NotificationButton>
) : (
<NotificationButton compact onClick={this.reportNotUseful}>
Not helpful
</NotificationButton>
)} )}
</FlexColumn> </FlexColumn>
)} )}

View File

@@ -143,9 +143,11 @@ export default class extends FlipperPlugin<State, *, PersistedState> {
persistedState: PersistedState, persistedState: PersistedState,
): Array<Notification> => { ): Array<Notification> => {
const responses = persistedState ? persistedState.responses || [] : []; const responses = persistedState ? persistedState.responses || [] : [];
return (
// $FlowFixMe Object.values returns Array<mixed>, but we know it is Array<Response> // $FlowFixMe Object.values returns Array<mixed>, but we know it is Array<Response>
(Object.values(responses): Array<Response>) const r: Array<Response> = Object.values(responses);
return (
r
// Show error messages for all status codes indicating a client or server error // Show error messages for all status codes indicating a client or server error
.filter((response: Response) => response.status >= 400) .filter((response: Response) => response.status >= 400)
.map((response: Response) => ({ .map((response: Response) => ({
@@ -155,7 +157,7 @@ export default class extends FlipperPlugin<State, *, PersistedState> {
'(URL missing)'}" failed. ${response.reason}`, '(URL missing)'}" failed. ${response.reason}`,
severity: 'error', severity: 'error',
timestamp: response.timestamp, timestamp: response.timestamp,
category: response.status, category: `HTTP${response.status}`,
action: response.id, action: response.id,
})) }))
); );