You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "jomarko (via GitHub)" <gi...@apache.org> on 2023/12/08 13:28:53 UTC
[PR] kie-issues#129: Draft solution [incubator-kie-tools]
jomarko opened a new pull request, #2087:
URL: https://github.com/apache/incubator-kie-tools/pull/2087
Closes https://github.com/apache/incubator-kie-issues/issues/129
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#129: Import private repository into sandbox via chrome-extension [backend-part] [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on PR #2087:
URL: https://github.com/apache/incubator-kie-tools/pull/2087#issuecomment-1875411942
I have incorporated the review feedback and tried the following scenarios:
### Public repository
Importing a public repository using link `https://localhost:9001/#/import?url=https://github.com/apache/incubator-kie-kogito-examples/tree/stable` worked. See the attached screenshot.
### Private repository, token available but not compatible
Using link `https://localhost:9001/#/import?url=https://github.com/jomarko/dmn-sample-private/tree/PR` (where `dmn-sample-private` is my private repository) opened and advanced import modal
### Private repository, no token available
Using same link as above resulted into the same result as above == an advanced import modal was opened.
### Private repository, compatible token available
Using link `https://localhost:9001/#/import?url=https://github.com/jomarko/dmn-sample-private/tree/PR` Worked successfully without opening an advanced import modal.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#129: Import private repository into sandbox via chrome-extension [backend-part] [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #2087:
URL: https://github.com/apache/incubator-kie-tools/pull/2087#discussion_r1434370671
##########
packages/online-editor/src/importFromUrl/NewWorkspaceFromUrlPage.tsx:
##########
@@ -96,6 +98,25 @@ export function NewWorkspaceFromUrlPage() {
);
const { clonableUrl, selectedGitRefName, gitServerRefsPromise } = clonableUrlObject;
+ if (
+ gitServerRefsPromise.error?.toString().includes("Unauthorized") &&
+ isCertainlyGit(importableUrl.type) &&
+ importableUrl.url &&
+ !authInfo
+ ) {
+ const { compatible } = getCompatibleAuthSessionWithUrlDomain({
+ authProviders,
+ authSessions,
+ authSessionStatus,
+ urlDomain: new URL(importableUrl.url).hostname,
+ });
+ setAuthInfo({
+ username: (compatible as GitAuthSession[])[0].login,
+ uuid: (compatible as GitAuthSession[])[0].uuid,
+ password: (compatible as GitAuthSession[])[0].token,
+ });
+ }
Review Comment:
Using an `if` statement directly on the component's function implementation is not something we really do, since it will potentially lead to very strange behaviors, as this `if` statement will be executed every time `NewWorkspaceFromUrlPage` re-renders.
That's one of the reasons why we have the general rule of always encapsulating code inside hooks, like `useEffect`.
More than that, this code waits for an error to happen, checks for the error string (which is very weak in terms of reliability) and tries to isolate a scenario so that `authInfo` can be set.
Instead of trying to find a way to really "ignore" everything else going around the fix, we always need to try and understand how the current code is working, and how the current abstractions can be adapted so that the bug can be fixed.
We have a very similar code to this one on the effect marked with `// Startup the page. Only import if those are set.`.
##########
packages/online-editor/src/importFromUrl/NewWorkspaceFromUrlPage.tsx:
##########
@@ -76,7 +76,9 @@ export function NewWorkspaceFromUrlPage() {
const authProviders = useAuthProviders();
const { authSessions, authSessionStatus } = useAuthSessions();
- const { authSession, gitConfig, authInfo } = useAuthSession(queryParamAuthSessionId);
+ const authSessionInfo = useAuthSession(queryParamAuthSessionId);
+ const { authSession, gitConfig } = authSessionInfo;
+ const [authInfo, setAuthInfo] = useState<AuthInfo | undefined>(authSessionInfo.authInfo);
Review Comment:
Creating this state introduces an ambiguity, as `authInfo` is already determined by the `queryParamAuthSessionId`. We should be setting `queryParamAuthSessionId` correctly and re-rendering the page instead of introducing a new state.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#129: Import private repository into sandbox via chrome-extension [backend-part] [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento merged PR #2087:
URL: https://github.com/apache/incubator-kie-tools/pull/2087
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org