-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement change alerts on Manage Queues and Queue Manager pages (#157, #160, #163) #258
Draft
ssciolla
wants to merge
22
commits into
tl-its-umich-edu:master
Choose a base branch
from
ssciolla:issues-157-160-change-event-alerts
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
25ec438
Add first draft of change log; implement in manage.tsx
ssciolla 6ea8a3f
Tweak syntax, import ordering
ssciolla ca12ebd
Implement change log for meetings in queueManager
ssciolla 4dfd86d
Replace EntityType with type checkers
ssciolla ee5966a
Use more specific var names with useEntityChanges in manage.tsx
ssciolla 41e7c6a
Modify identifier for meetings; implement detectChanges
ssciolla 2b0fc59
Switch to using ChangeEvents[] instead of ChangeEventMap
ssciolla bcf6702
Refactor deleteChangeEvent; implement TimedChangeAlert
ssciolla 1e3ce87
Add aria-live property to Alerts
ssciolla 03b94ae
Change location of meeting ChangeLog
ssciolla a5f546d
Use ChangeLog inside of QueueManager instead of using props.children
ssciolla 2a4b612
Compare values in detectChanges after user object handling; only anno…
ssciolla d16e771
Reassign falsy values to "None"
ssciolla 6bbbb7c
Add custom check for whether meeting has changed to in progress
ssciolla c698316
Tweak return value of detectChanges
ssciolla 54f18fc
Fix a few misc. issues
ssciolla 1bd1d20
Use ComparableEntity with generic instead of Base
ssciolla 2beee7b
Remove mention of EventMap
ssciolla affb03f
Refactor to allow multiple changeMessages, multiple changes; tidy up
ssciolla c10b9f1
Add missing semicolons and newline
ssciolla ba5614e
Simplify return types; rename a var
ssciolla f64b685
Remove stray indentation
ssciolla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import xorWith from "lodash.xorwith"; | ||
import isEqual from "lodash.isequal"; | ||
|
||
import { isMeeting, isQueueBase, isUser, Meeting, MeetingStatus, QueueBase } from "./models"; | ||
|
||
export type ComparableEntity = QueueBase | Meeting; | ||
|
||
export interface ChangeEvent { | ||
eventID: number; | ||
text: string; | ||
} | ||
|
||
const queueBasePropsToWatch: (keyof QueueBase)[] = ['status', 'name']; | ||
const meetingPropsToWatch: (keyof Meeting)[] = ['backend_type', 'assignee']; | ||
|
||
|
||
// Value Transformations | ||
|
||
type ValueTransform = (value: any) => any; | ||
|
||
const transformUserToUsername: ValueTransform = (value) => { | ||
return isUser(value) ? value.username : value; | ||
}; | ||
|
||
const transformFalsyToNone: ValueTransform = (value) => { | ||
return value === null || value === undefined || value === '' ? 'None' : value; | ||
} | ||
|
||
function transformValue (value: any, transforms: ValueTransform[]): any { | ||
let newValue = value; | ||
for (const transform of transforms) { | ||
newValue = transform(newValue); | ||
} | ||
return newValue; | ||
} | ||
|
||
const standardTransforms = [transformUserToUsername, transformFalsyToNone]; | ||
|
||
|
||
// Property Transformations | ||
|
||
interface HumanReadableMap { [key: string]: string; } | ||
|
||
const humanReadablePropertyMap: HumanReadableMap = { | ||
'backend_type': 'meeting type', | ||
'assignee': 'host' | ||
}; | ||
|
||
const transformProperty = (value: string, propertyMap: HumanReadableMap) => { | ||
return (value in propertyMap) ? propertyMap[value] : value; | ||
} | ||
|
||
|
||
// Core functions | ||
|
||
function detectChanges<T extends ComparableEntity> ( | ||
versOne: T, versTwo: T, propsToWatch: (keyof T)[], transforms: ValueTransform[]): string[] | ||
{ | ||
let changedPropMessages = []; | ||
for (const property of propsToWatch) { | ||
let valueOne = versOne[property] as T[keyof T] | string; | ||
let valueTwo = versTwo[property] as T[keyof T] | string; | ||
valueOne = transformValue(valueOne, transforms); | ||
valueTwo = transformValue(valueTwo, transforms); | ||
if (valueOne !== valueTwo) { | ||
const propName = transformProperty(property as string, humanReadablePropertyMap); | ||
changedPropMessages.push(`The ${propName} changed from "${valueOne}" to "${valueTwo}".`); | ||
} | ||
} | ||
return changedPropMessages; | ||
} | ||
|
||
|
||
// Any new types added to ComparableEntity need to be supported in this function. | ||
|
||
function describeEntity (entity: ComparableEntity): string[] { | ||
let entityType; | ||
let permIdent; | ||
if (isMeeting(entity)) { | ||
entityType = 'meeting'; | ||
permIdent = `attendee ${entity.attendees[0].username}`; | ||
} else { | ||
// Don't need to check if it's a QueueBase because it's the only other option | ||
entityType = 'queue'; | ||
permIdent = `ID number ${entity.id}`; | ||
} | ||
return [entityType, permIdent]; | ||
} | ||
|
||
|
||
// https://lodash.com/docs/4.17.15#xorWith | ||
|
||
export function compareEntities<T extends ComparableEntity> (oldOnes: T[], newOnes: T[]): string[] | ||
{ | ||
const symDiff = xorWith(oldOnes, newOnes, isEqual); | ||
if (symDiff.length === 0) return []; | ||
|
||
const oldIDs = oldOnes.map((value) => value.id); | ||
const newIDs = newOnes.map((value) => value.id); | ||
|
||
let changeMessages: string[] = []; | ||
let processedChangedObjectIDs: number[] = []; | ||
for (const entity of symDiff) { | ||
if (processedChangedObjectIDs.includes(entity.id)) continue; | ||
const [entityType, permIdent] = describeEntity(entity); | ||
if (oldIDs.includes(entity.id) && !newIDs.includes(entity.id)) { | ||
changeMessages.push(`The ${entityType} with ${permIdent} was deleted.`); | ||
} else if (!oldIDs.includes(entity.id) && newIDs.includes(entity.id)) { | ||
changeMessages.push(`A new ${entityType} with ${permIdent} was added.`); | ||
} else { | ||
// Assuming based on context that symDiff.length === 2 | ||
const [firstEntity, secondEntity] = symDiff.filter(value => value.id === entity.id); | ||
let changesDetected: string[] = []; | ||
if (isMeeting(firstEntity) && isMeeting(secondEntity)) { | ||
const changes = detectChanges<Meeting>(firstEntity, secondEntity, meetingPropsToWatch, standardTransforms); | ||
if (changes.length > 0) changesDetected.push(...changes); | ||
// Custom check for Meeting.status, since only some status changes are relevant here. | ||
if (firstEntity.status !== secondEntity.status && secondEntity.status === MeetingStatus.STARTED) { | ||
changesDetected.push('The meeting is now in progress.'); | ||
} | ||
} else if (isQueueBase(firstEntity) && isQueueBase(secondEntity)) { | ||
const changes = detectChanges<QueueBase>(firstEntity, secondEntity, queueBasePropsToWatch, standardTransforms); | ||
if (changes.length > 0) changesDetected.push(...changes); | ||
} | ||
if (changesDetected.length > 0) { | ||
changeMessages.push(`The ${entityType} with ${permIdent} was changed. ` + changesDetected.join(' ')); | ||
} | ||
processedChangedObjectIDs.push(entity.id) | ||
} | ||
} | ||
return changeMessages; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import * as React from "react"; | ||
import { useEffect } from "react"; | ||
import { Alert } from "react-bootstrap"; | ||
|
||
import { ChangeEvent } from "../changes"; | ||
|
||
|
||
// https://stackoverflow.com/a/56777520 | ||
|
||
interface TimedChangeAlertProps { | ||
changeEvent: ChangeEvent, | ||
deleteChangeEvent: (id: number) => void; | ||
} | ||
|
||
function TimedChangeAlert (props: TimedChangeAlertProps) { | ||
const deleteEvent = () => props.deleteChangeEvent(props.changeEvent.eventID); | ||
|
||
useEffect(() => { | ||
const timeoutID = setTimeout(deleteEvent, 7000); | ||
return () => clearTimeout(timeoutID); | ||
}, []); | ||
|
||
return ( | ||
<Alert variant='info' aria-live='polite' dismissible={true} onClose={deleteEvent}> | ||
{props.changeEvent.text} | ||
</Alert> | ||
); | ||
} | ||
|
||
interface ChangeLogProps { | ||
changeEvents: ChangeEvent[]; | ||
deleteChangeEvent: (id: number) => void; | ||
} | ||
|
||
export function ChangeLog (props: ChangeLogProps) { | ||
const changeAlerts = props.changeEvents.map( | ||
(e) => <TimedChangeAlert key={e.eventID} changeEvent={e} deleteChangeEvent={props.deleteChangeEvent}/> | ||
); | ||
return <div id='change-log'>{changeAlerts}</div>; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { useState } from "react"; | ||
|
||
import { ChangeEvent, compareEntities, ComparableEntity } from "../changes"; | ||
|
||
|
||
export function useEntityChanges<T extends ComparableEntity>(): | ||
[ChangeEvent[], (oldEntities: readonly T[], newEntities: readonly T[]) => void, (key: number) => void] | ||
{ | ||
const [changeEvents, setChangeEvents] = useState([] as ChangeEvent[]); | ||
const [nextID, setNextID] = useState(0); | ||
|
||
const compareAndSetChangeEvents = (oldEntities: readonly T[], newEntities: readonly T[]): void => { | ||
const changeMessages = compareEntities<T>(oldEntities.slice(), newEntities.slice()); | ||
if (changeMessages.length > 0) { | ||
let eventID = nextID; | ||
const newChangeEvents = changeMessages.map( | ||
(m) => { | ||
const newEvent = { eventID: eventID, text: m } as ChangeEvent; | ||
eventID++; | ||
return newEvent; | ||
} | ||
) | ||
setChangeEvents([...changeEvents].concat(newChangeEvents)); | ||
setNextID(eventID); | ||
} | ||
}; | ||
|
||
// https://reactjs.org/docs/hooks-reference.html#functional-updates | ||
const deleteChangeEvent = (id: number) => { | ||
setChangeEvents((prevChangeEvents) => prevChangeEvents.filter((e) => id !== e.eventID)); | ||
}; | ||
ssciolla marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return [changeEvents, compareAndSetChangeEvents, deleteChangeEvent]; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { useEffect, useRef } from "react"; | ||
|
||
|
||
// https://reactjs.org/docs/hooks-faq.html#how-to-get-the-previous-props-or-state | ||
|
||
export function usePreviousState(value: any): any { | ||
const ref = useRef(); | ||
useEffect(() => { | ||
ref.current = value; | ||
}); | ||
return ref.current; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really a technical comment but a stray observation - since these are positioned toward the top, the rest of the page shifts downward a bit when they appear, which might cause misclicks. Not sure what the best way to handle that would be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah, tricky, it seemed like a simpler implementation to just always report them at the top, but they could be hidden from view or result in some page shifting if there are a lot of meetings in play.