tiagobento commented on code in PR #3119:
URL:
https://github.com/apache/incubator-kie-tools/pull/3119#discussion_r2077582954
##########
packages/online-editor/src/authSessions/AuthSessionSelect.tsx:
##########
@@ -74,6 +74,7 @@ export function AuthSessionSelect(props: {
filter: AuthSessionSelectFilter;
menuAppendTo?: SelectProps["menuAppendTo"];
showOnlyThisAuthProviderGroupWhenConnectingToNewAccount: AuthProviderGroup |
undefined;
+ hideConnectToAccountButton?: boolean;
Review Comment:
Why would we want not to show the "connect to account" button?
##########
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 || !authSessionId) {
+ console.error("Missing required parameters to apply accelerator");
+ return;
+ }
Review Comment:
Not sure this is correct. Not having an authSessionId is fine, as
Accelerators may be publicly available.
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorModal.tsx:
##########
@@ -62,9 +133,39 @@ 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}>
+ <label htmlFor="auth-session-select">Select Authentication
Session:</label>
+ <AuthSessionSelect
+ menuAppendTo={document.body}
+ title={"Select authentication session"}
+ authSessionId={selectedAuthSessionId}
+ setAuthSessionId={setSelectedAuthSessionId}
+ isPlain={false}
+ filter={gitAuthSessionSelectFilter()}
+
showOnlyThisAuthProviderGroupWhenConnectingToNewAccount={undefined}
+ hideConnectToAccountButton={true}
+ />
+ </GridItem>
+
+ {selectedAuthSession && (
+ <GridItem span={12}>
+ {isCompatibleAuthSession ? (
+ <Alert variant="info" isInline title="Authentication Status">
+ Using {selectedAuthProvider?.domain} credentials for
{(selectedAuthSession as GitAuthSession).login}
+ </Alert>
+ ) : (
+ <Alert variant="danger" isInline title="Authentication
Status">
+ Selected auth session is not compatible with repository
domain: {urlDomain}
+ </Alert>
Review Comment:
Same here... mentioning "auth session" in the UI.... Probably should stick
to something like:
`Selected account is not compatible with '{urlDomain}', where [Accelerator
name] Accelerator is hosted.`
And maybe we should not have more UI elements for when everything is working
fine? I mean, IMHO the "Apply" button being enabled is enough for people to
move forward with applying their Accelerators... WDYT?
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorIndicator.tsx:
##########
@@ -64,6 +68,10 @@ export function AcceleratorIndicator(props: Props) {
isOpen={isAcceleratorDetailsModalOpen}
onClose={() => setAcceleratorDetailsModalOpen(false)}
accelerator={currentAccelerator}
+ isApplying={true}
Review Comment:
Not sure `true` is correct here? Since this is only to display the
Accelerator's information...
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorModal.tsx:
##########
@@ -26,18 +26,84 @@ import { Modal, ModalVariant } from
"@patternfly/react-core/dist/js/components/M
import { Button } from "@patternfly/react-core/dist/js/components/Button";
import { AcceleratorIcon } from "./AcceleratorIcon";
import { Divider } from "@patternfly/react-core/dist/js/components/Divider";
+import { AuthSession, AuthSessionStatus, GitAuthSession, isGitAuthSession }
from "../../../authSessions/AuthSessionApi";
+import { Alert } from "@patternfly/react-core/dist/js/components/Alert";
+import {
+ getCompatibleAuthSessionWithUrlDomain,
+ gitAuthSessionSelectFilter,
+ isAuthSessionCompatibleWithUrlDomain,
+} from "../../../authSessions/CompatibleAuthSessions";
+import { AuthProvider } from "../../../authProviders/AuthProvidersApi";
+import { AuthSessionSelect } from "../../../authSessions/AuthSessionSelect";
type Props = {
accelerator: AcceleratorAppliedConfig;
isOpen: boolean;
- onClose: () => any;
+ onClose: () => void;
isApplying?: boolean;
- onApplyAccelerator?: () => void;
+ onApplyAccelerator?: (authSessionId?: string) => void;
+ authProviders: AuthProvider[];
+ authSessions: Map<string, AuthSession>;
+ authSessionStatus: Map<string, AuthSessionStatus>;
};
+function getDomainFromUrl(url: string | undefined): string | undefined {
+ if (!url) return undefined;
+ try {
+ return new URL(url).hostname;
+ } catch {
+ return undefined;
+ }
+}
+
export function AcceleratorModal(props: Props) {
const { i18n } = useOnlineI18n();
+ const urlDomain = useMemo(
+ () => getDomainFromUrl(props.accelerator.gitRepositoryUrl),
+ [props.accelerator.gitRepositoryUrl]
+ );
+
+ const [selectedAuthSessionId, setSelectedAuthSessionId] = useState<string |
undefined>(undefined);
+
+ useEffect(() => {
+ const { compatible } = getCompatibleAuthSessionWithUrlDomain({
+ authProviders: props.authProviders,
+ authSessions: props.authSessions,
+ authSessionStatus: props.authSessionStatus,
+ urlDomain,
+ });
+
+ setSelectedAuthSessionId(compatible.length > 0 ? compatible[0].id : "");
+ }, [props.authProviders, props.authSessions, props.authSessionStatus,
urlDomain]);
+
+ const selectedAuthSession = useMemo(() => {
+ return selectedAuthSessionId ?
props.authSessions.get(selectedAuthSessionId) : undefined;
+ }, [selectedAuthSessionId, props.authSessions]);
+
+ const selectedAuthProvider = useMemo(() => {
+ if (selectedAuthSession && isGitAuthSession(selectedAuthSession)) {
+ return props.authProviders.find((p) => p.id ===
selectedAuthSession.authProviderId);
+ }
+ return undefined;
+ }, [selectedAuthSession, props.authProviders]);
+
+ const selectedStatus = useMemo(() => {
+ return selectedAuthSession ?
props.authSessionStatus.get(selectedAuthSession.id) : undefined;
+ }, [selectedAuthSession, props.authSessionStatus]);
Review Comment:
I have the impression this is duplicated code, would you be able to verify
please?
##########
packages/online-editor/src/editor/Toolbar/Accelerators/AcceleratorModal.tsx:
##########
@@ -62,9 +133,39 @@ 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}>
+ <label htmlFor="auth-session-select">Select Authentication
Session:</label>
Review Comment:
I don't think we have such a label in other places for the
`AuthSessionSelect`. Don't want to make the UI inconsistent for this case...
--
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]
