Returns a clearer error on invalid Helm version. Closes #2665 and #2736 (#2666)

reposerver/repository/repository.go

@@ -777,7 +777,7 @@ func (s *Service) newHelmClientResolveRevision(repo *v1alpha1.Repository, revisi
 	helmClient := s.newHelmClient(repo.Repo, repo.GetHelmCreds())
 	constraints, err := semver.NewConstraint(revision)
 	if err != nil {
-		return nil, "", err
+		return nil, "", fmt.Errorf("invalid revision '%s': %v", revision, err)
 	}
 	index, err := helmClient.GetIndex()
 	if err != nil {

reposerver/repository/repository_test.go

@@ -446,3 +446,16 @@ func Test_newEnv(t *testing.T) {
 		},
 	}, "my-revision"))
 }
+
+func TestService_newHelmClientResolveRevision(t *testing.T) {
+	service := newService(".")
+
+	t.Run("EmptyRevision", func(t *testing.T) {
+		_, _, err := service.newHelmClientResolveRevision(&argoappv1.Repository{}, "", "")
+		assert.EqualError(t, err, "invalid revision '': improper constraint: ")
+	})
+	t.Run("InvalidRevision", func(t *testing.T) {
+		_, _, err := service.newHelmClientResolveRevision(&argoappv1.Repository{}, "???", "")
+		assert.EqualError(t, err, "invalid revision '???': improper constraint: ???")
+	})
+}

ui/src/app/shared/components/__snapshots__/revision.test.tsx.snap

@@ -0,0 +1,33 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Revision.Branch.Children 1`] = `
+<a
+  href="http://github.com/my-org/my-repo/commit/long-branch-name"
+>
+  foo
+</a>
+`;
+
+exports[`Revision.Branch.NoChildren 1`] = `
+<a
+  href="http://github.com/my-org/my-repo/commit/long-branch-name"
+>
+  long-branch-name
+</a>
+`;
+
+exports[`Revision.SHA1.Children 1`] = `
+<a
+  href="http://github.com/my-org/my-repo/commit/24eb0b24099b2e9afff72558724e88125eaa0176"
+>
+  foo
+</a>
+`;
+
+exports[`Revision.SHA1.NoChildren 1`] = `
+<a
+  href="http://github.com/my-org/my-repo/commit/24eb0b24099b2e9afff72558724e88125eaa0176"
+>
+  24eb0b2
+</a>
+`;

ui/src/app/shared/components/revision.test.tsx

@@ -0,0 +1,33 @@
+import * as renderer from 'react-test-renderer';
+import * as React from 'react';
+import {isSHA, Revision} from "./revision";
+
+test('Revision.SHA1.Children', () => {
+    const tree = renderer.create(<Revision repoUrl='http://github.com/my-org/my-repo' revision='24eb0b24099b2e9afff72558724e88125eaa0176'>foo</Revision>).toJSON();
+
+    expect(tree).toMatchSnapshot()
+});
+
+test('Revision.SHA1.NoChildren', () => {
+    const tree = renderer.create(<Revision repoUrl='http://github.com/my-org/my-repo' revision='24eb0b24099b2e9afff72558724e88125eaa0176'/>).toJSON();
+
+    expect(tree).toMatchSnapshot()
+});
+
+test('Revision.Branch.Children', () => {
+    const tree = renderer.create(<Revision repoUrl='http://github.com/my-org/my-repo' revision='long-branch-name'>foo</Revision>).toJSON();
+
+    expect(tree).toMatchSnapshot()
+});
+
+
+test('Revision.Branch.NoChildren', () => {
+    const tree = renderer.create(<Revision repoUrl='http://github.com/my-org/my-repo' revision='long-branch-name'/>).toJSON();
+
+    expect(tree).toMatchSnapshot()
+});
+
+test('isSHA1', () => {
+    expect(isSHA('24eb0b24099b2e9afff72558724e88125eaa0176')).toBe(true);
+    expect(isSHA('master')).toBe(false);
+});

ui/src/app/shared/components/revision.tsx

@@ -4,6 +4,11 @@ import {revisionUrl} from './urls';
 export const Revision = ({repoUrl, revision, children}: {repoUrl: string; revision: string; children?: React.ReactNode}) => {
     revision = revision || '';
     const url = revisionUrl(repoUrl, revision);
-    const content = children || revision.substr(0, 7);
+    const content = children || (isSHA(revision) ? revision.substr(0, 7) : revision);
     return url !== null ? <a href={url}>{content}</a> : <span>{content}</span>;
 };
+
+export const isSHA = (revision: string) => {
+    // https://stackoverflow.com/questions/468370/a-regex-to-match-a-sha1
+    return revision.match(/[0-9a-f]{5,40}/) !== null;
+};