-
Notifications
You must be signed in to change notification settings - Fork 12
settings: Add organization and user settings (PROJQUAY-4553) #171
base: main
Are you sure you want to change the base?
Conversation
jonathankingfc
commented
Jan 24, 2023
- This PR includes a few features that enhance the org/settings customization flows
- Users can now convert their account from individual to organization
- Users can update their e-mail and other similar metadata regarding their account
- Error handling has been added to the settings pages / clear back to defaults
- Added ability to generate CLI token for user account
- This PR includes a few features that enhance the org/settings customization flows - Users can now convert their account from individual to organization - Users can update their e-mail and other similar metadata regarding their account - Error handling has been added to the settings pages / clear back to defaults - Added ability to generate CLI token for user account
@@ -16,6 +16,8 @@ import { | |||
import {useLocation} from 'react-router-dom'; | |||
import {useCurrentUser} from 'src/hooks/UseCurrentUser'; | |||
import {useOrganization} from 'src/hooks/UseOrganization'; | |||
import {BillingInformation} from './BillingInformation'; | |||
import {CliConfiguration} from './CLIConfiguration'; | |||
|
|||
const GeneralSettings = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this variable defined in GeneralSettings.tsx and one of it doesn't seem to be referenced anywhere. Could you take a look?
View Invoices | ||
</Button> | ||
|
||
{isUserOrganization && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unable to see this additional settings field for a user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Left some minor comments. The Change plan slider seen in mocks doesn't seem to be implemented - not sure if it was decided to not have it? but just wanted to call out so that it doesn't get missed
</HelperText> | ||
<FormGroup | ||
isInline | ||
label="Admin Username" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: mocks show this to just Username
</FormGroup> | ||
<FormGroup | ||
isInline | ||
label="Admin Username" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
label would be password
<Tbody> | ||
{paginatedBulkItemsList.map((item, idx) => ( | ||
<Tr key={idx}> | ||
<Td>{item.name}</Td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mock shows the org names to be clickable
import {GenerateEncryptedPassword} from 'src/components/modals/GenerateEncryptedPasswordModal'; | ||
import {useState} from 'react'; | ||
|
||
export const CliConfiguration = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be called anywhere?