-
Notifications
You must be signed in to change notification settings - Fork 74
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
THREESCALE-10245: access tokens expiration UI #3943
base: master
Are you sure you want to change the base?
Changes from 4 commits
dc05a73
4f4690d
995530a
4036f22
d2f7411
6d88683
14b083d
fda4ff2
667e146
0921951
588b314
587a618
39b8c24
88badaf
bfd0398
62d048d
7f45144
05bc601
6dbcdfe
63f4496
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { ExpirationDatePickerWrapper } from 'AccessTokens/components/ExpirationDatePicker' | ||
|
||
import type { Props } from 'AccessTokens/components/ExpirationDatePicker' | ||
|
||
import { safeFromJsonString } from '../src/utilities/json-utils' | ||
|
||
document.addEventListener('DOMContentLoaded', () => { | ||
const containerId = 'expiration-date-picker-container' | ||
const container = document.getElementById(containerId) | ||
|
||
if (!container) { | ||
throw new Error(`Missing container with id "${containerId}"`) | ||
} | ||
|
||
const props = safeFromJsonString<Props>(container.dataset.props) | ||
|
||
if (!props) { | ||
throw new Error('Missing props') | ||
} | ||
|
||
ExpirationDatePickerWrapper(props, containerId) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
.pf-c-calendar-month, .pf-c-form-control-expiration { | ||
width: 50%; | ||
} | ||
|
||
.pf-c-form-control-expiration { | ||
margin-right: 1em; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
import { useState, useMemo } from 'react' | ||
import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption } from '@patternfly/react-core' | ||
|
||
import { createReactWrapper } from 'utilities/createReactWrapper' | ||
import type { IRecord } from 'utilities/patternfly-utils' | ||
|
||
import type { FunctionComponent, FormEvent } from 'react' | ||
|
||
import './ExpirationDatePicker.scss' | ||
|
||
interface ExpirationItem extends IRecord { | ||
id: number; | ||
name: string; | ||
period: number; // In seconds | ||
} | ||
|
||
const collection: ExpirationItem[] = [ | ||
{ id: 1, name: '7 days', period: 7 }, | ||
{ id: 2, name: '30 days', period: 30 }, | ||
{ id: 3, name: '60 days', period: 60 }, | ||
{ id: 4, name: '90 days', period: 90 }, | ||
{ id: 5, name: 'Custom...', period: 0 }, | ||
{ id: 6, name: 'No expiration', period: 0 } | ||
] | ||
|
||
const dayMs = 60 * 60 * 24 * 1000 | ||
const msExp = /\.\d{3}Z$/ | ||
|
||
interface Props { | ||
id: string; | ||
label: string | null; | ||
} | ||
|
||
const ExpirationDatePicker: FunctionComponent<Props> = ({ id, label }) => { | ||
const [selectedItem, setSelectedItem] = useState(collection[0]) | ||
const [pickedDate, setPickedDate] = useState(new Date()) | ||
const fieldName = `human_${id}` | ||
const fieldLabel = label ?? 'Expires in' | ||
|
||
const fieldDate = useMemo(() => { | ||
if (selectedItem.period === 0) return null | ||
|
||
return new Date(new Date().getTime() + selectedItem.period * dayMs) | ||
}, [selectedItem]) | ||
|
||
const fieldHint = useMemo(() => { | ||
if (!fieldDate) return | ||
|
||
const date = new Date(fieldDate) | ||
date.setHours(0, 0, 0, 0) | ||
|
||
return `The token will expire on ${date.toLocaleDateString()}` | ||
}, [fieldDate]) | ||
|
||
const dateValue = useMemo(() => { | ||
let value = '' | ||
|
||
if (fieldDate) { | ||
value = fieldDate.toISOString().replace(msExp, 'Z') | ||
} else if (selectedItem.id === 5 ) { | ||
value = pickedDate.toISOString().replace(msExp, 'Z') | ||
} | ||
|
||
return value | ||
}, [fieldDate, selectedItem, pickedDate]) | ||
|
||
const handleOnChange = (_value: string, event: FormEvent<HTMLSelectElement>) => { | ||
const value = (event.target as HTMLSelectElement).value | ||
const selected = collection.find(i => i.id.toString() === value) ?? null | ||
|
||
if (selected === null) return | ||
|
||
setSelectedItem(selected) | ||
setPickedDate(new Date()) | ||
} | ||
|
||
return ( | ||
<> | ||
<FormGroup | ||
isRequired | ||
fieldId={fieldName} | ||
label={fieldLabel} | ||
> | ||
<FormSelect | ||
className="pf-c-form-control-expiration" | ||
id={fieldName} | ||
value={selectedItem.id} | ||
onChange={handleOnChange} | ||
> | ||
{collection.map((item: ExpirationItem) => { | ||
return ( | ||
<FormSelectOption | ||
key={item.id} | ||
label={item.name} | ||
value={item.id} | ||
/> | ||
) | ||
})} | ||
</FormSelect> | ||
<span className="pf-c-form-control-expiration-hint">{fieldHint}</span> | ||
</FormGroup> | ||
<input id={id} name={id} type="hidden" value={dateValue} /> | ||
{selectedItem.id === 5 && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love these synthetic ids on the options list. It's a bit hard to understand why this is checked for equality with Some alternative ideas I had:
or something like that. Or even remove the I also suggest to use
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Done fda4ff2
I didn't do this because I find this as confusing as the previous code.
Done 14b083d |
||
<> | ||
<br /> | ||
<CalendarMonth date={pickedDate} onChange={setPickedDate} /> | ||
<br /> | ||
</> | ||
)} | ||
{dateValue === '' && ( | ||
<> | ||
<br /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like this break here, but I couldn't find an easy way to add some top margin, so that's OK... I was looking at https://pf4.patternfly.org/utilities/spacing/, and was hoping that adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, now I think the parenthesis |
||
<Alert title="Expiration is recommended" variant="warning"> | ||
It is strongly recommended that you set an expiration date for your token to help keep your information | ||
secure | ||
</Alert> | ||
</> | ||
)} | ||
</> | ||
) | ||
} | ||
|
||
const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(<ExpirationDatePicker id={props.id} label={props.label} />, containerId) } | ||
|
||
export type { ExpirationItem, Props } | ||
export { ExpirationDatePicker, ExpirationDatePickerWrapper } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
import { mount } from 'enzyme' | ||
|
||
import { ExpirationDatePicker } from 'AccessTokens/components/ExpirationDatePicker' | ||
|
||
import type { ExpirationItem, Props } from 'AccessTokens/components/ExpirationDatePicker' | ||
import type { ReactWrapper } from 'enzyme' | ||
|
||
const defaultProps: Props = { | ||
id: 'expires_at', | ||
label: 'Expires in' | ||
} | ||
|
||
const mountWrapper = (props: Partial<Props> = {}) => mount(<ExpirationDatePicker {...{ ...defaultProps, ...props }} />) | ||
|
||
const selectItem = (wrapper: ReactWrapper<any, Readonly<object>>, item: ExpirationItem) => { | ||
wrapper.find('select.pf-c-form-control-expiration').simulate('change', { target: { value: item.id.toString() } }) | ||
} | ||
|
||
const pickDate = (wrapper: ReactWrapper<any, Readonly<object>>) => { | ||
/* | ||
* Pick tomorrow, to do so, we get the date selected by default which is today and click the next one. | ||
* It could happen that today is the last day in the calendar, in that case we pick the previous day, yesterday. | ||
* In any case, we return the picked date to the caller. | ||
*/ | ||
const targetDate = new Date() | ||
targetDate.setHours(0) | ||
targetDate.setMinutes(0) | ||
targetDate.setSeconds(0) | ||
targetDate.setMilliseconds(0) | ||
|
||
const tomorrowButton = wrapper.find('.pf-m-selected + td > button') | ||
|
||
if (tomorrowButton.length === 0) { | ||
// No tomorrow, pick yesterday | ||
const dayButtons = wrapper.find('button.pf-c-calendar-month__date') | ||
const yesterdayButton = dayButtons.at(dayButtons.length - 2) | ||
|
||
yesterdayButton.simulate('click') | ||
targetDate.setDate(targetDate.getDate() - 1) | ||
return targetDate | ||
} | ||
|
||
tomorrowButton.simulate('click') | ||
targetDate.setDate(targetDate.getDate() + 1) | ||
return targetDate | ||
} | ||
|
||
it('should render itself', () => { | ||
const wrapper = mountWrapper() | ||
expect(wrapper.exists()).toEqual(true) | ||
}) | ||
|
||
describe('select a period', () => { | ||
const targetItem: ExpirationItem = { id: 4, name: '90 days', period: 90 } | ||
|
||
it('should update hint to the correct date', () => { | ||
const wrapper = mountWrapper() | ||
const targetDate = new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * targetItem.period) | ||
const expectedHint = `The token will expire on ${targetDate.toLocaleDateString()}` | ||
|
||
selectItem(wrapper, targetItem) | ||
const hint = wrapper.find('.pf-c-form-control-expiration-hint').text() | ||
|
||
expect(hint).toBe(expectedHint) | ||
}) | ||
|
||
it('should update hidden input value to the correct timestamp', () => { | ||
const wrapper = mountWrapper() | ||
const targetDate = new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * targetItem.period) | ||
const expectedValue = targetDate.toISOString().replace(/\.\d{3}Z$/, 'Z') | ||
|
||
selectItem(wrapper, targetItem) | ||
const value = wrapper.find(`input#${defaultProps.id}`).prop('value') | ||
|
||
expect(value).toBe(expectedValue) | ||
}) | ||
}) | ||
|
||
describe('select "Custom"', () => { | ||
const targetItem: ExpirationItem = { id: 5, name: 'Custom...', period: 0 } | ||
|
||
it('should show a calendar', () => { | ||
const wrapper = mountWrapper() | ||
|
||
selectItem(wrapper, targetItem) | ||
const calendar = wrapper.find('.pf-c-calendar-month') | ||
|
||
expect(calendar.exists()).toBe(true) | ||
}) | ||
|
||
describe('pick a date from the calendar', () => { | ||
it('should update hidden input value to the correct timestamp', () => { | ||
const wrapper = mountWrapper() | ||
|
||
selectItem(wrapper, targetItem) | ||
const targetDate = pickDate(wrapper) | ||
const expectedValue = targetDate.toISOString().replace(/\.\d{3}Z$/, 'Z') | ||
const value = wrapper.find(`input#${defaultProps.id}`).prop('value') | ||
|
||
expect(value).toBe(expectedValue) | ||
}) | ||
}) | ||
}) | ||
|
||
describe('select "No expiration"', () => { | ||
const targetItem: ExpirationItem = { id: 6, name: 'No expiration', period: 0 } | ||
|
||
it('should show a warning', () => { | ||
const wrapper = mountWrapper() | ||
|
||
selectItem(wrapper, targetItem) | ||
const warning = wrapper.find('.pf-c-alert.pf-m-warning') | ||
|
||
expect(warning.exists()).toBe(true) | ||
}) | ||
|
||
it('should update hidden input value to empty', () => { | ||
const wrapper = mountWrapper() | ||
const expectedValue = '' | ||
|
||
selectItem(wrapper, targetItem) | ||
const value = wrapper.find(`input#${defaultProps.id}`).prop('value') | ||
|
||
expect(value).toBe(expectedValue) | ||
}) | ||
}) | ||
|
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.
Removed the suggestion, because noticed that it's not just "if-else", but "if - else if", and value can be empty at the return.
Maybe we can accept both formats on the backend by using
DateTime.parse
? It seems to work both with milliseconds and no milliseconds.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.
I'll explore this possibility, it might work
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.
d2f7411