From dc05a734cc96c7c23b86733cc82fd9932b8aad80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Tue, 26 Nov 2024 12:14:17 +0100 Subject: [PATCH 01/42] New React component: ExpirationDatePicker --- .../components/ExpirationDatePicker.scss | 7 + .../components/ExpirationDatePicker.tsx | 126 ++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss create mode 100644 app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss new file mode 100644 index 0000000000..eebf459957 --- /dev/null +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss @@ -0,0 +1,7 @@ +.pf-c-calendar-month, .pf-c-form-control-expiration { + width: 50%; +} + +.pf-c-form-control-expiration { + margin-right: 1em; +} diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx new file mode 100644 index 0000000000..7f20546b38 --- /dev/null +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -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 = ({ 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) => { + 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 ( + <> + + + {collection.map((item: ExpirationItem) => { + return ( + + ) + })} + + {fieldHint} + + + {selectedItem.id === 5 && ( + <> +
+ +
+ + )} + {dateValue === '' && ( + <> +
+ + It is strongly recommended that you set an expiration date for your token to help keep your information + secure + + + )} + + ) +} + +const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(, containerId) } + +export type { ExpirationItem, Props } +export { ExpirationDatePicker, ExpirationDatePickerWrapper } From 4f4690d3cd9bbce14fd5bb5385ab7abedfb56ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Tue, 26 Nov 2024 12:14:30 +0100 Subject: [PATCH 02/42] Add the new component to Access Token forms --- .../packs/expiration_date_picker.ts | 22 +++++++++++++++++++ .../admin/user/access_tokens/_form.html.slim | 11 ++++++++++ .../admin/user/access_tokens/index.html.slim | 7 ++++++ .../admin/user/access_tokens/new.html.slim | 1 + config/locales/en.yml | 4 ++++ 5 files changed, 45 insertions(+) create mode 100644 app/javascript/packs/expiration_date_picker.ts diff --git a/app/javascript/packs/expiration_date_picker.ts b/app/javascript/packs/expiration_date_picker.ts new file mode 100644 index 0000000000..1425b79051 --- /dev/null +++ b/app/javascript/packs/expiration_date_picker.ts @@ -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(container.dataset.props) + + if (!props) { + throw new Error('Missing props') + } + + ExpirationDatePickerWrapper(props, containerId) +}) diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index eccdf3a684..ec0cb28f26 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -7,3 +7,14 @@ = form.input :permission, as: :patternfly_select, collection: @access_token.available_permissions, include_blank: false + +- if @access_token.persisted? + .pf-c-form__group + .pf-c-form__group-label + label.pf-c-form__label + span.pf-c-form__label-text + = t('.expires_at') + .pf-c-form__group-control + = @access_token.expires_at.present? ? l(@access_token.expires_at) : t('provider.admin.user.access_tokens.no_expiration') +- else + div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('.expires_in') }.to_json diff --git a/app/views/provider/admin/user/access_tokens/index.html.slim b/app/views/provider/admin/user/access_tokens/index.html.slim index d571f646db..1e50323b63 100644 --- a/app/views/provider/admin/user/access_tokens/index.html.slim +++ b/app/views/provider/admin/user/access_tokens/index.html.slim @@ -31,6 +31,13 @@ dd class="pf-c-description-list__description" div class="pf-c-description-list__text" = token.human_permission + div class="pf-c-description-list__group" + dt class="pf-c-description-list__term" + span class="pf-c-description-list__text" + | Expires at + dd class="pf-c-description-list__description" + div class="pf-c-description-list__text" + = token.expires_at.present? ? l(token.expires_at) : t('provider.admin.user.access_tokens.no_expiration') div class="pf-c-description-list__group" dt class="pf-c-description-list__term" span class="pf-c-description-list__text" diff --git a/app/views/provider/admin/user/access_tokens/new.html.slim b/app/views/provider/admin/user/access_tokens/new.html.slim index be3ace8aaa..d69e35a522 100644 --- a/app/views/provider/admin/user/access_tokens/new.html.slim +++ b/app/views/provider/admin/user/access_tokens/new.html.slim @@ -2,6 +2,7 @@ - content_for :javascripts do = javascript_packs_with_chunks_tag 'pf_form' + = javascript_packs_with_chunks_tag 'expiration_date_picker' div class="pf-c-card" div class="pf-c-card__body" diff --git a/config/locales/en.yml b/config/locales/en.yml index 3c98638064..8609fc39bb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -510,6 +510,10 @@ en:

user: access_tokens: + no_expiration: Never expires + form: + expires_at: Expires at + expires_in: Expires in edit: page_header_title: Edit Access Token submit_button_label: Update Access Token From 995530a210c1eb11e5cf98bc76bdda64c2d2cea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Tue, 26 Nov 2024 12:14:38 +0100 Subject: [PATCH 03/42] Access token UI: Update and add tests --- .../provider/admin/user/access_tokens.feature | 20 +-- .../components/ExpirationDatePicker.spec.tsx | 127 ++++++++++++++++++ 2 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx diff --git a/features/provider/admin/user/access_tokens.feature b/features/provider/admin/user/access_tokens.feature index 191277e5f2..15c5343ca9 100644 --- a/features/provider/admin/user/access_tokens.feature +++ b/features/provider/admin/user/access_tokens.feature @@ -42,6 +42,7 @@ Feature: Provider Admin Access tokens Then there is a required field "Name" And there is a required field "Scopes" And there is a required field "Permission" + And there is a required field "Expires in" And the submit button is enabled Scenario: Create access tokens without required fields @@ -49,26 +50,29 @@ Feature: Provider Admin Access tokens Then field "Name" has inline error "can't be blank" And field "Scopes" has inline error "select at least one scope" And field "Permission" has no inline error + And field "Expires in" has no inline error Scenario: Create access token When they press "Create Access Token" And the form is submitted with: - | Name | LeToken | - | Analytics API | Yes | - | Permission | Read & Write | + | Name | LeToken | + | Analytics API | Yes | + | Permission | Read & Write | + | Expires in | No expiration | Then the current page is the personal tokens page And they should see the flash message "Access token was successfully created" And should see the following details: - | Name | LeToken | - | Scopes | Analytics API | - | Permission | Read & Write | + | Name | LeToken | + | Scopes | Analytics API | + | Permission | Read & Write | + | Expires at | Never expires | And there should be a link to "I have copied the token" Rule: Edit page Background: Given the provider has the following access tokens: - | Name | Scopes | Permission | - | LeToken | Billing API, Analytics API | Read Only | + | Name | Scopes | Permission | Expires at | + | LeToken | Billing API, Analytics API | Read Only | 2030-01-01T00:00:00Z | And they go to the access token's edit page Scenario: Navigation to edit page diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx new file mode 100644 index 0000000000..dbd9d91e26 --- /dev/null +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -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 = {}) => mount() + +const selectItem = (wrapper: ReactWrapper>, item: ExpirationItem) => { + wrapper.find('select.pf-c-form-control-expiration').simulate('change', { target: { value: item.id.toString() } }) +} + +const pickDate = (wrapper: ReactWrapper>) => { + /* + * 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) + }) +}) + From 4036f22506bd63824385258e798d9d3fad484279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Wed, 4 Dec 2024 09:24:02 +0100 Subject: [PATCH 04/42] Move access token strings to root level --- .../provider/admin/user/access_tokens/_form.html.slim | 6 +++--- .../provider/admin/user/access_tokens/index.html.slim | 2 +- config/locales/en.yml | 7 +++---- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index ec0cb28f26..876f30ef68 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -13,8 +13,8 @@ .pf-c-form__group-label label.pf-c-form__label span.pf-c-form__label-text - = t('.expires_at') + = t('access_token_options.expires_at') .pf-c-form__group-control - = @access_token.expires_at.present? ? l(@access_token.expires_at) : t('provider.admin.user.access_tokens.no_expiration') + = @access_token.expires_at.present? ? l(@access_token.expires_at) : t('access_token_options.no_expiration') - else - div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('.expires_in') }.to_json + div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('access_token_options.expires_in') }.to_json diff --git a/app/views/provider/admin/user/access_tokens/index.html.slim b/app/views/provider/admin/user/access_tokens/index.html.slim index 1e50323b63..1af5455351 100644 --- a/app/views/provider/admin/user/access_tokens/index.html.slim +++ b/app/views/provider/admin/user/access_tokens/index.html.slim @@ -37,7 +37,7 @@ | Expires at dd class="pf-c-description-list__description" div class="pf-c-description-list__text" - = token.expires_at.present? ? l(token.expires_at) : t('provider.admin.user.access_tokens.no_expiration') + = token.expires_at.present? ? l(token.expires_at) : t('access_token_options.no_expiration') div class="pf-c-description-list__group" dt class="pf-c-description-list__term" span class="pf-c-description-list__text" diff --git a/config/locales/en.yml b/config/locales/en.yml index 8609fc39bb..966ee00e2e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -65,6 +65,9 @@ en: cms: 'Developer Portal API' ro: 'Read Only' rw: 'Read & Write' + no_expiration: 'Never expires' + expires_at: 'Expires at' + expires_in: 'Expires in' notification_category_titles: account: 'Accounts' application: 'Applications' @@ -510,10 +513,6 @@ en:

