Skip to content
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

Fixes #36576 - Add Activation Key details top bar #10637

Merged
merged 8 commits into from
Aug 3, 2023

Conversation

trev-allison03
Copy link
Contributor

What are the changes introduced in this pull request?

  • add top bar
  • display activation key details
  • display host limit
  • working edit button
  • working delete button
  • link to old page
  • breadcrumb to previous page

Considerations taken when implementing this change?

-used window.location.replace instead of because redirect to an angularJS page did not work

What are the testing steps for this pull request?

  • Test that details are displayed correctly, including host count and limit
  • test that editing details works
  • test that delete works and redirects
  • test that breadcrumb works
  • test that link to old page works

@theforeman-bot
Copy link

Issues: #36576

@theforeman-bot
Copy link

Can one of the admins verify this patch?

Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Haven't tested but leaving few nitpicks..
PS: Audit the strings in the components for translations: __('Will be translated').

@jeremylenz
Copy link
Member

[test katello]

…strings, fix host limit issue with edit button, make akId required, import useState in delete menu
@trev-allison03
Copy link
Contributor Author

@sjha4 updated. Thanks

@jeremylenz
Copy link
Member

[test katello]

@sjha4
Copy link
Member

sjha4 commented Jul 14, 2023

When editing max_hosts in edit modal, I am unable to reduce the value. I am seeing this.

EditModal.js:83 Uncaught TypeError: maxHostsValue is not a function
    at onMinus (EditModal.js:83:5)
    at onClick (NumberInput.js:42:56)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237:1)
    at invokeGuardedCallback (react-dom.development.js:292:1)
    at invokeGuardedCallbackAndCatchFirstError (react-dom.development.js:306:1)
    at executeDispatch (react-dom.development.js:389:1)
    at executeDispatchesInOrder (react-dom.development.js:414:1)
    at executeDispatchesAndRelease (react-dom.development.js:3278:1)
    at executeDispatchesAndReleaseTopLevel (react-dom.development.js:3287:1)

@sjha4
Copy link
Member

sjha4 commented Jul 14, 2023

Also seeing some console errors as soon as I open edit modal:

Warning: `value` prop on `input` should not be null. Consider using an empty string to clear the component or `undefined` for uncontrolled components.
    in input (created by TextInputBase)
    in TextInputBase (created by TextInput)
    in TextInput (created by NumberInput)
    in div (created by InputGroup)
    in InputGroup (created by NumberInput)
    in div (created by NumberInput)
    in NumberInput (created by EditModal)
    in div (created by StackItem)
    in StackItem (created by EditModal)
    in div (created by Stack)
    in Stack (created by EditModal)
    in div (created by GenerateId)
    in div (created by GenerateId)
    in GenerateId (created by FormGroup)
    in FormGroup (created by EditModal)
    in form (created by FormBase)
    in FormBase (created by Form)
    in Form (created by EditModal)
    in div (created by ModalBoxBody)
    in ModalBoxBody (created by ModalContent)
    in div (created by ModalBox)
    in ModalBox (created by ModalContent)
    in div (created by FocusTrap)
    in FocusTrap (created by ForwardRef)
    in ForwardRef (created by ModalContent)
    in div (created by Backdrop)
    in Backdrop (created by ModalContent)
    in ModalContent (created by Modal)
    in Modal (created by EditModal)
    in EditModal (created by ActivationKeyDetails)
    in div (created by SplitItem)
    in SplitItem (created by ActivationKeyDetails)
    in div (created by Split)
    in Split (created by ActivationKeyDetails)
    in div (created by FlexItem)
    in FlexItem (created by ActivationKeyDetails)
    in div (created by Flex)
    in Flex (created by ActivationKeyDetails)
    in div (created by GridItem)
    in GridItem (created by ActivationKeyDetails)
    in div (created by Grid)
    in Grid (created by ActivationKeyDetails)
    in div (created by PanelBase)
    in PanelBase (created by Panel)
    in Panel (created by ActivationKeyDetails)
    in div (created by ActivationKeyDetails)
    in ActivationKeyDetails (created by Headers)
    in Headers (created by CheckOrg)
    in CheckOrg (created by ConnectFunction)
    in ConnectFunction (created by Context.Consumer)
    in Route (created by _default)
    in div (created by _default)
    in _default (created by Application)
    in Router (created by BrowserRouter)
    in BrowserRouter (created by Application)
    in Application (created by ConnectFunction)
    in ConnectFunction (created by I18nProviderWrapper(Connect(Application)))
    in IntlProvider (created by I18nProviderWrapper(Connect(Application)))
    in I18nProviderWrapper(Connect(Application)) (created by StoreProvider(I18nProviderWrapper(Connect(Application))))
    in Provider (created by StoreProvider(I18nProviderWrapper(Connect(Application))))
    in StoreProvider(I18nProviderWrapper(Connect(Application))) (created by DataProvider(StoreProvider(I18nProviderWrapper(Connect(Application)))))
    in DataProvider(StoreProvider(I18nProviderWrapper(Connect(Application))))
