content,internal: update versions page

The versions page is updated to: - Display incompatible versions in a separate section - Remove minor versions of other modules A version.IsIncompatible function is added to the internal/version package for determining if a version is an incompatible version. Mock: https://photos.app.goo.gl/wWEFsMrmtaxggTAk7 Change-Id: I9a1c533066c3ea8b3f9f9b65f9bd171290655b5e Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/288762 Trust: Julie Qiu <julie@golang.org> Run-TryBot: Julie Qiu <julie@golang.org> TryBot-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
2021-01-31 22:06:02 -05:00 · 2021-01-31 22:06:02 -05:00 · 84574e5b8d
--- a/content/static/css/stylesheet.css
+++ b/content/static/css/stylesheet.css
@ -1422,36 +1422,36 @@ pre {
  word-wrap: break-word;
 }

-.Versions-list {
-  list-style: none;
-  padding-left: 1rem;
+.Unit-content .Versions {
+  margin-top: 1rem;
 }
-.Versions-list li {
-  font-size: 1.125rem;
-  margin-bottom: 0.5rem;
-  line-height: 1.5rem;
-  padding-left: 1rem;
+.Versions table {
+  border-spacing: 0;
 }
-.Versions-item {
-  margin-left: 1rem;
+.Versions th {
+  text-align: left;
+}
+.Versions td {
+  padding-bottom: 1rem;
+}
+.Versions td:nth-child(1) {
+  padding-right: 3rem;
+}
+.Versions td:nth-child(2) {
+  padding-right: 1rem;
+  text-align: right;
+  border-right: 1px solid var(--gray-8);
+}
+.Versions td:nth-child(3) {
+  padding-left: 1rem;
 }
 .Versions-commitTime {
  color: var(--gray-3);
  font-weight: 400;
  font-size: 1rem;
 }
-.Versions-message {
-  color: var(--gray-3);
-  margin-bottom: 2rem;
-}
-.Versions-modulePath {
-  color: var(--gray-3);
-  font-size: 1rem;
-  font-weight: normal;
-}
-.Versions-separator {
-  border-bottom: 0.0625rem solid var(--gray-8);
-  margin: 2rem 0;
+.Versions-major {
+  font-weight: 600;
 }

 .Imports-list {
--- a/content/static/html/helpers/_versions.tmpl
+++ b/content/static/html/helpers/_versions.tmpl
@ -4,42 +4,40 @@
  license that can be found in the LICENSE file.
 -->

-{{define "module_list"}}
-  {{range $major := .}}
-    <h2>
-      {{.Major}}
-      {{if not (eq $major.ModulePath "std")}}
-        <span class="Versions-modulePath"> &ndash; {{$major.ModulePath}}</span>
-      {{end}}
-    </h2>
-    <ul class="Versions-list">
-      {{range $v := $major.Versions}}
-        <li class="Versions-item">
-          <a href="{{$v.Link}}">{{$v.Version}}</a>
-          <span class="Versions-commitTime"> &ndash; {{$v.CommitTime}}</span>
-        </li>
-      {{end}}
-    </ul>
-  {{end}}
-{{end}}
-
 {{define "versions"}}
  <div class="Versions">
-    {{if or .OtherModules .ThisModule}}
-      {{if .OtherModules}}
-        <h2>Versions in this module</h2>
+    <table>
+      <tr><th colspan="3"><h2>Versions in this module</h2></th></tr>
+      {{template "module_list" .ThisModule}}
+      {{if .IncompatibleModules}}
+        <tr><th colspan="3"><h2>Incompatible versions in this module</h2></th></tr>
+        {{template "module_list" .IncompatibleModules}}
      {{end}}
-      {{if .ThisModule}}
-        {{template "module_list" .ThisModule}}
-      {{else}}
-        <p class="Versions-message">There are no tagged versions of this module.</p>
+    </table>
+
+    {{if .OtherModules}}
+      <h2>Other modules containing this package</h2>
+      {{range .OtherModules}}
+        <div><a href="/{{.}}">{{.}}</a></div>
      {{end}}
-      {{if .OtherModules}}
-        <h2>Other modules containing this package</h2>
-        {{template "module_list" .OtherModules}}
-      {{end}}
-    {{else}}
-      {{template "empty_content" "No other known versions of this package!"}}
    {{end}}
  </div>
 {{end}}
+
+{{define "module_list"}}
+  {{range $major := .}}
+     {{range $i, $v := $major.Versions}}
+       <tr>
+         <td>
+           {{if eq $i 0 }}<div class="Versions-major">{{$major.Major}}</div>{{end}}
+         </td>
+         <td>
+           <a href="{{$v.Link}}">{{$v.Version}}</a>
+         </td>
+         <td>
+           <div class="Versions-commitTime">{{$v.CommitTime}}</div>
+         </td>
+       </tr>
+     {{end}}
+  {{end}}
+{{end}}
--- a/internal/frontend/versions.go
+++ b/internal/frontend/versions.go
@ -27,9 +27,13 @@ type VersionsDetails struct {
 	// current package.
 	ThisModule []*VersionList

+	// IncompatibleModules is the slice of the VersionsLists with the same
+	// module path as the current package, but with incompatible versions.
+	IncompatibleModules []*VersionList
+
 	// OtherModules is the slice of VersionLists with a different module path
 	// from the current package.
-	OtherModules []*VersionList
+	OtherModules []string
 }

 // VersionListKey identifies a version list on the versions tab. We have a
@ -39,8 +43,13 @@ type VersionsDetails struct {
 type VersionListKey struct {
 	// ModulePath is the module path of this major version.
 	ModulePath string
+
 	// Major is the major version string (e.g. v1, v2)
 	Major string
+
+	// Incompatible indicates whether the VersionListKey represents an
+	// incompatible module version.
+	Incompatible bool
 }

 // VersionList holds all versions corresponding to a unique (module path,
@ -110,7 +119,6 @@ func pathInVersion(v1Path string, mi *internal.ModuleInfo) string {
 // path as the package version under consideration, and those that don't.  The
 // given versions MUST be sorted first by module path and then by semver.
 func buildVersionDetails(currentModulePath string, modInfos []*internal.ModuleInfo, linkify func(v *internal.ModuleInfo) string) *VersionsDetails {
-
 	// lists organizes versions by VersionListKey. Note that major version isn't
 	// sufficient as a key: there are packages contained in the same major
 	// version of different modules, for example github.com/hashicorp/vault/api,
@ -140,11 +148,17 @@ func buildVersionDetails(currentModulePath string, modInfos []*internal.ModuleIn
 				// Trim both '/' and '.' from the path major version to account for
 				// standard and gopkg.in module paths.
 				major = strings.TrimLeft(pathMajor, "/.")
+			} else if version.IsIncompatible(mi.Version) {
+				major = semver.Major(mi.Version)
 			} else if major != "v0" && !strings.HasPrefix(major, "go") {
 				major = "v1"
 			}
 		}
-		key := VersionListKey{ModulePath: mi.ModulePath, Major: major}
+		key := VersionListKey{
+			ModulePath:   mi.ModulePath,
+			Major:        major,
+			Incompatible: version.IsIncompatible(mi.Version),
+		}
 		vs := &VersionSummary{
 			Link:       linkify(mi),
 			CommitTime: absoluteTime(mi.CommitTime),
@ -157,17 +171,25 @@ func buildVersionDetails(currentModulePath string, modInfos []*internal.ModuleIn
 	}

 	var details VersionsDetails
+	other := map[string]bool{}
 	for _, key := range seenLists {
 		vl := &VersionList{
 			VersionListKey: key,
 			Versions:       lists[key],
 		}
 		if key.ModulePath == currentModulePath {
-			details.ThisModule = append(details.ThisModule, vl)
+			if key.Incompatible {
+				details.IncompatibleModules = append(details.IncompatibleModules, vl)
+			} else {
+				details.ThisModule = append(details.ThisModule, vl)
+			}
 		} else {
-			details.OtherModules = append(details.OtherModules, vl)
+			other[key.ModulePath] = true
 		}
 	}
+	for m := range other {
+		details.OtherModules = append(details.OtherModules, m)
+	}
 	return &details
 }

--- a/internal/frontend/versions_test.go
+++ b/internal/frontend/versions_test.go
@ -79,9 +79,9 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 			true),
 		Documentation: []*internal.Documentation{sample.Doc},
 	}
-	makeList := func(pkgPath, modulePath, major string, versions []string) *VersionList {
+	makeList := func(pkgPath, modulePath, major string, versions []string, incompatible bool) *VersionList {
 		return &VersionList{
-			VersionListKey: VersionListKey{ModulePath: modulePath, Major: major},
+			VersionListKey: VersionListKey{ModulePath: modulePath, Major: major, Incompatible: incompatible},
 			Versions: versionSummaries(pkgPath, versions, func(path, version string) string {
 				return constructUnitURL(pkgPath, modulePath, version)
 			}),
@ -103,7 +103,7 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 			},
 			wantDetails: &VersionsDetails{
 				ThisModule: []*VersionList{
-					makeList("net/http", "std", "go1", []string{"go1.12.5", "go1.11.6"}),
+					makeList("net/http", "std", "go1", []string{"go1.12.5", "go1.11.6"}, false),
 				},
 			},
 		},
@ -113,6 +113,7 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 			modules: []*internal.Module{
 				sampleModule(modulePath1, "v0.0.0-20140414041502-3c2ca4d52544", version.TypePseudo, pkg2),
 				sampleModule(modulePath1, "v1.2.3", version.TypeRelease, pkg1),
+				sampleModule(modulePath1, "v2.1.0+incompatible", version.TypeRelease, pkg1),
 				sampleModule(modulePath2, "v2.0.0", version.TypeRelease, pkg2),
 				sampleModule(modulePath1, "v1.3.0", version.TypeRelease, pkg1),
 				sampleModule(modulePath1, "v1.2.1", version.TypeRelease, pkg1),
@ -121,12 +122,12 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 			},
 			wantDetails: &VersionsDetails{
 				ThisModule: []*VersionList{
-					makeList(v1Path, modulePath1, "v1", []string{"v1.3.0", "v1.2.3", "v1.2.1"}),
+					makeList(v1Path, modulePath1, "v1", []string{"v1.3.0", "v1.2.3", "v1.2.1"}, false),
 				},
-				OtherModules: []*VersionList{
-					makeList(v2Path, modulePath2, "v2", []string{"v2.2.1-alpha.1", "v2.0.0"}),
-					makeList(v1Path, "test.com", "v1", []string{"v1.2.1"}),
+				IncompatibleModules: []*VersionList{
+					makeList(v1Path, modulePath1, "v2", []string{"v2.1.0+incompatible"}, true),
 				},
+				OtherModules: []string{modulePath2, "test.com"},
 			},
 		},
 		{
@ -142,11 +143,9 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 			},
 			wantDetails: &VersionsDetails{
 				ThisModule: []*VersionList{
-					makeList(v2Path, modulePath2, "v2", []string{"v2.2.1-alpha.1", "v2.0.0"}),
-				},
-				OtherModules: []*VersionList{
-					makeList(v1Path, modulePath1, "v1", []string{"v1.2.3", "v1.2.1", "v2.1.0+incompatible"}),
+					makeList(v2Path, modulePath2, "v2", []string{"v2.2.1-alpha.1", "v2.0.0"}, false),
 				},
+				OtherModules: []string{modulePath1},
 			},
 		},
 		{
@ -157,12 +156,7 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 				sampleModule(modulePath1, "v0.0.0-20140414041502-4c2ca4d52544", version.TypePseudo, pkg2),
 			},
 			wantDetails: &VersionsDetails{
-				OtherModules: []*VersionList{
-					makeList(v1Path, modulePath1, "v0", []string{
-						"v0.0.0-20140414041502-4c2ca4d52544",
-						"v0.0.0-20140414041501-3c2ca4d52544",
-					}),
-				},
+				OtherModules: []string{modulePath1},
 			},
 		},
 	} {
@ -182,11 +176,6 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
 					v.CommitTime = absoluteTime(tc.modules[0].CommitTime)
 				}
 			}
-			for _, vl := range tc.wantDetails.OtherModules {
-				for _, v := range vl.Versions {
-					v.CommitTime = absoluteTime(tc.modules[0].CommitTime)
-				}
-			}
 			if diff := cmp.Diff(tc.wantDetails, got); diff != "" {
 				t.Errorf("mismatch (-want +got):\n%s", diff)
 			}
--- a/internal/postgres/insert_module.go
+++ b/internal/postgres/insert_module.go
@ -205,7 +205,7 @@ func insertModule(ctx context.Context, db *database.DB, m *internal.Module) (_ i
 		m.IsRedistributable,
 		m.HasGoMod,
 		m.DeprecatedComment,
-		isIncompatible(m.Version),
+		version.IsIncompatible(m.Version),
 	).Scan(&moduleID)
 	if err != nil {
 		return 0, err
@ -581,12 +581,6 @@ func lock(ctx context.Context, tx *database.DB, modulePath string) (err error) {
 	return nil
 }

-// isIncompatible reports whether the build metadata of the version is
-// "+incompatible", https://semver.org clause 10.
-func isIncompatible(version string) bool {
-	return strings.HasSuffix(version, "+incompatible")
-}
-
 // isLatestVersion reports whether version is the latest version of the module.
 func isLatestVersion(ctx context.Context, ddb *database.DB, modulePath, resolvedVersion string) (_ bool, err error) {
 	defer derrors.WrapStack(&err, "isLatestVersion(ctx, tx, %q)", modulePath)
--- a/internal/postgres/versionstate.go
+++ b/internal/postgres/versionstate.go
@ -27,7 +27,7 @@ func (db *DB) InsertIndexVersions(ctx context.Context, versions []*internal.Inde

 	var vals []interface{}
 	for _, v := range versions {
-		vals = append(vals, v.Path, v.Version, version.ForSorting(v.Version), v.Timestamp, 0, "", "", isIncompatible(v.Version))
+		vals = append(vals, v.Path, v.Version, version.ForSorting(v.Version), v.Timestamp, 0, "", "", version.IsIncompatible(v.Version))
 	}
 	cols := []string{"module_path", "version", "sort_version", "index_timestamp", "status", "error", "go_mod_path", "incompatible"}
 	conflictAction := `
@ -116,7 +116,7 @@ func upsertModuleVersionState(ctx context.Context, db *database.DB, modulePath,
 						CURRENT_TIMESTAMP + INTERVAL '1 hour'
 					END;`,
 		modulePath, vers, version.ForSorting(vers),
-		appVersion, timestamp, status, goModPath, sqlErrorMsg, numPackages, isIncompatible(vers))
+		appVersion, timestamp, status, goModPath, sqlErrorMsg, numPackages, version.IsIncompatible(vers))
 	if err != nil {
 		return err
 	}
--- a/internal/version/version.go
+++ b/internal/version/version.go
@ -41,6 +41,11 @@ func IsPseudo(v string) bool {
 	return strings.Count(v, "-") >= 2 && pseudoVersionRE.MatchString(v)
 }

+// IsIncompatible reports whether a valid version v is an incompatible version.
+func IsIncompatible(v string) bool {
+	return strings.HasSuffix(v, "+incompatible")
+}
+
 // ParseType returns the Type of a given a version.
 func ParseType(version string) (Type, error) {
 	if !semver.IsValid(version) {