thiagoelg commented on code in PR #3119:
URL:
https://github.com/apache/incubator-kie-tools/pull/3119#discussion_r2078287624
##########
packages/online-editor/src/accelerators/AcceleratorsHooks.tsx:
##########
@@ -128,7 +128,14 @@ export function useAcceleratorsDispatch(workspace:
ActiveWorkspace) {
}, [workspace.descriptor.workspaceId, workspaces]);
const applyAcceleratorToWorkspace = useCallback(
- async (accelerator: AcceleratorConfig, currentFile: WorkspaceFile) => {
+ async (
+ accelerator: AcceleratorConfig,
Review Comment:
`authInfo` can be optional here, right? In the case that the Accelerator is
hosted in a public repository (and there's no AuthSession selected in the
dropdown)
##########
packages/online-editor/src/authSessions/AuthSessionSelect.tsx:
##########
@@ -151,7 +151,7 @@ export function AuthSessionSelect(props: {
className={props.isPlain ? "kie-tools--masthead-hoverable" : ""}
menuAppendTo={props.menuAppendTo ?? "parent"}
maxHeight={"400px"}
- style={{ minWidth: "400px" }}
+ style={{ minWidth: "300px", maxHeight: "300px", overflowY: "scroll" }}
Review Comment:
There's a `maxHeight={"400px"}` prop just above, but in the style it's set
to `300px`. What should we use?
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorModal.tsx:
##########
@@ -46,7 +108,12 @@ export function AcceleratorModal(props: Props) {
variant={ModalVariant.medium}
actions={
props.isApplying && [
- <Button key="apply" variant="primary"
onClick={props.onApplyAccelerator}>
+ <Button
+ key="apply"
+ variant="primary"
+ onClick={() => props.onApplyAccelerator?.(selectedAuthSessionId)}
+ isDisabled={!isCompatibleAuthSession}
Review Comment:
What happens when the Accelerator repository is public but there's no
compatible AuthSession? Users should be able to apply the accelerator.
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorsDropdown.tsx:
##########
@@ -105,6 +125,9 @@ export function AcceleratorsDropdown(props: Props) {
onApplyAccelerator={onApplyAccelerator}
accelerator={selectedAccelerator}
isApplying={true}
+ authSessions={authSessions}
+ authProviders={authProviders}
+ authSessionStatus={authSessionStatus}
Review Comment:
Same thing here, just use the accelerators inside of the AcceleratorModal
component.
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorsDropdown.tsx:
##########
@@ -58,13 +64,27 @@ export function AcceleratorsDropdown(props: Props) {
[setAcceleratorsDropdownOpen]
);
- const onApplyAccelerator = useCallback(() => {
- if (selectedAccelerator) {
- applyAcceleratorToWorkspace(selectedAccelerator, props.workspaceFile);
- }
- setSelectedAccelerator(undefined);
- setApplyModalOpen(false);
- }, [applyAcceleratorToWorkspace, props.workspaceFile, selectedAccelerator]);
+ const onApplyAccelerator = useCallback(
+ async (authSessionId?: string) => {
+ if (!selectedAccelerator) {
+ console.error("Missing required parameters to apply accelerator");
+ return;
+ }
+ const authSessionObj = authSessions.get(authSessionId || "") as
GitAuthSession;
+ try {
+ await applyAcceleratorToWorkspace(selectedAccelerator,
props.workspaceFile, {
+ username: authSessionObj?.login,
+ password: authSessionObj?.token,
+ });
Review Comment:
Since `authSessionId` is optional, when it's not provided can skip passing
the `authInfo` argument here.
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorModal.tsx:
##########
@@ -46,7 +108,12 @@ export function AcceleratorModal(props: Props) {
variant={ModalVariant.medium}
actions={
props.isApplying && [
- <Button key="apply" variant="primary"
onClick={props.onApplyAccelerator}>
+ <Button
+ key="apply"
+ variant="primary"
+ onClick={() => props.onApplyAccelerator?.(selectedAuthSessionId)}
+ isDisabled={!isCompatibleAuthSession}
Review Comment:
Also, maybe we should let users try to apply the Accelerator even without a
compatible AuthSession, we never know what crazy Git providers and
authentication methods they might be using... WDYT @tiagobento?
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorIndicator.tsx:
##########
@@ -64,6 +68,9 @@ export function AcceleratorIndicator(props: Props) {
isOpen={isAcceleratorDetailsModalOpen}
onClose={() => setAcceleratorDetailsModalOpen(false)}
accelerator={currentAccelerator}
+ authProviders={authProviders}
+ authSessions={authSessions}
+ authSessionStatus={authSessionStatus}
Review Comment:
Is is possible to use the hooks inside of the `AcceleratorModal` instead of
passing the values as props? Just a nitpick, it should work the same.
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorModal.tsx:
##########
@@ -62,9 +129,41 @@ export function AcceleratorModal(props: Props) {
</Title>
<Grid style={{ margin: "1rem 0" }} hasGutter>
{props.isApplying && (
- <GridItem span={12}>
- <i>{i18n.accelerators.acceleratorDescription}</i>
- </GridItem>
+ <>
+ <GridItem span={12}>
+ <i>{i18n.accelerators.acceleratorDescription}</i>
+ </GridItem>
+
+ <GridItem span={12}>
+ <AuthSessionSelect
+ title={`Select Git authentication for
'${props.accelerator.name}' Accelerator`}
+ authSessionId={selectedAuthSessionId}
+ setAuthSessionId={(newSelectedAuthSessionId) => {
+ setSelectedAuthSessionId(newSelectedAuthSessionId);
+ accountsDispatch({ kind: AccountsDispatchActionKind.CLOSE });
+ }}
+ isPlain={false}
+ position={SelectPosition.right}
+ filter={gitAuthSessionSelectFilter()}
+
showOnlyThisAuthProviderGroupWhenConnectingToNewAccount={AuthProviderGroup.GIT}
+ />
+ </GridItem>
+
+ {authSession && (
+ <GridItem span={12}>
+ {isCompatibleAuthSession ? (
+ <Alert variant="info" isInline title="Authentication Status">
+ Using {selectedAuthProvider?.domain} credentials for
{(authSession as GitAuthSession).login}
+ </Alert>
+ ) : (
+ <Alert variant="danger" isInline title="Authentication
Status">
+ Selected account is not compatible with {urlDomain} ,
where {props.accelerator.name} Accelerator is
+ hosted.
Review Comment:
```suggestion
Selected account is not compatible with {urlDomain},
where {props.accelerator.name} Accelerator is hosted.
```
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorsDropdown.tsx:
##########
@@ -58,13 +64,27 @@ export function AcceleratorsDropdown(props: Props) {
[setAcceleratorsDropdownOpen]
);
- const onApplyAccelerator = useCallback(() => {
- if (selectedAccelerator) {
- applyAcceleratorToWorkspace(selectedAccelerator, props.workspaceFile);
- }
- setSelectedAccelerator(undefined);
- setApplyModalOpen(false);
- }, [applyAcceleratorToWorkspace, props.workspaceFile, selectedAccelerator]);
+ const onApplyAccelerator = useCallback(
+ async (authSessionId?: string) => {
+ if (!selectedAccelerator) {
+ console.error("Missing required parameters to apply accelerator");
+ return;
+ }
+ const authSessionObj = authSessions.get(authSessionId || "") as
GitAuthSession;
Review Comment:
If for whatever reason the selected `authSessionId` is not a
`GitAuthSession` the code below will break. We have a `isGitAuthSession`
function to validate the type of an AuthSession, you could use that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