overrideMethod @ console.js:213
printWarning @ react-dom.development.js:88
error @ react-dom.development.js:60
validateProperties$1 @ react-dom.development.js:5438
validatePropertiesInDevelopment @ react-dom.development.js:5661
setInitialProperties @ react-dom.development.js:5941
finalizeInitialChildren @ react-dom.development.js:7499
completeWork @ react-dom.development.js:18978
completeUnitOfWork @ react-dom.development.js:22192
performUnitOfWork @ react-dom.development.js:22165
workLoopSync @ react-dom.development.js:22130
performSyncWorkOnRoot @ react-dom.development.js:21756
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
discreteUpdates$1 @ react-dom.development.js:21893
discreteUpdates @ react-dom.development.js:806
dispatchDiscreteEvent @ react-dom.development.js:4168
Show 1 more frame
react-dom.development.js:88 Warning: TextArea contains a textarea with both value and defaultValue props. Textarea elements must be either controlled or uncontrolled (specify either the value prop, or the defaultValue prop, but not both). Decide between using a controlled or uncontrolled textarea and remove one of these props. More info: https://fb.me/react-controlled-components
    in textarea (created by TextArea)
    in TextArea (created by TextArea)
    in TextArea (created by EditModal)
    in div (created by GenerateId)
    in div (created by GenerateId)
    in GenerateId (created by FormGroup)
    in FormGroup (created by EditModal)
    in form (created by FormBase)
    in FormBase (created by Form)
    in Form (created by EditModal)
    in div (created by ModalBoxBody)
    in ModalBoxBody (created by ModalContent)
    in div (created by ModalBox)
    in ModalBox (created by ModalContent)
    in div (created by FocusTrap)
    in FocusTrap (created by ForwardRef)
    in ForwardRef (created by ModalContent)
    in div (created by Backdrop)
    in Backdrop (created by ModalContent)
    in ModalContent (created by Modal)
    in Modal (created by EditModal)
    in EditModal (created by ActivationKeyDetails)
    in div (created by SplitItem)
    in SplitItem (created by ActivationKeyDetails)
    in div (created by Split)
    in Split (created by ActivationKeyDetails)
    in div (created by FlexItem)
    in FlexItem (created by ActivationKeyDetails)
    in div (created by Flex)
    in Flex (created by ActivationKeyDetails)
    in div (created by GridItem)
    in GridItem (created by ActivationKeyDetails)
    in div (created by Grid)
    in Grid (created by ActivationKeyDetails)
    in div (created by PanelBase)
    in PanelBase (created by Panel)
    in Panel (created by ActivationKeyDetails)
    in div (created by ActivationKeyDetails)
    in ActivationKeyDetails (created by Headers)
    in Headers (created by CheckOrg)
    in CheckOrg (created by ConnectFunction)
    in ConnectFunction (created by Context.Consumer)
    in Route (created by _default)
    in div (created by _default)
    in _default (created by Application)
    in Router (created by BrowserRouter)
    in BrowserRouter (created by Application)
    in Application (created by ConnectFunction)
    in ConnectFunction (created by I18nProviderWrapper(Connect(Application)))
    in IntlProvider (created by I18nProviderWrapper(Connect(Application)))
    in I18nProviderWrapper(Connect(Application)) (created by StoreProvider(I18nProviderWrapper(Connect(Application))))
    in Provider (created by StoreProvider(I18nProviderWrapper(Connect(Application))))
    in StoreProvider(I18nProviderWrapper(Connect(Application))) (created by DataProvider(StoreProvider(I18nProviderWrapper(Connect(Application)))))
    in DataProvider(StoreProvider(I18nProviderWrapper(Connect(Application))))
overrideMethod @ console.js:213
printWarning @ react-dom.development.js:88
error @ react-dom.development.js:60
initWrapperState$2 @ react-dom.development.js:2364
setInitialProperties @ react-dom.development.js:6013
finalizeInitialChildren @ react-dom.development.js:7499
completeWork @ react-dom.development.js:18978
completeUnitOfWork @ react-dom.development.js:22192
performUnitOfWork @ react-dom.development.js:22165
workLoopSync @ react-dom.development.js:22130
performSyncWorkOnRoot @ react-dom.development.js:21756
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
discreteUpdates$1 @ react-dom.development.js:21893
discreteUpdates @ react-dom.development.js:806
dispatchDiscreteEvent @ react-dom.development.js:4168
Show 1 more frame

