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

fix: remotes url parameter in factory url #937

Closed
wants to merge 1 commit into from

Conversation

dkwon17
Copy link
Contributor

@dkwon17 dkwon17 commented Sep 29, 2023

What does this PR do?

Fixes eclipse-che/che#22530 by allowing redirect to workspace creation page when window.location.hash contains the following characters: {, }, and ,.

See:

const hash = window.location.hash.replace(/(\/?)#(\/?)/, '');
if (FactoryLocationAdapter.isHttpLocation(hash) || FactoryLocationAdapter.isSshLocation(hash)) {
// project url found, redirect to the workspaces creation page
window.location.href = window.location.origin + '/dashboard' + buildFactoryLoaderPath(hash);
return;
}

What issues does this PR fix or reference?

eclipse-che/che#22530

Is it tested? How?

  1. Deploy Eclipse Che, and set the following in the Che CR to test out this PR:
spec:
  components:
    dashboard:
      deployment:
        containers:
          - image: quay.io/dkwon17/che-dashboard:fixRemotesUrlParam
  1. Access the following url to start a new workspace:
{che-host}/#https://github.com/dkwon17/quarkus-api-example?remotes={https://github.com/che-incubator/quarkus-api-example}

and verify that the resulting dev workspace has the following:

    projects:
      - git:
          checkoutFrom:
            remote: origin
          remotes:
            origin: 'https://github.com/che-incubator/quarkus-api-example'
        name: quarkus-api-example
  1. Access the following url to start a new workspace:
{che-host}/#https://github.com/eclipse-che/che-plugin-registry?remotes={{origin,https://github.com/dkwon17/che-plugin-registry},{upstream,https://github.com/eclipse-che/che-plugin-registry}}

and verify that the resulting dev workspace has the following:

    projects:
      - git:
          checkoutFrom:
            remote: origin
          remotes:
            origin: 'https://github.com/dkwon17/che-plugin-registry'
            upstream: 'https://github.com/eclipse-che/che-plugin-registry'
        name: che-plugin-registry

Release Notes

Docs PR

Signed-off-by: David Kwon <dakwon@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Sep 29, 2023

Click here to review and test in web IDE: Contribute

@openshift-ci
Copy link

openshift-ci bot commented Sep 29, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dkwon17

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@@ -52,7 +52,7 @@ export class FactoryLocationAdapter implements FactoryLocation {
}

public static isHttpLocation(href: string): boolean {
return /^(https?:\/\/.)[-a-zA-Z0-9@:%._+~#=]{2,}\b([-a-zA-Z0-9@:%_+.~#?&/=]*)$/.test(href);
return /^(https?:\/\/.)[-a-zA-Z0-9@:%._+~#=]{2,}\b([-a-zA-Z0-9@:%_+.~#?&/={},]*)$/.test(href);
Copy link
Contributor Author

@dkwon17 dkwon17 Sep 29, 2023

Choose a reason for hiding this comment

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

{ and } characters are not considered valid for URIs, but since they can still exist in the window.location object, I propose to add it to this regex

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #937 (83cdfa6) into main (333d6cf) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #937   +/-   ##
=======================================
  Coverage   82.59%   82.59%           
=======================================
  Files         365      365           
  Lines       37941    37941           
  Branches     2414     2414           
=======================================
  Hits        31338    31338           
  Misses       6579     6579           
  Partials       24       24           
Flag Coverage Δ
unittests 82.59% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...end/src/services/factory-location-adapter/index.ts 100.00% <100.00%> (ø)

@dkwon17 dkwon17 closed this Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remotes parameter in factory url is not working
2 participants