user: access_tokens: - no_expiration: Never expires - form: - expires_at: Expires at - expires_in: Expires in edit: page_header_title: Edit Access Token submit_button_label: Update Access Token From d2f74118a5ef06c481056d65de34b645ae011065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 10:26:24 +0100 Subject: [PATCH 05/42] Accept timestamps with milliseconds --- app/models/access_token.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/access_token.rb b/app/models/access_token.rb index 1b1b79cad6..c38d0f224d 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -140,7 +140,7 @@ def validate_scope_exists def expires_at=(value) return if value.blank? - DateTime.strptime(value) + DateTime.parse(value) super value rescue StandardError From 6d8868300910ecd4fc10f80d64f4d179088e2d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 10:26:43 +0100 Subject: [PATCH 06/42] Send timestamps with milliseconds For simplicity --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 7f20546b38..3ad9564629 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -24,7 +24,6 @@ const collection: ExpirationItem[] = [ ] const dayMs = 60 * 60 * 24 * 1000 -const msExp = /\.\d{3}Z$/ interface Props { id: string; @@ -56,9 +55,9 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { let value = '' if (fieldDate) { - value = fieldDate.toISOString().replace(msExp, 'Z') + value = fieldDate.toISOString() } else if (selectedItem.id === 5 ) { - value = pickedDate.toISOString().replace(msExp, 'Z') + value = pickedDate.toISOString() } return value From 14b083d8d4eae1262f6f043a37ab0e349acd4fe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 10:37:41 +0100 Subject: [PATCH 07/42] Rename `ExpirationItem.name` to `label` Also we don't need to extend from `IRecord` --- .../components/ExpirationDatePicker.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 3ad9564629..edc3f7dc5e 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -2,25 +2,24 @@ 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 { +interface ExpirationItem { id: number; - name: string; + label: 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 } + { id: 1, label: '7 days', period: 7 }, + { id: 2, label: '30 days', period: 30 }, + { id: 3, label: '60 days', period: 60 }, + { id: 4, label: '90 days', period: 90 }, + { id: 5, label: 'Custom...', period: 0 }, + { id: 6, label: 'No expiration', period: 0 } ] const dayMs = 60 * 60 * 24 * 1000 @@ -90,7 +89,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { return ( ) From fda4ff27cc8055c67523056c3ce401910da82670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 12:04:10 +0100 Subject: [PATCH 08/42] Use more meaningful ids --- .../components/ExpirationDatePicker.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index edc3f7dc5e..1168706a21 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -8,18 +8,18 @@ import type { FunctionComponent, FormEvent } from 'react' import './ExpirationDatePicker.scss' interface ExpirationItem { - id: number; + id: string; label: string; period: number; // In seconds } const collection: ExpirationItem[] = [ - { id: 1, label: '7 days', period: 7 }, - { id: 2, label: '30 days', period: 30 }, - { id: 3, label: '60 days', period: 60 }, - { id: 4, label: '90 days', period: 90 }, - { id: 5, label: 'Custom...', period: 0 }, - { id: 6, label: 'No expiration', period: 0 } + { id: '7', label: '7 days', period: 7 }, + { id: '30', label: '30 days', period: 30 }, + { id: '60', label: '60 days', period: 60 }, + { id: '90', label: '90 days', period: 90 }, + { id: 'custom', label: 'Custom...', period: 0 }, + { id: 'no-exp', label: 'No expiration', period: 0 } ] const dayMs = 60 * 60 * 24 * 1000 @@ -55,7 +55,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { if (fieldDate) { value = fieldDate.toISOString() - } else if (selectedItem.id === 5 ) { + } else if (selectedItem.id === 'custom' ) { value = pickedDate.toISOString() } @@ -64,7 +64,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { const handleOnChange = (_value: string, event: FormEvent) => { const value = (event.target as HTMLSelectElement).value - const selected = collection.find(i => i.id.toString() === value) ?? null + const selected = collection.find(i => i.id === value) ?? null if (selected === null) return @@ -98,7 +98,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { {fieldHint} - {selectedItem.id === 5 && ( + {selectedItem.id === 'custom' && ( <>
From 667e146897df2fcb5492559d673909de45654221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 12:51:26 +0100 Subject: [PATCH 09/42] Better spacing --- .../src/AccessTokens/components/ExpirationDatePicker.scss | 2 ++ .../src/AccessTokens/components/ExpirationDatePicker.tsx | 6 +----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss index eebf459957..0e20f15e7b 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss @@ -1,3 +1,5 @@ +@import '~@patternfly/patternfly/patternfly-addons.css'; + .pf-c-calendar-month, .pf-c-form-control-expiration { width: 50%; } diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 1168706a21..b8ddff3495 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -99,11 +99,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { {selectedItem.id === 'custom' && ( - <> -
- -
- + )} {dateValue === '' && ( <> From 092195111f2db867f00c0c5ca32cd488f3855f67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 13:15:39 +0100 Subject: [PATCH 10/42] Fix tests after changes --- .../components/ExpirationDatePicker.spec.tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx index dbd9d91e26..9efbf429c5 100644 --- a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -5,6 +5,8 @@ import { ExpirationDatePicker } from 'AccessTokens/components/ExpirationDatePick import type { ExpirationItem, Props } from 'AccessTokens/components/ExpirationDatePicker' import type { ReactWrapper } from 'enzyme' +const msExp = /\.\d{3}Z$/ + const defaultProps: Props = { id: 'expires_at', label: 'Expires in' @@ -51,7 +53,7 @@ it('should render itself', () => { }) describe('select a period', () => { - const targetItem: ExpirationItem = { id: 4, name: '90 days', period: 90 } + const targetItem: ExpirationItem = { id: '90', label: '90 days', period: 90 } it('should update hint to the correct date', () => { const wrapper = mountWrapper() @@ -67,17 +69,17 @@ describe('select a period', () => { 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') + const expectedValue = targetDate.toISOString().replace(msExp, 'Z') selectItem(wrapper, targetItem) - const value = wrapper.find(`input#${defaultProps.id}`).prop('value') + const value = (wrapper.find(`input#${defaultProps.id}`).prop('value') as string).replace(msExp, 'Z') expect(value).toBe(expectedValue) }) }) describe('select "Custom"', () => { - const targetItem: ExpirationItem = { id: 5, name: 'Custom...', period: 0 } + const targetItem: ExpirationItem = { id: 'custom', label: 'Custom...', period: 0 } it('should show a calendar', () => { const wrapper = mountWrapper() @@ -94,8 +96,8 @@ describe('select "Custom"', () => { 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') + const expectedValue = targetDate.toISOString().replace(msExp, 'Z') + const value = (wrapper.find(`input#${defaultProps.id}`).prop('value') as string).replace(msExp, 'Z') expect(value).toBe(expectedValue) }) @@ -103,7 +105,7 @@ describe('select "Custom"', () => { }) describe('select "No expiration"', () => { - const targetItem: ExpirationItem = { id: 6, name: 'No expiration', period: 0 } + const targetItem: ExpirationItem = { id: 'no-exp', label: 'No expiration', period: 0 } it('should show a warning', () => { const wrapper = mountWrapper() From 588b314f4cdb3a95a12f5eacb555b7737e074e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 14:17:06 +0100 Subject: [PATCH 11/42] Show the hint as a helper text --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index b8ddff3495..362c8e442d 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -77,6 +77,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { = ({ id, label }) => { ) })} - {fieldHint} {selectedItem.id === 'custom' && ( From 587a618467d385db4f1bc152d14c39bdb1138490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 14:17:56 +0100 Subject: [PATCH 12/42] Hint visible when picking a date from calendar --- .../components/ExpirationDatePicker.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 362c8e442d..aa4eb28213 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -41,15 +41,6 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { 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 = '' @@ -62,6 +53,14 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { return value }, [fieldDate, selectedItem, pickedDate]) + const fieldHint = useMemo(() => { + if (!dateValue) return + + const date = new Date(dateValue) + + return `The token will expire on ${date.toLocaleDateString()}` + }, [dateValue]) + const handleOnChange = (_value: string, event: FormEvent) => { const value = (event.target as HTMLSelectElement).value const selected = collection.find(i => i.id === value) ?? null From 39b8c24881ce6404da5842daa753a188024276d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 14:20:26 +0100 Subject: [PATCH 13/42] Update tests after changes --- .../components/ExpirationDatePicker.spec.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx index 9efbf429c5..2cb7ddd851 100644 --- a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -61,7 +61,7 @@ describe('select a period', () => { const expectedHint = `The token will expire on ${targetDate.toLocaleDateString()}` selectItem(wrapper, targetItem) - const hint = wrapper.find('.pf-c-form-control-expiration-hint').text() + const hint = wrapper.find('.pf-c-form__helper-text').text() expect(hint).toBe(expectedHint) }) @@ -91,6 +91,17 @@ describe('select "Custom"', () => { }) describe('pick a date from the calendar', () => { + it('should update hint to the correct date', () => { + const wrapper = mountWrapper() + + selectItem(wrapper, targetItem) + const targetDate = pickDate(wrapper) + const expectedHint = `The token will expire on ${targetDate.toLocaleDateString()}` + const hint = wrapper.find('.pf-c-form__helper-text').text() + + expect(hint).toBe(expectedHint) + }) + it('should update hidden input value to the correct timestamp', () => { const wrapper = mountWrapper() From 88badaf88b1bf5e40c98811ab26a06d3641d3526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 14:36:27 +0100 Subject: [PATCH 14/42] Add expiration time to index table --- app/views/provider/admin/user/access_tokens/index.html.slim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/provider/admin/user/access_tokens/index.html.slim b/app/views/provider/admin/user/access_tokens/index.html.slim index 1af5455351..163a7edcc6 100644 --- a/app/views/provider/admin/user/access_tokens/index.html.slim +++ b/app/views/provider/admin/user/access_tokens/index.html.slim @@ -65,6 +65,7 @@ tr role="row" th role="columnheader" scope="col" Name th role="columnheader" scope="col" Scopes + th role="columnheader" scope="col" Expiration th role="columnheader" scope="col" Permission th role="columnheader" scope="col" class="pf-c-table__action pf-m-fit-content" = fancy_link_to 'Add Access Token', new_provider_admin_user_access_token_path, class: 'new' if allowed_scopes.any? @@ -74,6 +75,7 @@ tr role="row" td role="cell" data-label="Name" = token.name td role="cell" data-label="Scopes" = token.human_scopes.to_sentence + td role="cell" data-label="Expiration" = token.expires_at.present? ? l(token.expires_at) : t('access_token_options.no_expiration') td role="cell" data-label="Permission" = token.human_permission td role="cell" class="pf-c-table__action" div class="pf-c-overflow-menu" From bfd0398e768ec05ca4bfdfce9b9d9b584c84b543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 5 Dec 2024 14:36:34 +0100 Subject: [PATCH 15/42] Update cukes after changes --- features/provider/admin/user/access_tokens.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/provider/admin/user/access_tokens.feature b/features/provider/admin/user/access_tokens.feature index 15c5343ca9..eb9222edb0 100644 --- a/features/provider/admin/user/access_tokens.feature +++ b/features/provider/admin/user/access_tokens.feature @@ -24,9 +24,9 @@ Feature: Provider Admin Access tokens Scenario: Tokens are listed in a table Then the table should contain the following: - | Name | Scopes | Permission | - | Potato | Analytics API | Read Only | - | Banana | Billing API | Read & Write | + | Name | Scopes | Expiration | Permission | + | Potato | Analytics API | Never expires | Read Only | + | Banana | Billing API | Never expires | Read & Write | Rule: New page Background: From 62d048dd4cf1d3a5463fc3d963399c4360d72555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 12 Dec 2024 12:53:59 +0100 Subject: [PATCH 16/42] Better hint date format --- .../components/ExpirationDatePicker.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index aa4eb28213..110c3aeb01 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -53,14 +53,23 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { return value }, [fieldDate, selectedItem, pickedDate]) - const fieldHint = useMemo(() => { + const formattedDateValue = useMemo(() => { if (!dateValue) return + const formatter = Intl.DateTimeFormat('en-US', { + month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false + }) const date = new Date(dateValue) - return `The token will expire on ${date.toLocaleDateString()}` + return formatter.format(date) }, [dateValue]) + const fieldHint = useMemo(() => { + if (!formattedDateValue) return + + return `The token will expire on ${formattedDateValue}` + }, [formattedDateValue]) + const handleOnChange = (_value: string, event: FormEvent) => { const value = (event.target as HTMLSelectElement).value const selected = collection.find(i => i.id === value) ?? null From 7f451448a0acae7c7714e073f2a3b79da4ad4b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 12 Dec 2024 13:55:03 +0100 Subject: [PATCH 17/42] Add a time zone warning Add a warning when the time zone differs from the provider's one --- .../admin/user/access_tokens_controller.rb | 89 +++++++++++-------- .../components/ExpirationDatePicker.scss | 5 ++ .../components/ExpirationDatePicker.tsx | 45 +++++++++- .../admin/user/access_tokens_new_presenter.rb | 13 +++ .../admin/user/access_tokens/_form.html.slim | 2 +- 5 files changed, 113 insertions(+), 41 deletions(-) create mode 100644 app/presenters/provider/admin/user/access_tokens_new_presenter.rb diff --git a/app/controllers/provider/admin/user/access_tokens_controller.rb b/app/controllers/provider/admin/user/access_tokens_controller.rb index dc2d56ef68..fe248d8487 100644 --- a/app/controllers/provider/admin/user/access_tokens_controller.rb +++ b/app/controllers/provider/admin/user/access_tokens_controller.rb @@ -1,44 +1,59 @@ -class Provider::Admin::User::AccessTokensController < Provider::Admin::User::BaseController - inherit_resources - defaults route_prefix: 'provider_admin_user', resource_class: AccessToken - actions :index, :new, :create, :edit, :update, :destroy - - authorize_resource - activate_menu :account, :personal, :tokens - before_action :authorize_access_tokens - before_action :disable_client_cache - - def create - create! do |success, _failure| - success.html do - flash[:token] = @access_token.id - flash[:notice] = 'Access Token was successfully created.' - redirect_to(collection_url) - end - end - end +# frozen_string_literal: true - def index - index! - @last_access_key = flash[:token] - end +module Provider + module Admin + module User + class AccessTokensController < BaseController + inherit_resources + defaults route_prefix: 'provider_admin_user', resource_class: AccessToken + actions :index, :new, :create, :edit, :update, :destroy - def update - update! do |success, _failure| - success.html do - flash[:notice] = 'Access Token was successfully updated.' - redirect_to(collection_url) - end - end - end + authorize_resource + activate_menu :account, :personal, :tokens + before_action :authorize_access_tokens + before_action :disable_client_cache + before_action :load_presenter, only: %i[new create] - private + def new; end - def authorize_access_tokens - authorize! :manage, :access_tokens, current_user - end + def create + create! do |success, _failure| + success.html do + flash[:token] = @access_token.id + flash[:notice] = 'Access Token was successfully created.' + redirect_to(collection_url) + end + end + end - def begin_of_association_chain - current_user + def index + index! + @last_access_key = flash[:token] + end + + def update + update! do |success, _failure| + success.html do + flash[:notice] = 'Access Token was successfully updated.' + redirect_to(collection_url) + end + end + end + + private + + def load_presenter + @presenter = AccessTokensNewPresenter.new(current_account) + end + + def authorize_access_tokens + authorize! :manage, :access_tokens, current_user + end + + def begin_of_association_chain + current_user + end + end + end end end diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss index 0e20f15e7b..7ca7c4307f 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss @@ -7,3 +7,8 @@ .pf-c-form-control-expiration { margin-right: 1em; } + +button.pf-c-form__group-label-help { + min-width: auto; + width: auto; +} diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 110c3aeb01..360a95a3b5 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -1,5 +1,6 @@ import { useState, useMemo } from 'react' -import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption } from '@patternfly/react-core' +import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption, Popover } from '@patternfly/react-core' +import HelpIcon from '@patternfly/react-icons/dist/esm/icons/help-icon' import { createReactWrapper } from 'utilities/createReactWrapper' @@ -27,9 +28,10 @@ const dayMs = 60 * 60 * 24 * 1000 interface Props { id: string; label: string | null; + tzOffset?: number; } -const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { +const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { const [selectedItem, setSelectedItem] = useState(collection[0]) const [pickedDate, setPickedDate] = useState(new Date()) const fieldName = `human_${id}` @@ -70,6 +72,42 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { return `The token will expire on ${formattedDateValue}` }, [formattedDateValue]) + const tzMismatch = useMemo(() => { + if (tzOffset === undefined) return + + // Timezone offset in the same format as ActiveSupport + const jsTzOffset = new Date().getTimezoneOffset() * -60 + + return jsTzOffset !== tzOffset + }, [tzOffset]) + + const labelIcon = useMemo(() => { + if (!tzMismatch) return + + return ( + + Your local time zone differs from the provider default. + The token will expire at the time you selected in your local time zone. +

+ )} + headerContent={( + Time zone mismatch + )} + > + + + ) + }, [tzMismatch]) + const handleOnChange = (_value: string, event: FormEvent) => { const value = (event.target as HTMLSelectElement).value const selected = collection.find(i => i.id === value) ?? null @@ -87,6 +125,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label }) => { fieldId={fieldName} helperText={fieldHint} label={fieldLabel} + labelIcon={labelIcon} > = ({ id, label }) => { ) } -const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(, containerId) } +const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(, containerId) } export type { ExpirationItem, Props } export { ExpirationDatePicker, ExpirationDatePickerWrapper } diff --git a/app/presenters/provider/admin/user/access_tokens_new_presenter.rb b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb new file mode 100644 index 0000000000..ac85c33f5f --- /dev/null +++ b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Provider::Admin::User::AccessTokensNewPresenter + + def initialize(provider) + @provider = provider + @timezone = ActiveSupport::TimeZone.new(provider.timezone) + end + + def provider_timezone_offset + @timezone.utc_offset + end +end diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index 876f30ef68..8a32a15976 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -17,4 +17,4 @@ .pf-c-form__group-control = @access_token.expires_at.present? ? l(@access_token.expires_at) : t('access_token_options.no_expiration') - else - div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('access_token_options.expires_in') }.to_json + div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('access_token_options.expires_in'), tzOffset: @presenter.provider_timezone_offset }.to_json From 05bc601cf4d8ae9c6229791fa7e7cb7f10bf5aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 12 Dec 2024 14:27:46 +0100 Subject: [PATCH 18/42] Use OutlinedQuestionCircleIcon instead of HelpIcon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Jest complains about wrong syntax ¯\_(ツ)_/¯ --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 360a95a3b5..2e0f137e24 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -1,6 +1,7 @@ import { useState, useMemo } from 'react' import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption, Popover } from '@patternfly/react-core' -import HelpIcon from '@patternfly/react-icons/dist/esm/icons/help-icon' +import OutlinedQuestionCircleIcon from '@patternfly/react-icons/dist/js/icons/outlined-question-circle-icon' + import { createReactWrapper } from 'utilities/createReactWrapper' @@ -102,7 +103,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) className="pf-c-form__group-label-help" type="button" > - + ) From 6dbcdfed7c9abd33864a58007f01401e284cc392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 12 Dec 2024 14:28:08 +0100 Subject: [PATCH 19/42] Add tests for time zone mismatch warning --- .../components/ExpirationDatePicker.spec.tsx | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx index 2cb7ddd851..849d5fca2f 100644 --- a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -9,7 +9,8 @@ const msExp = /\.\d{3}Z$/ const defaultProps: Props = { id: 'expires_at', - label: 'Expires in' + label: 'Expires in', + tzOffset: 0 } const mountWrapper = (props: Partial = {}) => mount() @@ -47,6 +48,10 @@ const pickDate = (wrapper: ReactWrapper>) => { return targetDate } +const dateFormatter = Intl.DateTimeFormat('en-US', { + month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false +}) + it('should render itself', () => { const wrapper = mountWrapper() expect(wrapper.exists()).toEqual(true) @@ -58,7 +63,7 @@ describe('select a period', () => { 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()}` + const expectedHint = `The token will expire on ${dateFormatter.format(targetDate)}` selectItem(wrapper, targetItem) const hint = wrapper.find('.pf-c-form__helper-text').text() @@ -96,7 +101,7 @@ describe('select "Custom"', () => { selectItem(wrapper, targetItem) const targetDate = pickDate(wrapper) - const expectedHint = `The token will expire on ${targetDate.toLocaleDateString()}` + const expectedHint = `The token will expire on ${dateFormatter.format(targetDate)}` const hint = wrapper.find('.pf-c-form__helper-text').text() expect(hint).toBe(expectedHint) @@ -138,3 +143,20 @@ describe('select "No expiration"', () => { }) }) +describe('time zone matches', () => { + it('should not show a warning', ()=> { + jest.spyOn(Date.prototype, 'getTimezoneOffset').mockImplementation(() => (0)) + const wrapper = mountWrapper() + + expect(wrapper.exists('.pf-c-form__group-label-help')).toEqual(false) + }) +}) + +describe('time zone mismatches', () => { + it('should show a warning', ()=> { + jest.spyOn(Date.prototype, 'getTimezoneOffset').mockImplementation(() => (-120)) + const wrapper = mountWrapper() + + expect(wrapper.exists('.pf-c-form__group-label-help')).toEqual(true) + }) +}) \ No newline at end of file From 63f4496a8ead4a066847702f9b6b672c834b0762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 12 Dec 2024 14:50:52 +0100 Subject: [PATCH 20/42] Fix linter error --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 2e0f137e24..89a0d154f8 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -2,7 +2,6 @@ import { useState, useMemo } from 'react' import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption, Popover } from '@patternfly/react-core' import OutlinedQuestionCircleIcon from '@patternfly/react-icons/dist/js/icons/outlined-question-circle-icon' - import { createReactWrapper } from 'utilities/createReactWrapper' import type { FunctionComponent, FormEvent } from 'react' From 5a7255e7ac93ef315efe7ef140fada8cbe005554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 19 Dec 2024 18:00:45 +0100 Subject: [PATCH 21/42] Use PF class to add top margin Co-authored-by: Daria Mayorova --- .../components/ExpirationDatePicker.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 89a0d154f8..22798afbe3 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -148,15 +148,12 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) {selectedItem.id === 'custom' && ( )} - {dateValue === '' && ( - <> -
- - It is strongly recommended that you set an expiration date for your token to help keep your information - secure - - - )} + {dateValue === '' && + + It is strongly recommended that you set an expiration date for your token to help keep your information + secure + + } ) } From 85abc92374380cf6548cfca896d84f48baec36b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 19 Dec 2024 18:15:30 +0100 Subject: [PATCH 22/42] Apply suggestions from #3954 --- .../components/ExpirationDatePicker.tsx | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 22798afbe3..c5aedf6f19 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -32,39 +32,38 @@ interface Props { } const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { - const [selectedItem, setSelectedItem] = useState(collection[0]) - const [pickedDate, setPickedDate] = useState(new Date()) + const [dropdownSelectedItem, setDropdownSelectedItem] = useState(collection[0]) + const [calendarPickedDate, setCalendarPickedDate] = useState(new Date()) const fieldName = `human_${id}` const fieldLabel = label ?? 'Expires in' - const fieldDate = useMemo(() => { - if (selectedItem.period === 0) return null + const dropdownDate = useMemo(() => { + if (dropdownSelectedItem.period === 0) return null - return new Date(new Date().getTime() + selectedItem.period * dayMs) - }, [selectedItem]) + return new Date(new Date().getTime() + dropdownSelectedItem.period * dayMs) + }, [dropdownSelectedItem]) - const dateValue = useMemo(() => { - let value = '' + const selectedDate = useMemo(() => { + let value = null - if (fieldDate) { - value = fieldDate.toISOString() - } else if (selectedItem.id === 'custom' ) { - value = pickedDate.toISOString() + if (dropdownDate) { + value = dropdownDate + } else if (dropdownSelectedItem.id === 'custom' ) { + value = calendarPickedDate } return value - }, [fieldDate, selectedItem, pickedDate]) + }, [dropdownDate, dropdownSelectedItem, calendarPickedDate]) const formattedDateValue = useMemo(() => { - if (!dateValue) return + if (!selectedDate) return const formatter = Intl.DateTimeFormat('en-US', { month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false }) - const date = new Date(dateValue) - return formatter.format(date) - }, [dateValue]) + return formatter.format(selectedDate) + }, [selectedDate]) const fieldHint = useMemo(() => { if (!formattedDateValue) return @@ -72,6 +71,12 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) return `The token will expire on ${formattedDateValue}` }, [formattedDateValue]) + const inputDateValue = useMemo(() => { + if (!selectedDate) return '' + + return selectedDate.toISOString() + }, [selectedDate]) + const tzMismatch = useMemo(() => { if (tzOffset === undefined) return @@ -114,8 +119,8 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) if (selected === null) return - setSelectedItem(selected) - setPickedDate(new Date()) + setDropdownSelectedItem(selected) + setCalendarPickedDate(new Date()) } return ( @@ -130,7 +135,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) {collection.map((item: ExpirationItem) => { @@ -144,16 +149,16 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) })} - - {selectedItem.id === 'custom' && ( - + + {dropdownSelectedItem.id === 'custom' && ( + )} - {dateValue === '' && + {!selectedDate && ( It is strongly recommended that you set an expiration date for your token to help keep your information secure - } + )} ) } From 6bec27c7fd0068348ddb3e860d721c5821a457b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Fri, 20 Dec 2024 13:28:12 +0100 Subject: [PATCH 23/42] Calendar: Accept only future dates --- .../components/ExpirationDatePicker.tsx | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index c5aedf6f19..809cb1e860 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -32,15 +32,18 @@ interface Props { } const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { + const today: Date = new Date() + const tomorrow: Date = new Date(today) + tomorrow.setDate(today.getDate() + 1) const [dropdownSelectedItem, setDropdownSelectedItem] = useState(collection[0]) - const [calendarPickedDate, setCalendarPickedDate] = useState(new Date()) + const [calendarPickedDate, setCalendarPickedDate] = useState(tomorrow) const fieldName = `human_${id}` const fieldLabel = label ?? 'Expires in' const dropdownDate = useMemo(() => { if (dropdownSelectedItem.period === 0) return null - return new Date(new Date().getTime() + dropdownSelectedItem.period * dayMs) + return new Date(today.getTime() + dropdownSelectedItem.period * dayMs) }, [dropdownSelectedItem]) const selectedDate = useMemo(() => { @@ -120,7 +123,11 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) if (selected === null) return setDropdownSelectedItem(selected) - setCalendarPickedDate(new Date()) + setCalendarPickedDate(tomorrow) + } + + const dateValidator = (date: Date): boolean => { + return date >= today } return ( @@ -151,7 +158,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) {dropdownSelectedItem.id === 'custom' && ( - + )} {!selectedDate && ( From 25ce970fb832767139ff9b14c6fb62196ccd0ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Fri, 20 Dec 2024 14:14:13 +0100 Subject: [PATCH 24/42] Fix tests --- .../components/ExpirationDatePicker.spec.tsx | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx index 849d5fca2f..8aed14c575 100644 --- a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -31,17 +31,7 @@ const pickDate = (wrapper: ReactWrapper>) => { 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 - } + const tomorrowButton = wrapper.find('.pf-m-selected > button') tomorrowButton.simulate('click') targetDate.setDate(targetDate.getDate() + 1) From b4802bf5adb1af870b2609d7e30e892f8afc738c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 12:41:39 +0100 Subject: [PATCH 25/42] Timezone mismatch warning: Use exclamation icon More likely for the user to click and get warned --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 809cb1e860..1bc1d38b8f 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -1,6 +1,6 @@ import { useState, useMemo } from 'react' import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption, Popover } from '@patternfly/react-core' -import OutlinedQuestionCircleIcon from '@patternfly/react-icons/dist/js/icons/outlined-question-circle-icon' +import ExclamationTriangleIcon from '@patternfly/react-icons/dist/js/icons/exclamation-triangle-icon' import { createReactWrapper } from 'utilities/createReactWrapper' @@ -110,7 +110,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) className="pf-c-form__group-label-help" type="button" > - + ) From ee5b4c26943048ca336d2dc6d6ee7ba5ca81f406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 12:44:02 +0100 Subject: [PATCH 26/42] Don't use relative imports --- app/javascript/packs/expiration_date_picker.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/javascript/packs/expiration_date_picker.ts b/app/javascript/packs/expiration_date_picker.ts index 1425b79051..7d53e3251e 100644 --- a/app/javascript/packs/expiration_date_picker.ts +++ b/app/javascript/packs/expiration_date_picker.ts @@ -1,9 +1,8 @@ import { ExpirationDatePickerWrapper } from 'AccessTokens/components/ExpirationDatePicker' +import { safeFromJsonString } from 'utilities/json-utils' 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) From 6c597c60951df3e3589022a9b3d1b9f15c85c198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 13:32:06 +0100 Subject: [PATCH 27/42] Controller: init presenter in each action --- .../provider/admin/user/access_tokens_controller.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/provider/admin/user/access_tokens_controller.rb b/app/controllers/provider/admin/user/access_tokens_controller.rb index fe248d8487..11e6d083ff 100644 --- a/app/controllers/provider/admin/user/access_tokens_controller.rb +++ b/app/controllers/provider/admin/user/access_tokens_controller.rb @@ -12,13 +12,16 @@ class AccessTokensController < BaseController activate_menu :account, :personal, :tokens before_action :authorize_access_tokens before_action :disable_client_cache - before_action :load_presenter, only: %i[new create] - def new; end + def new + @presenter = AccessTokensNewPresenter.new(current_account) + end def create create! do |success, _failure| success.html do + @presenter = AccessTokensNewPresenter.new(current_account) + flash[:token] = @access_token.id flash[:notice] = 'Access Token was successfully created.' redirect_to(collection_url) @@ -42,10 +45,6 @@ def update private - def load_presenter - @presenter = AccessTokensNewPresenter.new(current_account) - end - def authorize_access_tokens authorize! :manage, :access_tokens, current_user end From abdbd8973f1dcb53a7f87ba3f26ba5da9e0a1999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 13:32:44 +0100 Subject: [PATCH 28/42] Apply suggestion in styling --- .../AccessTokens/components/ExpirationDatePicker.scss | 3 ++- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 9 ++++++++- .../provider/admin/user/access_tokens/new.html.slim | 3 +-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss index 7ca7c4307f..c57c442596 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss @@ -1,6 +1,7 @@ @import '~@patternfly/patternfly/patternfly-addons.css'; -.pf-c-calendar-month, .pf-c-form-control-expiration { +.pf-c-calendar-month, +.pf-c-form-control-expiration { width: 50%; } diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 1bc1d38b8f..e0fed6bd3f 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -1,5 +1,12 @@ import { useState, useMemo } from 'react' -import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption, Popover } from '@patternfly/react-core' +import { + Alert, + CalendarMonth, + FormGroup, + FormSelect, + FormSelectOption, + Popover +} from '@patternfly/react-core' import ExclamationTriangleIcon from '@patternfly/react-icons/dist/js/icons/exclamation-triangle-icon' import { createReactWrapper } from 'utilities/createReactWrapper' diff --git a/app/views/provider/admin/user/access_tokens/new.html.slim b/app/views/provider/admin/user/access_tokens/new.html.slim index d69e35a522..f0c3426507 100644 --- a/app/views/provider/admin/user/access_tokens/new.html.slim +++ b/app/views/provider/admin/user/access_tokens/new.html.slim @@ -1,8 +1,7 @@ - content_for :page_header_title, t('.page_header_title') - content_for :javascripts do - = javascript_packs_with_chunks_tag 'pf_form' - = javascript_packs_with_chunks_tag 'expiration_date_picker' + = javascript_packs_with_chunks_tag 'pf_form', 'expiration_date_picker' div class="pf-c-card" div class="pf-c-card__body" From a1511b5caaff52f4638183efd052ae9f8cf2c513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 13:33:03 +0100 Subject: [PATCH 29/42] Fix wrong comment --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index e0fed6bd3f..f1d8fbc3bf 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -18,7 +18,7 @@ import './ExpirationDatePicker.scss' interface ExpirationItem { id: string; label: string; - period: number; // In seconds + period: number; // In days } const collection: ExpirationItem[] = [ From 5ada2fa500a9868bca1a7783bffb5c1d55d2c3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 13:33:20 +0100 Subject: [PATCH 30/42] Presenter: Remove unused instance var --- .../provider/admin/user/access_tokens_new_presenter.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/presenters/provider/admin/user/access_tokens_new_presenter.rb b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb index ac85c33f5f..b61ab1cabd 100644 --- a/app/presenters/provider/admin/user/access_tokens_new_presenter.rb +++ b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb @@ -3,7 +3,6 @@ class Provider::Admin::User::AccessTokensNewPresenter def initialize(provider) - @provider = provider @timezone = ActiveSupport::TimeZone.new(provider.timezone) end From cca221d51a8f4505d80939e2cb0ea9c4a48e64ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 13:51:01 +0100 Subject: [PATCH 31/42] Move some constants out of the component --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index f1d8fbc3bf..d3ac16d11b 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -30,6 +30,8 @@ const collection: ExpirationItem[] = [ { id: 'no-exp', label: 'No expiration', period: 0 } ] +const today: Date = new Date() +const tomorrow: Date = new Date(today) const dayMs = 60 * 60 * 24 * 1000 interface Props { @@ -39,8 +41,6 @@ interface Props { } const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { - const today: Date = new Date() - const tomorrow: Date = new Date(today) tomorrow.setDate(today.getDate() + 1) const [dropdownSelectedItem, setDropdownSelectedItem] = useState(collection[0]) const [calendarPickedDate, setCalendarPickedDate] = useState(tomorrow) From 4acb540acc7dbe4a110625d6a73d4e8f174a478b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 14:09:55 +0100 Subject: [PATCH 32/42] Use locals in partials And not instance variables --- .../provider/admin/user/access_tokens/_form.html.slim | 8 ++++---- .../provider/admin/user/access_tokens/edit.html.slim | 2 +- app/views/provider/admin/user/access_tokens/new.html.slim | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index 8a32a15976..b05a7de101 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -2,19 +2,19 @@ input_html: { autofocus: true } = form.input :scopes, as: :patternfly_check_boxes, - collection: @access_token.available_scopes.to_collection_for_check_boxes + collection: access_token.available_scopes.to_collection_for_check_boxes = form.input :permission, as: :patternfly_select, - collection: @access_token.available_permissions, + collection: access_token.available_permissions, include_blank: false -- if @access_token.persisted? +- if access_token.persisted? .pf-c-form__group .pf-c-form__group-label label.pf-c-form__label span.pf-c-form__label-text = t('access_token_options.expires_at') .pf-c-form__group-control - = @access_token.expires_at.present? ? l(@access_token.expires_at) : t('access_token_options.no_expiration') + = access_token.expires_at.present? ? l(access_token.expires_at) : t('access_token_options.no_expiration') - else div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('access_token_options.expires_in'), tzOffset: @presenter.provider_timezone_offset }.to_json diff --git a/app/views/provider/admin/user/access_tokens/edit.html.slim b/app/views/provider/admin/user/access_tokens/edit.html.slim index 293160ed0f..165f3cf402 100644 --- a/app/views/provider/admin/user/access_tokens/edit.html.slim +++ b/app/views/provider/admin/user/access_tokens/edit.html.slim @@ -8,7 +8,7 @@ div class="pf-c-card" = semantic_form_for @access_token, builder: Fields::PatternflyFormBuilder, url: [:provider, :admin, :user, @access_token], html: { class: 'pf-c-form pf-m-limit-width' } do |f| - = render 'form', form: f + = render partial: 'form', locals: { form: f, access_token: @access_token } = f.actions do = f.commit_button t('.submit_button_label') diff --git a/app/views/provider/admin/user/access_tokens/new.html.slim b/app/views/provider/admin/user/access_tokens/new.html.slim index f0c3426507..2807750b0b 100644 --- a/app/views/provider/admin/user/access_tokens/new.html.slim +++ b/app/views/provider/admin/user/access_tokens/new.html.slim @@ -8,7 +8,7 @@ div class="pf-c-card" = semantic_form_for @access_token, builder: Fields::PatternflyFormBuilder, url: [:provider, :admin, :user, @access_token], html: { class: 'pf-c-form pf-m-limit-width' } do |f| - = render 'form', form: f + = render partial: 'form', locals: { form: f, access_token: @access_token } = f.actions do = f.commit_button t('.submit_button_label') From 96c709717cee4f6479b6aaf04d71c3f1385dd284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Thu, 9 Jan 2025 14:23:50 +0100 Subject: [PATCH 33/42] Move date picker props to presenter --- .../provider/admin/user/access_tokens_new_presenter.rb | 8 ++++++++ .../provider/admin/user/access_tokens/_form.html.slim | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/presenters/provider/admin/user/access_tokens_new_presenter.rb b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb index b61ab1cabd..e40021fde8 100644 --- a/app/presenters/provider/admin/user/access_tokens_new_presenter.rb +++ b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb @@ -9,4 +9,12 @@ def initialize(provider) def provider_timezone_offset @timezone.utc_offset end + + def date_picker_props + { + id: 'access_token[expires_at]', + label: I18n.t('access_token_options.expires_in'), + tzOffset: provider_timezone_offset + } + end end diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index b05a7de101..5a97650019 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -17,4 +17,4 @@ .pf-c-form__group-control = access_token.expires_at.present? ? l(access_token.expires_at) : t('access_token_options.no_expiration') - else - div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('access_token_options.expires_in'), tzOffset: @presenter.provider_timezone_offset }.to_json + div id='expiration-date-picker-container' data-props=@presenter.date_picker_props.to_json From 542518381e121981cafb423b7e2ec10919157a7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Fri, 10 Jan 2025 13:44:50 +0100 Subject: [PATCH 34/42] Props spreading --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index d3ac16d11b..3dcb6b5677 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -32,6 +32,7 @@ const collection: ExpirationItem[] = [ const today: Date = new Date() const tomorrow: Date = new Date(today) +tomorrow.setDate(today.getDate() + 1) const dayMs = 60 * 60 * 24 * 1000 interface Props { @@ -41,7 +42,6 @@ interface Props { } const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { - tomorrow.setDate(today.getDate() + 1) const [dropdownSelectedItem, setDropdownSelectedItem] = useState(collection[0]) const [calendarPickedDate, setCalendarPickedDate] = useState(tomorrow) const fieldName = `human_${id}` @@ -177,7 +177,8 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) ) } -const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(, containerId) } +// eslint-disable-next-line react/jsx-props-no-spreading +const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(, containerId) } export type { ExpirationItem, Props } export { ExpirationDatePicker, ExpirationDatePickerWrapper } From 4d8a47c74c14a0d858782b8b78b816ef13691064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Fri, 10 Jan 2025 14:36:11 +0100 Subject: [PATCH 35/42] Get rid of useMemo() --- .../components/ExpirationDatePicker.tsx | 144 +++++++++--------- 1 file changed, 73 insertions(+), 71 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 3dcb6b5677..b04ae488e8 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo } from 'react' +import { useState } from 'react' import { Alert, CalendarMonth, @@ -35,93 +35,95 @@ const tomorrow: Date = new Date(today) tomorrow.setDate(today.getDate() + 1) const dayMs = 60 * 60 * 24 * 1000 -interface Props { - id: string; - label: string | null; - tzOffset?: number; -} - -const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { - const [dropdownSelectedItem, setDropdownSelectedItem] = useState(collection[0]) - const [calendarPickedDate, setCalendarPickedDate] = useState(tomorrow) - const fieldName = `human_${id}` - const fieldLabel = label ?? 'Expires in' - - const dropdownDate = useMemo(() => { - if (dropdownSelectedItem.period === 0) return null - - return new Date(today.getTime() + dropdownSelectedItem.period * dayMs) - }, [dropdownSelectedItem]) +const computeDropdownDate = (dropdownSelectedItem: ExpirationItem) => { + if (dropdownSelectedItem.period === 0) return null - const selectedDate = useMemo(() => { - let value = null - - if (dropdownDate) { - value = dropdownDate - } else if (dropdownSelectedItem.id === 'custom' ) { - value = calendarPickedDate - } + return new Date(today.getTime() + dropdownSelectedItem.period * dayMs) +} - return value - }, [dropdownDate, dropdownSelectedItem, calendarPickedDate]) +const computeSelectedDate = (dropdownDate: Date | null, dropdownSelectedItem: ExpirationItem, calendarPickedDate: Date) => { + let value = null - const formattedDateValue = useMemo(() => { - if (!selectedDate) return + if (dropdownDate) { + value = dropdownDate + } else if (dropdownSelectedItem.id === 'custom' ) { + value = calendarPickedDate + } - const formatter = Intl.DateTimeFormat('en-US', { - month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false - }) + return value +} - return formatter.format(selectedDate) - }, [selectedDate]) +const computeFormattedDateValue = (selectedDate: Date | null) => { + if (!selectedDate) return - const fieldHint = useMemo(() => { - if (!formattedDateValue) return + const formatter = Intl.DateTimeFormat('en-US', { + month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false + }) - return `The token will expire on ${formattedDateValue}` - }, [formattedDateValue]) + return formatter.format(selectedDate) +} - const inputDateValue = useMemo(() => { - if (!selectedDate) return '' +const computeFieldHint = (formattedDateValue: string | undefined) => { + if (!formattedDateValue) return - return selectedDate.toISOString() - }, [selectedDate]) + return `The token will expire on ${formattedDateValue}` +} - const tzMismatch = useMemo(() => { - if (tzOffset === undefined) return +const computeTzMismatch = (tzOffset: number | undefined) => { + if (tzOffset === undefined) return - // Timezone offset in the same format as ActiveSupport - const jsTzOffset = new Date().getTimezoneOffset() * -60 + // Timezone offset in the same format as ActiveSupport + const jsTzOffset = new Date().getTimezoneOffset() * -60 - return jsTzOffset !== tzOffset - }, [tzOffset]) + return jsTzOffset !== tzOffset +} - const labelIcon = useMemo(() => { - if (!tzMismatch) return +const computeLabelIcon = (tzMismatch: boolean | undefined) => { + if (!tzMismatch) return - return ( - + return ( + Your local time zone differs from the provider default. The token will expire at the time you selected in your local time zone. -

- )} - headerContent={( - Time zone mismatch - )} +

+ )} + headerContent={( + Time zone mismatch + )} + > + - - ) - }, [tzMismatch]) + + + + ) +} + +interface Props { + id: string; + label: string | null; + tzOffset?: number; +} + +const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { + const [dropdownSelectedItem, setDropdownSelectedItem] = useState(collection[0]) + const [calendarPickedDate, setCalendarPickedDate] = useState(tomorrow) + + const dropdownDate = computeDropdownDate(dropdownSelectedItem) + const selectedDate = computeSelectedDate(dropdownDate, dropdownSelectedItem, calendarPickedDate) + const formattedDateValue = computeFormattedDateValue(selectedDate) + const fieldHint = computeFieldHint(formattedDateValue) + const tzMismatch = computeTzMismatch(tzOffset) + const labelIcon = computeLabelIcon(tzMismatch) + const inputDateValue = selectedDate ? selectedDate.toISOString() : '' + const fieldName = `human_${id}` + const fieldLabel = label ?? 'Expires in' const handleOnChange = (_value: string, event: FormEvent) => { const value = (event.target as HTMLSelectElement).value From f26aced542473127cc46b16a15138e373c4b985c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Fri, 10 Jan 2025 14:56:22 +0100 Subject: [PATCH 36/42] Props: make tzOffset mandatory Must be set from the server --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index b04ae488e8..2b9f605914 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -69,16 +69,14 @@ const computeFieldHint = (formattedDateValue: string | undefined) => { return `The token will expire on ${formattedDateValue}` } -const computeTzMismatch = (tzOffset: number | undefined) => { - if (tzOffset === undefined) return - +const computeTzMismatch = (tzOffset: number) => { // Timezone offset in the same format as ActiveSupport const jsTzOffset = new Date().getTimezoneOffset() * -60 return jsTzOffset !== tzOffset } -const computeLabelIcon = (tzMismatch: boolean | undefined) => { +const computeLabelIcon = (tzMismatch: boolean) => { if (!tzMismatch) return return ( @@ -108,7 +106,7 @@ const computeLabelIcon = (tzMismatch: boolean | undefined) => { interface Props { id: string; label: string | null; - tzOffset?: number; + tzOffset: number; } const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { From 3101fe99583910af810f5ae37630d078fcd9cc3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Fri, 10 Jan 2025 14:57:36 +0100 Subject: [PATCH 37/42] formattedDateValue can't be undefined But can be empty when there's no expiration date --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 2b9f605914..0ff2492e91 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -54,7 +54,7 @@ const computeSelectedDate = (dropdownDate: Date | null, dropdownSelectedItem: Ex } const computeFormattedDateValue = (selectedDate: Date | null) => { - if (!selectedDate) return + if (!selectedDate) return '' const formatter = Intl.DateTimeFormat('en-US', { month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false @@ -63,7 +63,7 @@ const computeFormattedDateValue = (selectedDate: Date | null) => { return formatter.format(selectedDate) } -const computeFieldHint = (formattedDateValue: string | undefined) => { +const computeFieldHint = (formattedDateValue: string) => { if (!formattedDateValue) return return `The token will expire on ${formattedDateValue}` From 231b1f022d238413e0cc6ef90fce20584cdb0619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 13 Jan 2025 10:01:19 +0100 Subject: [PATCH 38/42] Simplify `computeSelectedDate` --- .../AccessTokens/components/ExpirationDatePicker.tsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 0ff2492e91..03d6483c94 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -42,15 +42,9 @@ const computeDropdownDate = (dropdownSelectedItem: ExpirationItem) => { } const computeSelectedDate = (dropdownDate: Date | null, dropdownSelectedItem: ExpirationItem, calendarPickedDate: Date) => { - let value = null + if (dropdownDate) return dropdownDate - if (dropdownDate) { - value = dropdownDate - } else if (dropdownSelectedItem.id === 'custom' ) { - value = calendarPickedDate - } - - return value + return dropdownSelectedItem.id === 'custom' ? calendarPickedDate : null } const computeFormattedDateValue = (selectedDate: Date | null) => { From c2057e9c88c296831382a14df6462346ec9e71f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 13 Jan 2025 11:10:25 +0100 Subject: [PATCH 39/42] ;ake date picker props a local in form template --- app/views/provider/admin/user/access_tokens/_form.html.slim | 2 +- app/views/provider/admin/user/access_tokens/new.html.slim | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index 5a97650019..503e270933 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -17,4 +17,4 @@ .pf-c-form__group-control = access_token.expires_at.present? ? l(access_token.expires_at) : t('access_token_options.no_expiration') - else - div id='expiration-date-picker-container' data-props=@presenter.date_picker_props.to_json + div id='expiration-date-picker-container' data-props=date_picker_props.to_json diff --git a/app/views/provider/admin/user/access_tokens/new.html.slim b/app/views/provider/admin/user/access_tokens/new.html.slim index 2807750b0b..c1ea81083e 100644 --- a/app/views/provider/admin/user/access_tokens/new.html.slim +++ b/app/views/provider/admin/user/access_tokens/new.html.slim @@ -8,7 +8,7 @@ div class="pf-c-card" = semantic_form_for @access_token, builder: Fields::PatternflyFormBuilder, url: [:provider, :admin, :user, @access_token], html: { class: 'pf-c-form pf-m-limit-width' } do |f| - = render partial: 'form', locals: { form: f, access_token: @access_token } + = render partial: 'form', locals: { form: f, access_token: @access_token, date_picker_props: @presenter.date_picker_props } = f.actions do = f.commit_button t('.submit_button_label') From d339b14b088e85e0efd50d248c0c4c438f91fc33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 13 Jan 2025 11:11:52 +0100 Subject: [PATCH 40/42] Properly init presenter in `#create` action --- .../provider/admin/user/access_tokens_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/provider/admin/user/access_tokens_controller.rb b/app/controllers/provider/admin/user/access_tokens_controller.rb index 11e6d083ff..188d4506ad 100644 --- a/app/controllers/provider/admin/user/access_tokens_controller.rb +++ b/app/controllers/provider/admin/user/access_tokens_controller.rb @@ -18,10 +18,9 @@ def new end def create + @presenter = AccessTokensNewPresenter.new(current_account) create! do |success, _failure| success.html do - @presenter = AccessTokensNewPresenter.new(current_account) - flash[:token] = @access_token.id flash[:notice] = 'Access Token was successfully created.' redirect_to(collection_url) From 3047613835b0cb22f78bfca8734699c4ae06e6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Wed, 22 Jan 2025 12:01:58 +0100 Subject: [PATCH 41/42] Simplify callback --- .../src/AccessTokens/components/ExpirationDatePicker.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx index 03d6483c94..2c7dba9f30 100644 --- a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -11,7 +11,7 @@ import ExclamationTriangleIcon from '@patternfly/react-icons/dist/js/icons/excla import { createReactWrapper } from 'utilities/createReactWrapper' -import type { FunctionComponent, FormEvent } from 'react' +import type { FunctionComponent } from 'react' import './ExpirationDatePicker.scss' @@ -117,8 +117,7 @@ const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) const fieldName = `human_${id}` const fieldLabel = label ?? 'Expires in' - const handleOnChange = (_value: string, event: FormEvent) => { - const value = (event.target as HTMLSelectElement).value + const handleOnChange = (value: string) => { const selected = collection.find(i => i.id === value) ?? null if (selected === null) return From 7fd6dae0a67d63abb2099634a9020bc5d9b2724e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Wed, 22 Jan 2025 13:26:17 +0100 Subject: [PATCH 42/42] Update tests --- .../AccessTokens/components/ExpirationDatePicker.spec.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx index 8aed14c575..91db3d92bb 100644 --- a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -16,7 +16,11 @@ const defaultProps: Props = { const mountWrapper = (props: Partial = {}) => mount() const selectItem = (wrapper: ReactWrapper>, item: ExpirationItem) => { - wrapper.find('select.pf-c-form-control-expiration').simulate('change', { target: { value: item.id.toString() } }) + wrapper.find('select.pf-c-form-control-expiration option').forEach((op) => { + const elem: HTMLOptionElement = op.getDOMNode() + elem.selected = elem.value === item.id // Mark option as selected if it's value matches the given one + }) + wrapper.find('select.pf-c-form-control-expiration').simulate('change') } const pickDate = (wrapper: ReactWrapper>) => {