@sjha4
Copy link
Member

sjha4 commented Jul 14, 2023

Able to edit/delete AK. Able to navigate to new AK page and old page with new page action. The host_limit correctly displays on the UI and the breadcrumb redirects to the AK index page. ✅

The page looks good and works great other than the couple of issues above. Nice work!

@sjha4 sjha4 self-assigned this Jul 20, 2023
@sjha4
Copy link
Member

sjha4 commented Jul 26, 2023

React CI lint failures are related..

@sjha4
Copy link
Member

sjha4 commented Jul 26, 2023

[test katello]

@sjha4
Copy link
Member

sjha4 commented Aug 1, 2023

[test katello]

@sjha4
Copy link
Member

sjha4 commented Aug 1, 2023

Couple of notes:

  1. When opening an edit modal for AK with empty description, console complains about value prop being null for textarea. We might want to add a default of '' for that.
Warning: `value` prop on `textarea` should not be null. Consider using an empty string to clear the component or `undefined` for uncontrolled components.
    in textarea (created by TextArea)
    in TextArea (created by TextArea)
    in TextArea (created by EditModal)
    in div (created by GenerateId)
    in div (created by GenerateId)
    in GenerateId (created by FormGroup)
    in FormGroup (created by EditModal)
    in form (created by FormBase)
    in FormBase (created by Form)
    in Form (created by EditModal)
    in div (created by ModalBoxBody)
    in ModalBoxBody (created by ModalContent)
    in div (created by ModalBox)
    in ModalBox (created by ModalContent)
    in div (created by FocusTrap)
    in FocusTrap (created by ForwardRef)
    in ForwardRef (created by ModalContent)
    in div (created by Backdrop)
    in Backdrop (created by ModalContent)
    in ModalContent (created by Modal)
    in Modal (created by EditModal)
    in EditModal (created by ActivationKeyDetails)
    in div (created by SplitItem)
    in SplitItem (created by ActivationKeyDetails)
    in div (created by Split)
    in Split (created by ActivationKeyDetails)
    in div (created by FlexItem)
    in FlexItem (created by ActivationKeyDetails)
    in div (created by Flex)
    in Flex (created by ActivationKeyDetails)
    in div (created by GridItem)
    in GridItem (created by ActivationKeyDetails)
    in div (created by Grid)
    in Grid (created by ActivationKeyDetails)
    in div (created by PanelBase)
    in PanelBase (created by Panel)
    in Panel (created by ActivationKeyDetails)
    in div (created by ActivationKeyDetails)
    in ActivationKeyDetails (created by Headers)
    in Headers (created by CheckOrg)
    in CheckOrg (created by ConnectFunction)
    in ConnectFunction (created by Context.Consumer)
    in Route (created by _default)
    in div (created by _default)
    in _default (created by Application)
    in Router (created by BrowserRouter)
    in BrowserRouter (created by Application)
    in Application (created by ConnectFunction)
    in ConnectFunction (created by I18nProviderWrapper(Connect(Application)))
    in IntlProvider (created by I18nProviderWrapper(Connect(Application)))
    in I18nProviderWrapper(Connect(Application)) (created by StoreProvider(I18nProviderWrapper(Connect(Application))))
    in Provider (created by StoreProvider(I18nProviderWrapper(Connect(Application))))
    in StoreProvider(I18nProviderWrapper(Connect(Application))) (created by DataProvider(StoreProvider(I18nProviderWrapper(Connect(Application)))))
    in DataProvider(StoreProvider(I18nProviderWrapper(Connect(Application))))
  1. When the edit fails, (ex: trying to update name to an existing name), it closes the modal but leaves page in a weird state.
    Screenshot from 2023-08-01 11-02-20

@trev-allison03
Copy link
Contributor Author

@sjha4 updated

@sjha4
Copy link
Member

sjha4 commented Aug 3, 2023

[test katello]

@sjha4
Copy link
Member

sjha4 commented Aug 3, 2023

[test katello]

Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor nitpicks.. Looks ready to go otherwise.. 🎉

@sjha4
Copy link
Member

sjha4 commented Aug 3, 2023

[test katello]

@sjha4
Copy link
Member

sjha4 commented Aug 3, 2023

One thing I just noticed is AK create/update/delete etc are tasks and sometimes they can take longer than modal closing. I filed an issue to handle that with some sort of loading state or whatever UX suggests. https://projects.theforeman.org/issues/36635

Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. Nice work @trev-allison03 ! 🎉

@jeremylenz jeremylenz merged commit b4f5025 into Katello:master Aug 3, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants